Skip to content

Commit f6406cc

Browse files
authored
Fix negative token width for glyphs wrapped after a diacritic (#192) (#246)
A word break at a line wrap was suppressed when the preceding glyph was a combining mark, gluing the wrapped glyph onto the previous line's word and giving that token a negative WIDTH. Overriding the suppression on a baseline shift alone proved too broad — accents are drawn raised above their letter, so they shift the baseline on the same line, and ordinary words were split apart ("Álvarez" → "´" + "lvarez"); over the 5,927-PDF GROBID corpus that changed 1,209 files when only 297 had anything to fix. Baseline shift can't separate the two cases (0.24 vs 0.21 median relative to font size), but the horizontal gap can: a combining mark never starts more than 0.79 font sizes left of the word's end, while a wrapped glyph restarts at the left margin and reaches −67. The suppression is now overridden only when both hold — baseline shifted and sp < -fontSize. This PR also merges the duplicated classifyChar tables, which had drifted so that U+00B0 DEGREE SIGN counted as a combining mark in one live call site but not the other, running axis labels together as "0°0", "°20°4" instead of "0°", "20°". Result across the full corpus: negative widths 536 → 37, 394 files changed (vs 1,209), +905 tokens (vs +12,591), no crashes or unparseable output.
1 parent ec939ad commit f6406cc

1 file changed

Lines changed: 37 additions & 89 deletions

File tree

src/XmlAltoOutputDev.cc

Lines changed: 37 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,7 +2346,19 @@ void TextPage::beginWord(GfxState *state, double x0, double y0) {
23462346
curWord = new TextRawWord(state, x0, y0, curFont, curFontSize, getIdWORD(), getIdx());
23472347
}
23482348

2349-
ModifierClass TextPage::classifyChar(Unicode u) {
2349+
// Single source of truth for combining-mark classification.
2350+
//
2351+
// This table used to be duplicated as TextPage::classifyChar and
2352+
// IWord::classifyChar, and the two copies had drifted: the TextPage one also
2353+
// listed U+00B0 DEGREE SIGN as NORDIC_RING. Both were live -- TextPage's from
2354+
// addCharToRawWord, IWord's from TextRawWord::addChar -- so the same character
2355+
// was a combining mark for one word-break decision and not for another.
2356+
//
2357+
// The degree sign is a standalone symbol, not a combining mark; classifying it
2358+
// as one suppressed the word break and glued axis labels such as "40 deg N"
2359+
// into unusable tokens. U+02DA RING ABOVE (730) and U+030A COMBINING RING
2360+
// ABOVE (778) remain, as those genuinely are marks.
2361+
static ModifierClass classifyModifierChar(Unicode u) {
23502362
switch (u) {
23512363
case (Unicode) 776: //COMBINING DIAERESIS
23522364
case (Unicode) 168: //DIAERESIS
@@ -2393,7 +2405,7 @@ ModifierClass TextPage::classifyChar(Unicode u) {
23932405
return TILDE;
23942406

23952407
case 778: //COMBINING
2396-
case 176:
2408+
//case 176:
23972409
case 730:
23982410
return NORDIC_RING;//LOOK AT UNICODE RING BELOW...
23992411

@@ -2431,6 +2443,8 @@ ModifierClass TextPage::classifyChar(Unicode u) {
24312443

24322444
}
24332445

2446+
ModifierClass TextPage::classifyChar(Unicode u) { return classifyModifierChar(u); }
2447+
24342448
/*
24352449
* Returns the correct base char for composition with icu4c following unicode standard.
24362450
*/
@@ -2504,90 +2518,7 @@ Unicode TextPage::getCombiningDiacritic(ModifierClass modifierClass) {
25042518
return diactritic;
25052519
}
25062520

2507-
ModifierClass IWord::classifyChar(Unicode u) {
2508-
switch (u) {
2509-
case (Unicode) 776: //COMBINING DIAERESIS
2510-
case (Unicode) 168: //DIAERESIS
2511-
return DIAERESIS;
2512-
2513-
case 833:
2514-
case 779: // COMBINING DOUBLE_ACUTE_ACCENT
2515-
case 733:
2516-
return DOUBLE_ACUTE_ACCENT;
2517-
case 180:
2518-
case 769: //COMBINING
2519-
case 714:
2520-
return ACUTE_ACCENT;
2521-
2522-
case 768: //COMBINING
2523-
case 832:
2524-
case 715:
2525-
case 96:
2526-
return GRAVE_ACCENT;
2527-
2528-
case 783: //COMBINING
2529-
return DOUBLE_GRAVE_ACCENT;
2530-
2531-
case 774: //COMBINING
2532-
case 728:
2533-
//case '\uA67C':
2534-
return BREVE_ACCENT;
2535-
2536-
case 785: //COMBINING
2537-
case 1156:
2538-
case 1159:
2539-
return INVERTED_BREVE_ACCENT;
2540-
2541-
2542-
case 770: //COMBINING
2543-
case 94:
2544-
case 710:
2545-
return CIRCUMFLEX;
2546-
2547-
2548-
case 771: //COMBINING
2549-
case 126:
2550-
case 732:
2551-
return TILDE;
2552-
2553-
case 778: //COMBINING
2554-
//case 176:
2555-
case 730:
2556-
return NORDIC_RING;//LOOK AT UNICODE RING BELOW...
2557-
2558-
case 780: //COMBINING
2559-
case 711:
2560-
return CZECH_CARON;
2561-
2562-
case 807: //COMBINING
2563-
case 184:
2564-
return CEDILLA;
2565-
2566-
case 775: //COMBINING
2567-
case 729:
2568-
return DOT_ABOVE;
2569-
2570-
case 777: //COMBINING
2571-
case 704:
2572-
return HOOK;
2573-
2574-
case 795: //COMBINING
2575-
return HORN;
2576-
2577-
case 808: //COMBINING
2578-
case 731:
2579-
//case '\u1AB7':// combining open mark below
2580-
return OGONEK;
2581-
2582-
case 772: //COMBINING
2583-
case 175:
2584-
case 713:
2585-
return MACRON;
2586-
default:
2587-
return NOT_A_MODIFIER;
2588-
}
2589-
2590-
}
2521+
ModifierClass IWord::classifyChar(Unicode u) { return classifyModifierChar(u); }
25912522

25922523
Unicode IWord::getCombiningDiacritic(ModifierClass modifierClass) {
25932524

@@ -2964,12 +2895,29 @@ void TextPage::addCharToRawWord(GfxState *state, double x, double y, double dx,
29642895
((TextChar *) curWord->chars->get(curWord->chars->getLength() - 1))->spaceAfter =
29652896
(char) gTrue;
29662897
}
2898+
// A combining mark sits on or beside the glyph it modifies: it shares the
2899+
// baseline and it is never far to the left of the word built so far. A
2900+
// glyph that breaks both of those -- different baseline *and* starting a
2901+
// whole font size or more before the end of the current word -- has
2902+
// wrapped to the next line and is not a mark on this word. Gluing it on
2903+
// produces a token whose width is negative (issue #192), so the
2904+
// diacritic break-suppression below must not apply to it.
2905+
//
2906+
// Both halves are needed. Baseline alone is far too broad: accents drawn
2907+
// raised above their base letter shift the baseline on the *same* line,
2908+
// and splitting there breaks ordinary words apart. Measured over the
2909+
// GROBID end-to-end corpus, sp/fontSize never goes below -0.79 for those
2910+
// same-line marks, while wrapped glyphs reach -67, so the two populations
2911+
// separate cleanly at one font size.
2912+
GBool sameBaseline = fabs(base - curWord->base) <= 1;
2913+
GBool wrappedToNextLine = !sameBaseline && sp < -curWord->fontSize;
2914+
29672915
// take into account rotation angle ??
2968-
if ( (overlap ||
2916+
if ( (overlap ||
29692917
fabs(base - curWord->base) > 1 ||
29702918
space ||
2971-
(sp < -minDupBreakOverlap * curWord->fontSize))
2972-
&& modifierClass == NOT_A_MODIFIER) {
2919+
(sp < -minDupBreakOverlap * curWord->fontSize))
2920+
&& (modifierClass == NOT_A_MODIFIER || wrappedToNextLine)) {
29732921
endWord();
29742922
beginWord(state, x, y);
29752923
}

0 commit comments

Comments
 (0)