Line break refactoring in preparation for bidirectional text support.

* Separates line-breaking logic from glyph placement * Adds U+2027 "Interpunct" to ideographic breaking character set * Trims whitespace from beginning and end of line before calculating width for alignment purposes. * Fixes a crash on labels that generated lines with a single whitespace glyph.
mapbox · Dec 2, 2016 · 427195f · 427195f
1 parent 5db58df
commit 427195f
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 113 deletions.
diff --git a/js/symbol/shaping.js b/js/symbol/shaping.js
@@ -38,38 +38,36 @@ function Shaping(positionedGlyphs, text, top, bottom, left, right, writingMode)
 
 const newLine = 0x0a;
 
-function shapeText(text, glyphs, maxWidth, lineHeight, horizontalAlign, verticalAlign, justify, spacing, translate, verticalHeight, writingMode) {
+function breakLines(text, lineBreakPoints) {
+    const lines = [];
+    let start = 0;
+    for (const point in lineBreakPoints) {
+        const lineBreak = lineBreakPoints[point];
+        lines.push(text.substring(start, lineBreak));
+        start = lineBreak;
+    }
+
+    if (start < text.length) {
+        lines.push(text.substring(start, text.length));
+    }
+    return lines;
+}
 
+function shapeText(text, glyphs, maxWidth, lineHeight, horizontalAlign, verticalAlign, justify, spacing, translate, verticalHeight, writingMode) {
     text = text.trim();
     if (writingMode === WritingMode.vertical) text = verticalizePunctuation(text);
 
     const positionedGlyphs = [];
     const shaping = new Shaping(positionedGlyphs, text, translate[1], translate[1], translate[0], translate[0], writingMode);
 
-    // the y offset *should* be part of the font metadata
-    const yOffset = -17;
-
-    let x = 0;
-
-    for (let i = 0; i < text.length; i++) {
-        const codePoint = text.charCodeAt(i);
-        const glyph = glyphs[codePoint];
+    const lines = (writingMode === WritingMode.horizontal && maxWidth) ?
+        breakLines(text, determineLineBreaks(text, spacing, maxWidth, glyphs)) :
+        [text];
 
-        if (!glyph && codePoint !== newLine) continue;
+    shapeLines(shaping, glyphs, lines, lineHeight, horizontalAlign, verticalAlign, justify, translate, writingMode, spacing, verticalHeight);
 
-        if (!scriptDetection.charHasUprightVerticalOrientation(codePoint) || writingMode === WritingMode.horizontal) {
-            positionedGlyphs.push(new PositionedGlyph(codePoint, x, yOffset, glyph, 0));
-            if (glyph) x += glyph.advance + spacing;
-
-        } else {
-            positionedGlyphs.push(new PositionedGlyph(codePoint, x, 0, glyph, -Math.PI / 2));
-            if (glyph) x += verticalHeight + spacing;
-        }
-    }
-
-    if (!positionedGlyphs.length) return false;
-
-    linewrap(shaping, glyphs, lineHeight, maxWidth, horizontalAlign, verticalAlign, justify, translate, scriptDetection.allowsIdeographicBreaking(text), writingMode);
+    if (!positionedGlyphs.length)
+        return false;
 
     return shaping;
 }
@@ -94,91 +92,142 @@ const breakable = {
 
 invisible[newLine] = breakable[newLine] = true;
 
-function linewrap(shaping, glyphs, lineHeight, maxWidth, horizontalAlign, verticalAlign, justify, translate, useBalancedIdeographicBreaking, writingMode) {
-    let lastSafeBreak = null;
-    let lengthBeforeCurrentLine = 0;
-    let lineStartIndex = 0;
-    let line = 0;
+function determineIdeographicLineWidth(logicalInput, spacing, maxWidth, glyphs) {
+    let totalWidth = 0;
+
+    // totalWidth doesn't include the last character for magical tuning reasons. This makes the
+    // algorithm a little more agressive about trying to fit the text into fewer lines, taking
+    // advantage of the tolerance for going a little over maxWidth
+    for (let i = 0; i < logicalInput.length - 1; i++) {
+        const glyph = glyphs[logicalInput.charCodeAt(i)];
+        if (!glyph)
+            continue;
+        totalWidth += glyph.advance + spacing;
+    }
 
-    let maxLineLength = 0;
+    const lineCount = Math.max(1, Math.ceil(totalWidth / maxWidth));
+    return totalWidth / lineCount;
+}
 
-    const positionedGlyphs = shaping.positionedGlyphs;
+function determineLineBreaks(logicalInput, spacing, maxWidth, glyphs) {
+    if (!maxWidth)
+        return [];
+
+    if (!logicalInput)
+        return [];
+
+    if (scriptDetection.allowsIdeographicBreaking(logicalInput))
+        maxWidth = determineIdeographicLineWidth(logicalInput, spacing, maxWidth, glyphs);
 
-    if (writingMode === WritingMode.horizontal && maxWidth) {
-        if (useBalancedIdeographicBreaking) {
-            const lastPositionedGlyph = positionedGlyphs[positionedGlyphs.length - 1];
-            const estimatedLineCount = Math.max(1, Math.ceil(lastPositionedGlyph.x / maxWidth));
-            maxWidth = lastPositionedGlyph.x / estimatedLineCount;
+    const lineBreakPoints = [];
+    let currentX = 0;
+    let lastSafeBreakIndex = 0;
+    let lastSafeBreakX = 0;
+
+    for (let i = 0; i < logicalInput.length; i++) {
+        const codePoint = logicalInput.charCodeAt(i);
+        const glyph = glyphs[codePoint];
+
+        // newlines treatment slightly different from gl-native. See: https://github.com/mapbox/mapbox-gl-native/issues/7253
+        if (!glyph && codePoint !== newLine)
+            continue;
+
+        // Ideographic characters, spaces, and word-breaking punctuation that often appear without
+        // surrounding spaces.
+        if (breakable[codePoint] ||
+            scriptDetection.charAllowsIdeographicBreaking(codePoint)) {
+            lastSafeBreakIndex = i;
+            lastSafeBreakX = currentX;
         }
 
-        for (let i = 0; i < positionedGlyphs.length; i++) {
-            const positionedGlyph = positionedGlyphs[i];
+        // Break at the last safe break if we're over maxWidth. Always break on newlines.
+        if ((currentX > maxWidth && lastSafeBreakIndex > 0) ||
+            codePoint === newLine) {
+            lineBreakPoints.push(lastSafeBreakIndex);
+            currentX -= lastSafeBreakX;
+            lastSafeBreakX = 0;
+        }
 
-            positionedGlyph.x -= lengthBeforeCurrentLine;
-            positionedGlyph.y += lineHeight * line;
+        if (glyph)
+            currentX += glyph.advance + spacing;
+    }
 
-            if (lastSafeBreak !== null && (positionedGlyph.x > maxWidth ||
-                    positionedGlyphs[lastSafeBreak].codePoint === newLine)) {
+    return lineBreakPoints;
+}
 
-                const lineLength = positionedGlyphs[lastSafeBreak + 1].x;
-                maxLineLength = Math.max(lineLength, maxLineLength);
+function shapeLines(shaping, glyphs, lines, lineHeight, horizontalAlign, verticalAlign, justify, translate, writingMode, spacing, verticalHeight) {
+    // the y offset *should* be part of the font metadata
+    const yOffset = -17;
 
-                for (let k = lastSafeBreak + 1; k <= i; k++) {
-                    positionedGlyphs[k].y += lineHeight;
-                    positionedGlyphs[k].x -= lineLength;
-                }
+    let x = 0;
+    let y = yOffset;
 
-                if (justify && lastSafeBreak > lineStartIndex) {
-                    // Collapse invisible characters.
-                    let lineEnd = lastSafeBreak;
-                    if (invisible[positionedGlyphs[lastSafeBreak].codePoint]) {
-                        lineEnd--;
-                    }
+    let maxLineLength = 0;
+    const positionedGlyphs = shaping.positionedGlyphs;
 
-                    justifyLine(positionedGlyphs, glyphs, lineStartIndex, lineEnd, justify);
-                }
+    for (const i in lines) {
+        const line = lines[i].trim();
 
-                lineStartIndex = lastSafeBreak + 1;
-                lastSafeBreak = null;
-                lengthBeforeCurrentLine += lineLength;
-                line++;
-            }
+        if (!line.length) {
+            y += lineHeight; // Still need a line feed after empty line
+            continue;
+        }
+
+        const lineStartIndex = positionedGlyphs.length;
+        for (let i = 0; i < line.length; i++) {
+            const codePoint = line.charCodeAt(i);
+            const glyph = glyphs[codePoint];
 
-            if (useBalancedIdeographicBreaking || breakable[positionedGlyph.codePoint] || scriptDetection.charAllowsIdeographicBreaking(positionedGlyph.codePoint)) {
-                lastSafeBreak = i;
+            if (!glyph) continue;
+
+            if (!scriptDetection.charHasUprightVerticalOrientation(codePoint) || writingMode === WritingMode.horizontal) {
+                positionedGlyphs.push(new PositionedGlyph(codePoint, x, y, glyph, 0));
+                x += glyph.advance + spacing;
+            } else {
+                positionedGlyphs.push(new PositionedGlyph(codePoint, x, 0, glyph, -Math.PI / 2));
+                x += verticalHeight + spacing;
             }
         }
-    }
 
-    const lastPositionedGlyph = positionedGlyphs[positionedGlyphs.length - 1];
-    const lastLineLength = lastPositionedGlyph.x + glyphs[lastPositionedGlyph.codePoint].advance;
-    maxLineLength = Math.max(maxLineLength, lastLineLength);
+        // Only justify if we placed at least one glyph
+        if (positionedGlyphs.length !== lineStartIndex) {
+            const lineLength = x - spacing;
+            maxLineLength = Math.max(lineLength, maxLineLength);
 
-    const height = (line + 1) * lineHeight;
+            justifyLine(positionedGlyphs, glyphs, lineStartIndex, positionedGlyphs.length - 1, justify);
+        }
 
-    justifyLine(positionedGlyphs, glyphs, lineStartIndex, positionedGlyphs.length - 1, justify);
-    align(positionedGlyphs, justify, horizontalAlign, verticalAlign, maxLineLength, lineHeight, line, translate);
+        x = 0;
+        y += lineHeight;
+    }
+
+    align(positionedGlyphs, justify, horizontalAlign, verticalAlign, maxLineLength, lineHeight, lines.length, translate);
 
     // Calculate the bounding box
+    const height = lines.length * lineHeight;
+
     shaping.top += -verticalAlign * height;
     shaping.bottom = shaping.top + height;
     shaping.left += -horizontalAlign * maxLineLength;
     shaping.right = shaping.left + maxLineLength;
 }
 
+// justify left = 0, right = 1, center = .5
 function justifyLine(positionedGlyphs, glyphs, start, end, justify) {
+    if (!justify)
+        return;
+
     const lastAdvance = glyphs[positionedGlyphs[end].codePoint].advance;
     const lineIndent = (positionedGlyphs[end].x + lastAdvance) * justify;
 
     for (let j = start; j <= end; j++) {
         positionedGlyphs[j].x -= lineIndent;
     }
-
 }
 
-function align(positionedGlyphs, justify, horizontalAlign, verticalAlign, maxLineLength, lineHeight, line, translate) {
+function align(positionedGlyphs, justify, horizontalAlign, verticalAlign, maxLineLength, lineHeight, lineCount, translate) {
     const shiftX = (justify - horizontalAlign) * maxLineLength + translate[0];
-    const shiftY = (-verticalAlign * (line + 1) + 0.5) * lineHeight + translate[1];
+    const shiftY = (-verticalAlign * lineCount + 0.5) * lineHeight + translate[1];
 
     for (let j = 0; j < positionedGlyphs.length; j++) {
         positionedGlyphs[j].x += shiftX;

diff --git a/js/util/script_detection.js b/js/util/script_detection.js
@@ -18,6 +18,10 @@ module.exports.allowsVerticalWritingMode = function(chars) {
 };
 
 module.exports.charAllowsIdeographicBreaking = function(char) {
+    // Allow U+2027 "Interpunct" for hyphenation of Chinese words
+    // See https://github.com/mapbox/mapbox-gl-js/issues/3658
+    if (char === 0x2027) return true;
+
     // Return early for characters outside all ideographic ranges.
     if (char < 0x2E80) return false;
 

diff --git a/package.json b/package.json
@@ -62,7 +62,7 @@
     "in-publish": "^2.0.0",
     "jsdom": "^9.4.2",
     "lodash.template": "^4.4.0",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#623745e501a90a12cb5c750f3f8276cdf5c88bc4",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#0a7f9677854ab207ed492e352af5bf7e65dc6ff6",
     "minifyify": "^7.0.1",
     "npm-run-all": "^3.0.0",
     "nyc": "^8.3.0",

diff --git a/test/expected/text-shaping-linebreak.json b/test/expected/text-shaping-linebreak.json
@@ -70,20 +70,6 @@
       },
       "angle": 0
     },
-    {
-      "codePoint": 32,
-      "x": 65,
-      "y": -29,
-      "glyph": {
-        "id": 32,
-        "width": 0,
-        "height": 0,
-        "left": 0,
-        "top": -26,
-        "advance": 6
-      },
-      "angle": 0
-    },
     {
       "codePoint": 97,
       "x": -32.5,
@@ -158,7 +144,7 @@
   "text": "abcde abcde",
   "top": -24,
   "bottom": 24,
-  "left": -35.5,
-  "right": 35.5,
+  "left": -32.5,
+  "right": 32.5,
   "writingMode": 1
-}
+}
diff --git a/test/expected/text-shaping-newline.json b/test/expected/text-shaping-newline.json
@@ -70,13 +70,6 @@
       },
       "angle": 0
     },
-    {
-      "codePoint": 10,
-      "x": 65,
-      "y": -29,
-      "glyph": null,
-      "angle": 0
-    },
     {
       "codePoint": 97,
       "x": -32.5,
@@ -154,4 +147,4 @@
   "left": -32.5,
   "right": 32.5,
   "writingMode": 1
-}
+}
diff --git a/test/expected/text-shaping-newlines-in-middle.json b/test/expected/text-shaping-newlines-in-middle.json
@@ -70,20 +70,6 @@
       },
       "angle": 0
     },
-    {
-      "codePoint": 10,
-      "x": 65,
-      "y": -41,
-      "glyph": null,
-      "angle": 0
-    },
-    {
-      "codePoint": 10,
-      "x": 0,
-      "y": -17,
-      "glyph": null,
-      "angle": 0
-    },
     {
       "codePoint": 97,
       "x": -32.5,
@@ -161,4 +147,4 @@
   "left": -32.5,
   "right": 32.5,
   "writingMode": 1
-}
+}