Small line breaking improvements based on testing with Chinese data:

- Put "breakable" punctuation (such as a hyphen) on the line that starts the break, not the line after the break.
- Process all characters with the line breaking algorithm, even if we don't have glyphs for them. Some fonts have glyph-less breakable characters (we end up treating them similarly to a "zero-width space").
- Don't include trailing white space in raggedness calculations
- Make the "favor short final lines" rule more aggressive (unlike the other changes, this one is purely an aesthetic choice)

2 files changed, 24 insertions, 22 deletions

diff --git a/package.json b/package.json
index 59562f041f..835b5613d2 100644
--- a/package.json
+++ b/package.json
@@ -23,7 +23,7 @@
     "express": "^4.11.1",
     "lodash": "^4.16.4",
     "mapbox-gl-style-spec": "mapbox/mapbox-gl-style-spec#49e8b407bdbbe6f7c92dbcb56d3d51f425fc2653",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#f20a94e329e9af1abf42044a0dd1b96925cf629b",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#e714350e53da55be6ebbbee9e5a5512ae2b1630d",
     "mkdirp": "^0.5.1",
     "node-cmake": "^1.2.1",
     "pixelmatch": "^4.0.2",
diff --git a/src/mbgl/text/glyph_set.cpp b/src/mbgl/text/glyph_set.cpp
index 0040926103..f9a90f9bb0 100644
--- a/src/mbgl/text/glyph_set.cpp
+++ b/src/mbgl/text/glyph_set.cpp
@@ -117,9 +117,13 @@ float GlyphSet::determineAverageLineWidth(const std::u16string& logicalInput,
     
 float calculateBadness(const float lineWidth, const float targetWidth, const float penalty, const bool isLastBreak) {
     const float raggedness = std::pow(lineWidth - targetWidth, 2);
-    if (isLastBreak && lineWidth < targetWidth) {
-        // Be more tolerant of short final lines
-        return std::fmax(0, raggedness - 150);
+    if (isLastBreak) {
+        // Favor finals lines shorter than average over longer than average
+        if (lineWidth < targetWidth) {
+            return raggedness / 2;
+        } else {
+            return raggedness * 2;
+        }
     }
     if (penalty < 0) {
         return raggedness - std::pow(penalty, 2);
@@ -127,20 +131,22 @@ float calculateBadness(const float lineWidth, const float targetWidth, const flo
     return raggedness + std::pow(penalty, 2);
 }
     
-float calculatePenalty(char16_t codePoint, char16_t previousCodePoint) {
+float calculatePenalty(char16_t codePoint, char16_t nextCodePoint) {
     float penalty = 0;
     // Force break on newline
     if (codePoint == 0x0a) {
         penalty -= 10000;
     }
     // Penalize open parenthesis at end of line
-    if (previousCodePoint && (previousCodePoint == 0x28 || previousCodePoint == 0xff08)) {
+    if (codePoint == 0x28 || codePoint == 0xff08) {
         penalty += 50;
     }
+
     // Penalize close parenthesis at beginning of line
-    if (codePoint == 0x29 || codePoint == 0xff09) {
+    if (nextCodePoint == 0x29 || nextCodePoint == 0xff09) {
         penalty += 50;
     }
+    
     return penalty;
 }
     
@@ -164,16 +170,16 @@ PotentialBreak evaluateBreak(const std::size_t breakIndex, const float breakX, c
     
     const PotentialBreak* bestPriorBreak = nullptr;
     float bestBreakBadness = calculateBadness(breakX, targetWidth, penalty, isLastBreak);
-
     for (const auto& potentialBreak : potentialBreaks) {
         const float lineWidth = breakX - potentialBreak.x;
-        float breakBadness = calculateBadness(lineWidth, targetWidth, penalty, isLastBreak) + potentialBreak.badness;
+        float breakBadness =
+            calculateBadness(lineWidth, targetWidth, penalty, isLastBreak) + potentialBreak.badness;
         if (breakBadness <= bestBreakBadness) {
             bestPriorBreak = &potentialBreak;
             bestBreakBadness = breakBadness;
         }
     }
-    
+
     return PotentialBreak(breakIndex, breakX, bestPriorBreak, bestBreakBadness);
 }
     
@@ -200,7 +206,7 @@ std::set<std::size_t> GlyphSet::determineLineBreaks(const std::u16string& logica
     if (logicalInput.empty()) {
         return {};
     }
-    
+ 
     const float targetWidth = determineAverageLineWidth(logicalInput, spacing, maxWidth);
     
     std::list<PotentialBreak> potentialBreaks;
@@ -209,22 +215,18 @@ std::set<std::size_t> GlyphSet::determineLineBreaks(const std::u16string& logica
     for (std::size_t i = 0; i < logicalInput.size(); i++) {
         const char16_t codePoint = logicalInput[i];
         auto it = sdfs.find(codePoint);
-        if (it == sdfs.end()) {
-            continue;
+        if (it != sdfs.end() && !boost::algorithm::is_any_of(u" \t\n\v\f\r")(codePoint)) {
+            currentX += it->second.metrics.advance + spacing;
         }
-
+        
         // Ideographic characters, spaces, and word-breaking punctuation that often appear without
         // surrounding spaces.
-        if (util::i18n::allowsWordBreaking(codePoint) ||
-            util::i18n::allowsIdeographicBreaking(codePoint)) {
-            const char16_t previousCodePoint = i > 0 ? logicalInput[i-1] : 0;
-
-            potentialBreaks.push_back(evaluateBreak(i, currentX, targetWidth, potentialBreaks,
-                                                     calculatePenalty(codePoint, previousCodePoint),
+        if ((i < logicalInput.size() - 1) &&
+            (util::i18n::allowsWordBreaking(codePoint) || util::i18n::allowsIdeographicBreaking(codePoint))) {
+            potentialBreaks.push_back(evaluateBreak(i+1, currentX, targetWidth, potentialBreaks,
+                                                     calculatePenalty(codePoint, logicalInput[i+1]),
                                                      false));
         }
-
-        currentX += it->second.metrics.advance + spacing;
     }
 
     return leastBadBreaks(evaluateBreak(logicalInput.size(), currentX, targetWidth, potentialBreaks, 0, true));