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 000000000..376691f5a Binary files /dev/null and b/metrics/integration/render-tests/text-breaking/left-parenthesis/expected.png differ diff --git a/metrics/integration/render-tests/text-breaking/left-parenthesis/style.json b/metrics/integration/render-tests/text-breaking/left-parenthesis/style.json new file mode 100644 index 000000000..c9ded9cc8 --- /dev/null +++ b/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" + ] + } + } + ] +} diff --git a/metrics/integration/render-tests/text-breaking/whitespace/expected.png b/metrics/integration/render-tests/text-breaking/whitespace/expected.png new file mode 100644 index 000000000..376691f5a Binary files /dev/null and b/metrics/integration/render-tests/text-breaking/whitespace/expected.png differ diff --git a/metrics/integration/render-tests/text-breaking/whitespace/style.json b/metrics/integration/render-tests/text-breaking/whitespace/style.json new file mode 100644 index 000000000..a96e63e91 --- /dev/null +++ b/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" + ] + } + } + ] +} diff --git a/src/mln/text/shaping.cpp b/src/mln/text/shaping.cpp index 2aa3193f4..6bad45c05 100644 --- a/src/mln/text/shaping.cpp +++ b/src/mln/text/shaping.cpp @@ -385,7 +385,10 @@ std::set 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;