fix(core): break a label before a left parenthesis, not after it (#4553)

metrics/integration/render-tests/text-breaking/left-parenthesis/style.json

@@ -0,0 +1,43 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 128,
+      "height": 128
+    }
+  },
+  "bearing": 45,
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "Point",
+        "coordinates":
+          [
+            0,
+            0
+          ]
+      }
+    }
+  },
+  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
+  "layers": [
+    {
+      "id": "symbol",
+      "type": "symbol",
+      "source": "geojson",
+      "layout": {
+        "symbol-placement": "point",
+        "symbol-spacing": 20,
+        "text-allow-overlap": true,
+        "text-rotation-alignment": "auto",
+        "text-max-width": 5,
+        "text-field": "Freiburg(Bresigau) HBF",
+        "text-font": [
+          "Open Sans Semibold",
+          "Arial Unicode MS Bold"
+        ]
+      }
+    }
+  ]
+}

metrics/integration/render-tests/text-breaking/whitespace/style.json

@@ -0,0 +1,43 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 128,
+      "height": 128
+    }
+  },
+  "bearing": 45,
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "Point",
+        "coordinates":
+          [
+            0,
+            0
+          ]
+      }
+    }
+  },
+  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
+  "layers": [
+    {
+      "id": "symbol",
+      "type": "symbol",
+      "source": "geojson",
+      "layout": {
+        "symbol-placement": "point",
+        "symbol-spacing": 20,
+        "text-allow-overlap": true,
+        "text-rotation-alignment": "auto",
+        "text-max-width": 5,
+        "text-field": "Freiburg (Bresigau) HBF",
+        "text-font": [
+          "Open Sans Semibold",
+          "Arial Unicode MS Bold"
+        ]
+      }
+    }
+  ]
+}

src/mln/text/shaping.cpp

@@ -385,7 +385,10 @@ std::set<std::size_t> 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(

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));

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);

test/text/shaping.test.cpp

@@ -148,6 +148,58 @@ void testApplyTextFit(const Rect<uint16_t>& 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<std::string> 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<Glyph>(makeMutable<Glyph>(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<std::size_t> 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<std::size_t>{8, 11}));
+    EXPECT_EQ(glyphsPerLine(u"oooooooo (aaaaaaaaa)"), (std::vector<std::size_t>{8, 11}));
+    // A trailing parenthesis never becomes a line of its own.
+    EXPECT_EQ(glyphsPerLine(u"oooooooo("), (std::vector<std::size_t>{9}));
+}
+
 TEST(Shaping, applyTextFit) {
     float textSize = 4;
     float fontScale = 4;