Repository navigation
Don't draw the trailing space of wrapped right-to-left lines (#1422) - #1831
Open
ZahraaSadeqProd wants to merge 1 commit into
Open
ZahraaSadeqProd wants to merge 1 commit into
ZahraaSadeqProd wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1422
What kind of change does this PR introduce?
Bug fix.
When
featuresis set,EmbeddedFont.layoutshapes a line as one run, so fontkit reverses a right-to-left line as a whole. A wrapped line keeps its trailing space ('هذا نص '), and after the reversal that space is the first glyph drawn, on the left._fragmentcomputes thealign: 'right'offset without the trailing space (text.replace(/\s+$/, '')), so the visible glyphs end one space past the right margin. The last line has no trailing space and lines up, which is the "indent" in the issue._fragmentnow leaves the trailing whitespace of a wrapped line (options.width) out of the glyphs it draws. The justify path already does this withtext.trim(). Nothing that is measured changes: the alignment, the underline, strike-through and link rectangles, anddoc.xfor continued text are computed from the text as before. Left-to-right lines only lose an invisible space at their end, and no existing test or visual snapshot changes.With the issue's text in Noto Kufi Arabic at 16 pt,
width: 195,align: 'right',features: ['rtla']:هذا نص عربي طويل يحتويعلى عدة أسطر لإظهارالخطأ(last)Without
features,layoutshapes each word separately and Arabic words come out in left-to-right order. That's a separate issue (#1722 adds bidi for it) and not part of this change.Checklist:
tests/unit/text.spec.js"right-to-left wrapped lines do not draw their trailing space - issue #1422" checks that the first line draws 6 glyphs and doesn't start with the space. It fails before the change.tests/fonts/NotoKufiArabic-Subset.ttfis a 5 KB subset of Noto Kufi Arabic (Apache 2.0, from the fontkit test data) with the letters of the test text; none of the existing test fonts has Arabic or Hebrew glyphs.yarn test: 580 passed. eslint and prettier are clean.