Skip to content

Line break refactoring in preparation for bidirectional text support. - #3725

Merged
ChrisLoer merged 1 commit into
masterfrom
cloer_linebreak_refactor
Dec 5, 2016
Merged

ChrisLoer merged 1 commit into
masterfrom
cloer_linebreak_refactor

Conversation

@ChrisLoer

@ChrisLoer ChrisLoer commented Nov 30, 2016 •

Copy link
Copy Markdown
Contributor

Separates line-breaking logic from glyph placement. Introduces a few small behavior changes:

  • 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.

shaping.test.js updated to reflect invisible behavior changes (e.g. invisible glyphs no longer added to shaping).

cc @lucaswoj @jfirebaugh @mollymerp @1ec5

@ChrisLoer ChrisLoer added GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform refactoring 🏗️ labels Nov 30, 2016
@ChrisLoer ChrisLoer self-assigned this Nov 30, 2016
@ChrisLoer
ChrisLoer force-pushed the cloer_linebreak_refactor branch from 31cbcab to 2bad8cc Compare November 30, 2016 23:25
@nickidlugash

Copy link
Copy Markdown

@ChrisLoer is this going to be incorporated into this PR? #3658 (comment)

Solving the line-breaking issue with () is currently blocking an urgent project. Please let us know if this will be added. Thanks!! /cc @andreasviglakis

@ChrisLoer
ChrisLoer force-pushed the cloer_linebreak_refactor branch 2 times, most recently from 401abe8 to 5e10967 Compare December 1, 2016 04:16
@ChrisLoer

Copy link
Copy Markdown
Contributor Author

@nickidlugash As you mentioned in #3658 (comment), the parentheses issue will probably go beyond the scope of this PR, although I'm happy to start working on a separate PR for that issue.

Comment thread js/symbol/shaping.js Outdated
lineBreakPoints.forEach(lineBreak => {
lines.push(text.substring(start, lineBreak));
start = lineBreak;
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a for (const lineBreak of lineBreakPoints) { ... } loop here instead of forEach.

@ChrisLoer
ChrisLoer force-pushed the cloer_linebreak_refactor branch 2 times, most recently from 427195f to 48818e7 Compare December 2, 2016 18:30
* 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.
@ChrisLoer
ChrisLoer force-pushed the cloer_linebreak_refactor branch from 48818e7 to 8408313 Compare December 5, 2016 18:09
@ChrisLoer
ChrisLoer merged commit 9c8f855 into master Dec 5, 2016
@jfirebaugh
jfirebaugh deleted the cloer_linebreak_refactor branch February 3, 2017 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform refactoring 🏗️

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants