From 6541b024043f1ed2607c23a150f0fb1db52ab4c9 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Thu, 16 May 2019 10:48:06 -0700 Subject: [PATCH 1/3] Prefer breaking lines at a zero width space. Mapbox data sources use zero width spaces to suggest better break points for Japanese labels. This somewhat hacky approach avoids adding more complex line breaking logic to -gl-js. --- src/symbol/shaping.js | 5 + .../text-shaping-zero-width-space.json | 195 ++++++++++++++++++ test/unit/symbol/shaping.test.js | 9 + 3 files changed, 209 insertions(+) create mode 100644 test/expected/text-shaping-zero-width-space.json diff --git a/src/symbol/shaping.js b/src/symbol/shaping.js index 68f27369157..a9d0da59155 100644 --- a/src/symbol/shaping.js +++ b/src/symbol/shaping.js @@ -282,6 +282,11 @@ function calculatePenalty(codePoint: number, nextCodePoint: number) { if (codePoint === 0x0a) { penalty -= 10000; } + // Prioritize zero width spaces because the are used by mapbox as a way + // to mark ideal break points in Japanese text. + if (codePoint === 0x200b) { + penalty -= 50; + } // Penalize open parenthesis at end of line if (codePoint === 0x28 || codePoint === 0xff08) { penalty += 50; diff --git a/test/expected/text-shaping-zero-width-space.json b/test/expected/text-shaping-zero-width-space.json new file mode 100644 index 00000000000..a725d339e63 --- /dev/null +++ b/test/expected/text-shaping-zero-width-space.json @@ -0,0 +1,195 @@ +{ + "positionedGlyphs": [ + { + "glyph": 65, + "x": -52.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": -37.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 66, + "x": -31.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 67, + "x": -16.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": -1.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 68, + "x": 4.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 69, + "x": 21.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": 34.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 70, + "x": 40.5, + "y": -41, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 71, + "x": -51, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": -34, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 72, + "x": -28, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": -11, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 73, + "x": -5, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": 1, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 74, + "x": 7, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": 13, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 75, + "x": 19, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": 33, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 76, + "x": 39, + "y": -17, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 77, + "x": -22.5, + "y": 7, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 32, + "x": -1.5, + "y": 7, + "vertical": false, + "scale": 1, + "fontStack": "Test" + }, + { + "glyph": 78, + "x": 4.5, + "y": 7, + "vertical": false, + "scale": 1, + "fontStack": "Test" + } + ], + "text": "A B​C D​E F​G H I J K L​M N", + "top": -36, + "bottom": 36, + "left": -52.5, + "right": 52.5, + "writingMode": 1, + "lineCount": 3 +} \ No newline at end of file diff --git a/test/unit/symbol/shaping.test.js b/test/unit/symbol/shaping.test.js index 8dc110dd77b..41cc1d521d7 100644 --- a/test/unit/symbol/shaping.test.js +++ b/test/unit/symbol/shaping.test.js @@ -55,6 +55,15 @@ test('shaping', (t) => { if (UPDATE) fs.writeFileSync(path.join(__dirname, '/../../expected/text-shaping-newlines-in-middle.json'), JSON.stringify(shaped, null, 2)); t.deepEqual(shaped, expectedNewLinesInMiddle); + // Prefer zero width spaces when breaking lines. Zero width spaces are used by Mapbox data sources as a hint that + // a position is ideal for breaking. + const expectedZeroWidthSpaceBreak = JSON.parse(fs.readFileSync(path.join(__dirname, '/../../expected/text-shaping-zero-width-space.json'))); + + shaped = shaping.shapeText(Formatted.fromString('A B​C D​E F​G H I J K L​M N'), glyphs, fontStack, 5 * oneEm, oneEm, 'center', 'center', 0, [0, 0], WritingMode.horizontal); + if (UPDATE) fs.writeFileSync(path.join(__dirname, '/../../expected/text-shaping-zero-width-space.json'), JSON.stringify(shaped, null, 2)); + t.deepEqual(shaped, expectedZeroWidthSpaceBreak); + + // Null shaping. shaped = shaping.shapeText(Formatted.fromString(''), glyphs, fontStack, 15 * oneEm, oneEm, 'center', 'center', 0 * oneEm, [0, 0], WritingMode.horizontal); t.equal(false, shaped); From 832623498c8f6a1f4ca6b76abec235cec652359f Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Mon, 20 May 2019 17:06:30 -0400 Subject: [PATCH 2/3] slightly penalize ideographic breaking A negative penalty on zero width spaces was not enough to prioritize them. --- src/symbol/shaping.js | 36 ++--- .../text-shaping-zero-width-space.json | 134 ++++-------------- test/fixtures/fontstack-glyphs.json | 12 +- test/unit/symbol/shaping.test.js | 2 +- 4 files changed, 62 insertions(+), 122 deletions(-) diff --git a/src/symbol/shaping.js b/src/symbol/shaping.js index a9d0da59155..b493b1a58ab 100644 --- a/src/symbol/shaping.js +++ b/src/symbol/shaping.js @@ -276,17 +276,18 @@ function calculateBadness(lineWidth: number, return raggedness + Math.abs(penalty) * penalty; } -function calculatePenalty(codePoint: number, nextCodePoint: number) { +function calculatePenalty(codePoint: number, nextCodePoint: number, ideographicBreak: boolean) { let penalty = 0; // Force break on newline if (codePoint === 0x0a) { penalty -= 10000; } - // Prioritize zero width spaces because the are used by mapbox as a way - // to mark ideal break points in Japanese text. - if (codePoint === 0x200b) { - penalty -= 50; + // Penalize breaks between characters that allow ideographic breaking because + // they are less preferable than breaks at spaces (or zero width spaces). + if (ideographicBreak) { + penalty += 150; } + // Penalize open parenthesis at end of line if (codePoint === 0x28 || codePoint === 0xff08) { penalty += 50; @@ -371,18 +372,19 @@ function determineLineBreaks(logicalInput: TaggedString, // Ideographic characters, spaces, and word-breaking punctuation that often appear without // surrounding spaces. - if ((i < logicalInput.length() - 1) && - (breakable[codePoint] || - charAllowsIdeographicBreaking(codePoint))) { - - potentialLineBreaks.push( - evaluateBreak( - i + 1, - currentX, - targetWidth, - potentialLineBreaks, - calculatePenalty(codePoint, logicalInput.getCharCode(i + 1)), - false)); + if ((i < logicalInput.length() - 1)) { + const ideographicBreak = !breakable[codePoint] && charAllowsIdeographicBreaking(codePoint); + if (breakable[codePoint] || ideographicBreak) { + + potentialLineBreaks.push( + evaluateBreak( + i + 1, + currentX, + targetWidth, + potentialLineBreaks, + calculatePenalty(codePoint, logicalInput.getCharCode(i + 1), ideographicBreak), + false)); + } } } diff --git a/test/expected/text-shaping-zero-width-space.json b/test/expected/text-shaping-zero-width-space.json index a725d339e63..4145103857e 100644 --- a/test/expected/text-shaping-zero-width-space.json +++ b/test/expected/text-shaping-zero-width-space.json @@ -1,195 +1,123 @@ { "positionedGlyphs": [ { - "glyph": 65, - "x": -52.5, + "glyph": 19977, + "x": -63, "y": -41, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 32, - "x": -37.5, + "glyph": 19977, + "x": -42, "y": -41, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 66, - "x": -31.5, + "glyph": 19977, + "x": -21, "y": -41, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 67, - "x": -16.5, + "glyph": 19977, + "x": 0, "y": -41, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 32, - "x": -1.5, + "glyph": 19977, + "x": 21, "y": -41, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 68, - "x": 4.5, + "glyph": 19977, + "x": 42, "y": -41, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 69, - "x": 21.5, - "y": -41, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 32, - "x": 34.5, - "y": -41, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 70, - "x": 40.5, - "y": -41, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 71, - "x": -51, - "y": -17, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 32, - "x": -34, - "y": -17, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 72, - "x": -28, - "y": -17, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 32, - "x": -11, + "glyph": 19977, + "x": -63, "y": -17, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 73, - "x": -5, + "glyph": 19977, + "x": -42, "y": -17, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 32, - "x": 1, + "glyph": 19977, + "x": -21, "y": -17, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 74, - "x": 7, + "glyph": 19977, + "x": 0, "y": -17, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 32, - "x": 13, + "glyph": 19977, + "x": 21, "y": -17, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 75, - "x": 19, + "glyph": 19977, + "x": 42, "y": -17, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 32, - "x": 33, - "y": -17, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 76, - "x": 39, - "y": -17, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 77, - "x": -22.5, - "y": 7, - "vertical": false, - "scale": 1, - "fontStack": "Test" - }, - { - "glyph": 32, - "x": -1.5, + "glyph": 19977, + "x": -21, "y": 7, "vertical": false, "scale": 1, "fontStack": "Test" }, { - "glyph": 78, - "x": 4.5, + "glyph": 19977, + "x": 0, "y": 7, "vertical": false, "scale": 1, "fontStack": "Test" } ], - "text": "A B​C D​E F​G H I J K L​M N", + "text": "三三​三三​三三​三三三三三三​三三", "top": -36, "bottom": 36, - "left": -52.5, - "right": 52.5, + "left": -63, + "right": 63, "writingMode": 1, "lineCount": 3 } \ No newline at end of file diff --git a/test/fixtures/fontstack-glyphs.json b/test/fixtures/fontstack-glyphs.json index edfc6386a8b..7dbbcee7e49 100644 --- a/test/fixtures/fontstack-glyphs.json +++ b/test/fixtures/fontstack-glyphs.json @@ -1918,5 +1918,15 @@ "top": -5, "advance": 15 } + }, + "19977": { + "id": 19977, + "metrics": { + "width": 18, + "height": 18, + "left": 2, + "top": -8, + "advance": 21 + } } -} \ No newline at end of file +} diff --git a/test/unit/symbol/shaping.test.js b/test/unit/symbol/shaping.test.js index 41cc1d521d7..f27cd004731 100644 --- a/test/unit/symbol/shaping.test.js +++ b/test/unit/symbol/shaping.test.js @@ -59,7 +59,7 @@ test('shaping', (t) => { // a position is ideal for breaking. const expectedZeroWidthSpaceBreak = JSON.parse(fs.readFileSync(path.join(__dirname, '/../../expected/text-shaping-zero-width-space.json'))); - shaped = shaping.shapeText(Formatted.fromString('A B​C D​E F​G H I J K L​M N'), glyphs, fontStack, 5 * oneEm, oneEm, 'center', 'center', 0, [0, 0], WritingMode.horizontal); + shaped = shaping.shapeText(Formatted.fromString('三三\u200b三三\u200b三三\u200b三三三三三三\u200b三三'), glyphs, fontStack, 5 * oneEm, oneEm, 'center', 'center', 0, [0, 0], WritingMode.horizontal); if (UPDATE) fs.writeFileSync(path.join(__dirname, '/../../expected/text-shaping-zero-width-space.json'), JSON.stringify(shaped, null, 2)); t.deepEqual(shaped, expectedZeroWidthSpaceBreak); From d82ec8cc644d6b236c9a5f7683991460e046941c Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Tue, 21 May 2019 12:58:21 -0400 Subject: [PATCH 3/3] only penalize ideographic breaks if zero width spaces are in string --- src/symbol/shaping.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/symbol/shaping.js b/src/symbol/shaping.js index b493b1a58ab..0386a3feb88 100644 --- a/src/symbol/shaping.js +++ b/src/symbol/shaping.js @@ -276,7 +276,7 @@ function calculateBadness(lineWidth: number, return raggedness + Math.abs(penalty) * penalty; } -function calculatePenalty(codePoint: number, nextCodePoint: number, ideographicBreak: boolean) { +function calculatePenalty(codePoint: number, nextCodePoint: number, penalizableIdeographicBreak: boolean) { let penalty = 0; // Force break on newline if (codePoint === 0x0a) { @@ -284,7 +284,7 @@ function calculatePenalty(codePoint: number, nextCodePoint: number, ideographicB } // Penalize breaks between characters that allow ideographic breaking because // they are less preferable than breaks at spaces (or zero width spaces). - if (ideographicBreak) { + if (penalizableIdeographicBreak) { penalty += 150; } @@ -359,6 +359,8 @@ function determineLineBreaks(logicalInput: TaggedString, const potentialLineBreaks = []; const targetWidth = determineAverageLineWidth(logicalInput, spacing, maxWidth, glyphMap); + const hasServerSuggestedBreakpoints = logicalInput.text.indexOf("\u200b") >= 0; + let currentX = 0; for (let i = 0; i < logicalInput.length(); i++) { @@ -373,7 +375,7 @@ function determineLineBreaks(logicalInput: TaggedString, // Ideographic characters, spaces, and word-breaking punctuation that often appear without // surrounding spaces. if ((i < logicalInput.length() - 1)) { - const ideographicBreak = !breakable[codePoint] && charAllowsIdeographicBreaking(codePoint); + const ideographicBreak = charAllowsIdeographicBreaking(codePoint); if (breakable[codePoint] || ideographicBreak) { potentialLineBreaks.push( @@ -382,7 +384,7 @@ function determineLineBreaks(logicalInput: TaggedString, currentX, targetWidth, potentialLineBreaks, - calculatePenalty(codePoint, logicalInput.getCharCode(i + 1), ideographicBreak), + calculatePenalty(codePoint, logicalInput.getCharCode(i + 1), ideographicBreak && hasServerSuggestedBreakpoints), false)); } }