Gate the up/down OCR line splitter on ink height, not bitmap height - #14490
Merged
Merged
Conversation
Fixes #14292: nOCR on a one-line DVD subtitle whose leftmost glyph has a descender - "(", "J", ",", "g", "q" - produced an extra unknown character. The VobSub decoder leaves up to seven transparent rows under the text. For a DVD-sized font the adaptive minimum line height settles around 14, and that padded one-line image (31-35 rows) cleared the 2.2 x minLineHeight gate in front of SplitToLinesNew, the "allows for up/down" line splitter. Its path walked down the outer edge of the "(", and since its cut starts one row above the path it eroded the glyph's edge and left the crumbs as a bogus second line - the 2x5 blob in the reporter's screenshots. SplitToLinesNew had been dead code until 9a917c6 (beta27) moved its "started" flag out of the scan loop, which is why earlier builds were fine and why "Crop transparent colors" works around it: without the margin the image never reaches the gate. Gate on GetNonTransparentHeight instead. On the reporter's file the first 600 subtitles go from 19 one-line images split in two (all 11 reported lines among them) to 0, while all 315 genuine two-line images still split. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14292 - nOCR on a one-line DVD subtitle whose leftmost glyph has a descender ("(", "J", ",", "g", "q") produced an extra unknown character.
Cause
The VobSub decoder leaves up to seven transparent rows under the text. For a DVD-sized font the adaptive minimum line height settles around 14, and that padded one-line image (31-35 rows) cleared the
2.2 x minLineHeightgate in front ofSplitToLinesNew, the "allows for up/down" line splitter. Its path walked down the outer edge of the "(", and since its cut starts one row above the path it eroded the glyph's edge and left the crumbs as a bogus second line - the 2x5 blob in the reporter's screenshots.SplitToLinesNewhad been dead code until 9a917c6 (beta27) moved itsstartedflag out of the scan loop, which is why earlier builds were fine and why "Crop transparent colors" works around it: without the margin the image never reaches the gate.Fix
Gate on
GetNonTransparentHeight()instead of the bitmap height.Verification
On the reporter's file, first 600 subtitles, nOCR pipeline as the OCR window runs it:
A regression test rebuilds the "(" from subtitle 125 with the decoder's bottom margin; it fails on main with the 2x5 crumb and passes with the fix. All 352 libuilogic OCR tests pass. Also verified in the UI on the reporter's file.
🤖 Generated with Claude Code