From 474d127bfb081bd1b0074b8894daa5169a06d9f1 Mon Sep 17 00:00:00 2001 From: John Carmack Date: Sat, 5 Sep 2026 15:03:38 -0700 Subject: [PATCH] fix(core): break a label before a left parenthesis, not after it (#4553) --- .../left-parenthesis/expected.png | Bin 0 -> 2757 bytes .../text-breaking/left-parenthesis/style.json | 43 +++++++++++++++ .../text-breaking/whitespace/expected.png | Bin 0 -> 2757 bytes .../text-breaking/whitespace/style.json | 43 +++++++++++++++ src/mln/text/shaping.cpp | 5 +- src/mln/util/i18n.cpp | 5 +- src/mln/util/i18n.hpp | 4 ++ test/text/shaping.test.cpp | 52 ++++++++++++++++++ 8 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 metrics/integration/render-tests/text-breaking/left-parenthesis/expected.png create mode 100644 metrics/integration/render-tests/text-breaking/left-parenthesis/style.json create mode 100644 metrics/integration/render-tests/text-breaking/whitespace/expected.png create mode 100644 metrics/integration/render-tests/text-breaking/whitespace/style.json diff --git a/metrics/integration/render-tests/text-breaking/left-parenthesis/expected.png b/metrics/integration/render-tests/text-breaking/left-parenthesis/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..376691f5a644b8917d5df20d53d29ee0b9157d79 GIT binary patch literal 2757 zcmbW3|34FmAIHsii?R8#FNRFMQ{+n{WcE#3#Z0-lwJ*Y<+JxIDa?F^SZK;Hk)a+`y zY90|4Wya>Z-K9sAE@@v*t?$-XcjxNk{)Kxy-d}!uzF+V65AXL|kiL!R1VjKWEG(Qh zr{c&fx#nNDw_S;FEF7Qp7sI1GgYc&$8ithKz>LicRs^C|igjFk+Va7j;R=k!Nx zflxS@oLL|D;h`NZ`7%;@@!|C56SZ zst^Ss4+Noi^>sJwHyCZ_nk0tGAwcLW6~jscZs&XS-;SoEa$jFZ%;E|SB>9xoKv_L=Rw?Dn9-OWzziZzAAE2W>iZ3p%5(cZU4-C=b$?ZWtI1E$I+5pC6G3>?CT zD1)ke0=sQP*@;xOekqbV22DR458cUt->kPXyL?$y%#H(%S|*5o82fJ4Injjii8sTe zUlQXg`SvmGqWboRHgo0pkOP#N=pnUwV7NfWvZIUgdRYLqfj%sbV!Jk^dhr|@kDX}P zB@by<1!p&gECY(!@U^OtT(O;*-LJb*c>fN=Y76KDT?)tUPNt78?o_IkUZk&U-98_7c15_dG~K^HH`} zwMd%5|GQ6R;o`}8$HB-a-wd2}{$8K|;pa7`GGCCclDr}`zyiu4CIM)$wSjvQJJ){ZV_BE%F+@_?r6uts^HtZ zp@)JHs~MHI7KWfu1AEySg5-t|jbHMI31r!Pc)OUth-}Ih-%?s}Z25FY;3tw_5qsl& z3ivG}ZXZ!veS8B8S?`-&3q5vH7|ua%UGLD95*VOkwl}R!s4a+eFXwpQF+?;gx58?_ zfpJe85BB>i5zS-PB=s*t?si6-gnEQl86Sql^dbtF*7Fk}w@8Nqy4&&R1ueDy5)@>i zl#IWzQ>0Iws^wC6NFW*f&U6$}i@X{NrCXy)+L|(wQLRLN*-uZJG=3dajxf?sDr zu78xyj*=CX__vp#Be(s7CfAsbKBRw!lm*S4qhPLKBY`3lo|^zlj-g@(t34c&!iWa% zW)-5JmH@(S^w7co34>I%gC=(CBgP9=?>Uwt?rM83fMH+yDYE5Di{=SE@D0IA-1Zz3 zwZn+_xp{C-l&{5REOc@=X_%ah`iZ|ZHbaO9TYeoIyxRc!Bb{&knFatRC1k+_)IZWG zahDCYNa3o!Oq+$Qc~%%9^iuM=e0|Nd#k`)OMome++OeE!SEfgvqgJm zdnw*v$c;84NU(|ONiZ;e8jE*#yD)q&68?a}Cvn3$)FK;1gpX^_zU1Z!wA~f+lOdbP zwWDwtU&OJHq=}RTk#^E4p8HbdUyIJOHO1-Ufs1iSvE|jx?kv0G#)Qu1;KoH@HSbx^ zK3urR*^Ht!mo(dQ1(x3Gwg?}|>I~n$+b8S1IBD0ADBO8XUKuSMMN&DElx!33^7ICF z@cbwODY-T7J_HrAz&mJu0Zho3%w7OrZ%Z13P>e~_21ps*;XV^~liQMAzpDxmsLz~j zVzQ9cH3T^rZTBd>ZrgA}IT~U|KME9N?TGnN3cnoOs!Gt}V?Y?^OG+S)Y{zs>5<(ST zMEy$pHicu+%fL2629m#Z$nipk=wQjXwJb>>AF})^MymCcWk`T7WfV(PD1ipOoy*oZNII>B_5|YP-3pqfG-rb1qQ;h z#dezX+Y1<8g&cKNiooufAI;N_Mhpw>|NhfBQs;YqC#LUquhONgd%GO9PqlZFj=ZGW zselirl^e5UYLxn6dvFvaC%tYzJ#d>6n_Zu?n7PoUj@4rQ2>LZ8qoL#|mX}yf=x3r! zliES!FgcgeIwMSSBzhlO?{NLD-^T!NWdW<1#O0E|4WdO>H*rbaHrZ1nEVt%=L+<-& zN6B||PgpVgu)Kk#wXwdEml1YP)c=^qWoWA0zC|11n7I!PG*MPmzQ6^(h;Z|&xC|sp zZBNZV@|mhT2=)Fj-!;Z;7gd@=-Pl2lDyu=` z)?qH}bPANZb6-#Z4OMEwI>+h?3MT@zqiqVr=}~iP4$J-3IW4t0 zDO1y(wk2CZ997nxwFlU)9)|sFj~)7=-1;QBKN=EcmdrM36z>1)sEcsAtP6K{lMmnJ zAod%}K6C>=rj<)=@EumN*heE~nBM^WQ4gxhx;#|@-b;>aCPEJ`q7Ry5hW%p@4~)^v zHeIShQu2oT(i*mDtU{g;z=X{cq6mJHi{#zoqNR{GnJe=tR^HNubP8p&g01sU^54Go z8kXjAMYHE~!6$pfV`aK0o?qW5MEKpzx!_|9nbo z?K7vJ)bP~B5CYv6!E(O;=e;Oa@?F5=P)S9a9arFjPuCK zzOrqd)J4Kw@A3xHylSl$W2vntc!KpKg&I%Okq(}q;`QdrYqL6bl84SS`@%G-%5KoO zOa7kiU(uZtBH1w~*eLr=lQmoG-I3o}|NW~8Mja7UJ{jDG^T? zr_w&9tns9|hFrirQ;M&0{_M-&JOmU@iDhJbn98;z%eHo<25l9UToeFVfJ8G#ri*8N zoY3xDWRL{04p(OmqmzLst$n}xro#9R{i z)Kak)55i70oMfM-fcFN$c&IvJqBz-g1&&bDy<)e_80}>hw8UL|Ua+lCEFM`o693Jd cXTAbrTmG=$p8LgTZ-K9sAE@@v*t?$-XcjxNk{)Kxy-d}!uzF+V65AXL|kiL!R1VjKWEG(Qh zr{c&fx#nNDw_S;FEF7Qp7sI1GgYc&$8ithKz>LicRs^C|igjFk+Va7j;R=k!Nx zflxS@oLL|D;h`NZ`7%;@@!|C56SZ zst^Ss4+Noi^>sJwHyCZ_nk0tGAwcLW6~jscZs&XS-;SoEa$jFZ%;E|SB>9xoKv_L=Rw?Dn9-OWzziZzAAE2W>iZ3p%5(cZU4-C=b$?ZWtI1E$I+5pC6G3>?CT zD1)ke0=sQP*@;xOekqbV22DR458cUt->kPXyL?$y%#H(%S|*5o82fJ4Injjii8sTe zUlQXg`SvmGqWboRHgo0pkOP#N=pnUwV7NfWvZIUgdRYLqfj%sbV!Jk^dhr|@kDX}P zB@by<1!p&gECY(!@U^OtT(O;*-LJb*c>fN=Y76KDT?)tUPNt78?o_IkUZk&U-98_7c15_dG~K^HH`} zwMd%5|GQ6R;o`}8$HB-a-wd2}{$8K|;pa7`GGCCclDr}`zyiu4CIM)$wSjvQJJ){ZV_BE%F+@_?r6uts^HtZ zp@)JHs~MHI7KWfu1AEySg5-t|jbHMI31r!Pc)OUth-}Ih-%?s}Z25FY;3tw_5qsl& z3ivG}ZXZ!veS8B8S?`-&3q5vH7|ua%UGLD95*VOkwl}R!s4a+eFXwpQF+?;gx58?_ zfpJe85BB>i5zS-PB=s*t?si6-gnEQl86Sql^dbtF*7Fk}w@8Nqy4&&R1ueDy5)@>i zl#IWzQ>0Iws^wC6NFW*f&U6$}i@X{NrCXy)+L|(wQLRLN*-uZJG=3dajxf?sDr zu78xyj*=CX__vp#Be(s7CfAsbKBRw!lm*S4qhPLKBY`3lo|^zlj-g@(t34c&!iWa% zW)-5JmH@(S^w7co34>I%gC=(CBgP9=?>Uwt?rM83fMH+yDYE5Di{=SE@D0IA-1Zz3 zwZn+_xp{C-l&{5REOc@=X_%ah`iZ|ZHbaO9TYeoIyxRc!Bb{&knFatRC1k+_)IZWG zahDCYNa3o!Oq+$Qc~%%9^iuM=e0|Nd#k`)OMome++OeE!SEfgvqgJm zdnw*v$c;84NU(|ONiZ;e8jE*#yD)q&68?a}Cvn3$)FK;1gpX^_zU1Z!wA~f+lOdbP zwWDwtU&OJHq=}RTk#^E4p8HbdUyIJOHO1-Ufs1iSvE|jx?kv0G#)Qu1;KoH@HSbx^ zK3urR*^Ht!mo(dQ1(x3Gwg?}|>I~n$+b8S1IBD0ADBO8XUKuSMMN&DElx!33^7ICF z@cbwODY-T7J_HrAz&mJu0Zho3%w7OrZ%Z13P>e~_21ps*;XV^~liQMAzpDxmsLz~j zVzQ9cH3T^rZTBd>ZrgA}IT~U|KME9N?TGnN3cnoOs!Gt}V?Y?^OG+S)Y{zs>5<(ST zMEy$pHicu+%fL2629m#Z$nipk=wQjXwJb>>AF})^MymCcWk`T7WfV(PD1ipOoy*oZNII>B_5|YP-3pqfG-rb1qQ;h z#dezX+Y1<8g&cKNiooufAI;N_Mhpw>|NhfBQs;YqC#LUquhONgd%GO9PqlZFj=ZGW zselirl^e5UYLxn6dvFvaC%tYzJ#d>6n_Zu?n7PoUj@4rQ2>LZ8qoL#|mX}yf=x3r! zliES!FgcgeIwMSSBzhlO?{NLD-^T!NWdW<1#O0E|4WdO>H*rbaHrZ1nEVt%=L+<-& zN6B||PgpVgu)Kk#wXwdEml1YP)c=^qWoWA0zC|11n7I!PG*MPmzQ6^(h;Z|&xC|sp zZBNZV@|mhT2=)Fj-!;Z;7gd@=-Pl2lDyu=` z)?qH}bPANZb6-#Z4OMEwI>+h?3MT@zqiqVr=}~iP4$J-3IW4t0 zDO1y(wk2CZ997nxwFlU)9)|sFj~)7=-1;QBKN=EcmdrM36z>1)sEcsAtP6K{lMmnJ zAod%}K6C>=rj<)=@EumN*heE~nBM^WQ4gxhx;#|@-b;>aCPEJ`q7Ry5hW%p@4~)^v zHeIShQu2oT(i*mDtU{g;z=X{cq6mJHi{#zoqNR{GnJe=tR^HNubP8p&g01sU^54Go z8kXjAMYHE~!6$pfV`aK0o?qW5MEKpzx!_|9nbo z?K7vJ)bP~B5CYv6!E(O;=e;Oa@?F5=P)S9a9arFjPuCK zzOrqd)J4Kw@A3xHylSl$W2vntc!KpKg&I%Okq(}q;`QdrYqL6bl84SS`@%G-%5KoO zOa7kiU(uZtBH1w~*eLr=lQmoG-I3o}|NW~8Mja7UJ{jDG^T? zr_w&9tns9|hFrirQ;M&0{_M-&JOmU@iDhJbn98;z%eHo<25l9UToeFVfJ8G#ri*8N zoY3xDWRL{04p(OmqmzLst$n}xro#9R{i z)Kak)55i70oMfM-fcFN$c&IvJqBz-g1&&bDy<)e_80}>hw8UL|Ua+lCEFM`o693Jd cXTAbrTmG=$p8LgT determineLineBreaks(const TaggedString& logicalInput, // often appear without surrounding spaces. if (i < logicalInput.length() - 1) { const bool allowsIdeographicBreak = util::i18n::allowsIdeographicBreaking(codePoint); - if (section.imageID || allowsIdeographicBreak || util::i18n::allowsWordBreaking(codePoint)) { + const bool allowsBreakBeforeNext = i != logicalInput.length() - 2 && + util::i18n::allowsWordBreakingBefore(logicalInput.getCharCodeAt(i + 1)); + if (section.imageID || allowsIdeographicBreak || util::i18n::allowsWordBreaking(codePoint) || + allowsBreakBeforeNext) { const bool penalizableIdeographicBreak = allowsIdeographicBreak && hasServerSuggestedBreaks; const std::size_t nextIndex = i + 1; potentialBreaks.push_back(evaluateBreak( diff --git a/src/mln/util/i18n.cpp b/src/mln/util/i18n.cpp index 7bbbfef7f..1d06f3954 100644 --- a/src/mln/util/i18n.cpp +++ b/src/mln/util/i18n.cpp @@ -352,7 +352,6 @@ bool allowsWordBreaking(char16_t chr) { return (chr == 0x0a /* newline */ || chr == 0x20 /* space */ || chr == 0x26 /* ampersand */ - || chr == 0x28 /* open parenthesis */ || chr == 0x29 /* close parenthesis */ || chr == 0x2b /* plus sign */ || chr == 0x2d /* hyphen-minus */ @@ -365,6 +364,10 @@ bool allowsWordBreaking(char16_t chr) { ); } +bool allowsWordBreakingBefore(char16_t chr) { + return chr == 0x28; /* open parenthesis */ +} + bool charAllowsLetterSpacing(char16_t chr) { return !(isInArabic(chr) || isInArabicSupplement(chr) || isInArabicExtendedA(chr) || isInArabicPresentationFormsA(chr) || isInArabicPresentationFormsB(chr)); diff --git a/src/mln/util/i18n.hpp b/src/mln/util/i18n.hpp index e2d633adc..9f306ef8e 100644 --- a/src/mln/util/i18n.hpp +++ b/src/mln/util/i18n.hpp @@ -10,6 +10,10 @@ namespace i18n { by the given Unicode codepoint due to word breaking. */ bool allowsWordBreaking(char16_t chr); +/** Returns whether a line break can be inserted before the character indicated + by the given Unicode codepoint due to word breaking. */ +bool allowsWordBreakingBefore(char16_t chr); + /** Returns whether the given string can be displayed with letter-spacing. False for Arabic scripts, where letter-spacing will break ligatures. */ bool allowsLetterSpacing(const std::u16string& string); diff --git a/test/text/shaping.test.cpp b/test/text/shaping.test.cpp index 09c06e508..cd0e09605 100644 --- a/test/text/shaping.test.cpp +++ b/test/text/shaping.test.cpp @@ -148,6 +148,58 @@ void testApplyTextFit(const Rect& rectangle, ASSERT_EQ(icon.bottom(), expectedBottom); } +TEST(Shaping, BreakBeforeLeftParenthesis) { + GlyphPosition glyphPosition; + glyphPosition.metrics.width = 18; + glyphPosition.metrics.height = 18; + glyphPosition.metrics.left = 2; + glyphPosition.metrics.top = -8; + glyphPosition.metrics.advance = 21; + + BiDi bidi; + const std::vector fontStack{{"font-stack"}}; + const SectionOptions sectionOptions(1.0f, fontStack, GlyphIDType::FontPBF, 0); + GlyphMap glyphs; + GlyphPositions glyphPositions; + for (const char16_t id : {u'o', u'a', u'(', u')', u' '}) { + Glyph glyph; + glyph.id = id; + glyph.metrics = glyphPosition.metrics; + glyphs[FontStackHasher()(fontStack)].emplace(id, Immutable(makeMutable(std::move(glyph)))); + glyphPositions[FontStackHasher()(fontStack)].emplace(id, glyphPosition); + } + ImagePositions imagePositions; + + const auto glyphsPerLine = [&](const std::u16string& text) { + const Shaping shaping = getShaping(TaggedString(text, sectionOptions), + 5 * ONE_EM, + ONE_EM, // lineHeight + style::SymbolAnchorType::Center, + style::TextJustifyType::Center, + 0, // spacing + {{0.0f, 0.0f}}, // translate + WritingModeType::Horizontal, + bidi, + glyphs, + glyphPositions, + imagePositions, + 16.0f, // layoutTextSize + 16.0f, // layoutTextSizeAtBucketZoomLevel + /*allowVerticalPlacement*/ false); + std::vector counts; + for (const auto& line : shaping.positionedLines) { + counts.push_back(line.positionedGlyphs.size()); + } + return counts; + }; + + // The break goes before the parenthesis, so it opens the second line instead of closing the first. + EXPECT_EQ(glyphsPerLine(u"oooooooo(aaaaaaaaa)"), (std::vector{8, 11})); + EXPECT_EQ(glyphsPerLine(u"oooooooo (aaaaaaaaa)"), (std::vector{8, 11})); + // A trailing parenthesis never becomes a line of its own. + EXPECT_EQ(glyphsPerLine(u"oooooooo("), (std::vector{9})); +} + TEST(Shaping, applyTextFit) { float textSize = 4; float fontScale = 4;