Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upUse byte indices instead of char indices for text runs #10895
Merged
Conversation
Shaper::save_glyph_results incorrectly starts its loop by setting glyph_span to a length of 1. This means that the `if glyph_span.len() == 0` test in the inner loop will never succeed. Instead the glyph span should start out empty, and a glyph should be added only as the corresponding char is found. For comparison, see the Gecko code this was ported from: https://hg.mozilla.org/mozilla-central/file/ab0044bf/gfx/thebes/gfxHarfBuzzShaper.cpp#l1682
This is a no-op, since a "non-glyph" is simply `GlyphEntry(0)`. This is the same as `GlyphEntry::initial()`, which all the entries are already initialized to.
highfive
commented
Apr 28, 2016
|
Heads up! This PR modifies the following files:
|
highfive
commented
Apr 28, 2016
|
|
|
@bors-servo try |
bors-servo
added a commit
that referenced
this pull request
Apr 28, 2016
Use byte indices instead of char indices for text runs Replace character indices with UTF-8 byte offsets throughout all code dealing with text runs. This eliminates a lot of complexity when converting from one to the other, and interoperates better with the rest of the Rust ecosystem. For most code this is just a simple replacement of char indices with byte indices. In a few places like glyph storage and text fragment scanning, it also lets us get rid of code that existed only to map between bytes and chars. Also includes some related fixes to text shaping, discovered while working on this conversion. See the commit messages for details. r? @pcwalton <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10895) <!-- Reviewable:end -->
|
|
highfive
commented
Apr 28, 2016
|
Fixed. (I accidentally removed an |
Replace character indices with UTF-8 byte offsets throughout the code dealing with text shaping and breaking. This eliminates a lot of complexity when converting from one to the other, and interoperates better with the rest of the Rust ecosystem.
|
@bors-servo: r+ This is awesome! |
|
|
1 similar comment
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 29, 2016
Use byte indices instead of char indices for text runs Replace character indices with UTF-8 byte offsets throughout all code dealing with text runs. This eliminates a lot of complexity when converting from one to the other, and interoperates better with the rest of the Rust ecosystem. For most code this is just a simple replacement of char indices with byte indices. In a few places like glyph storage and text fragment scanning, it also lets us get rid of code that existed only to map between bytes and chars. Also includes some related fixes to text shaping, discovered while working on this conversion. See the commit messages for details. r? @pcwalton <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10895) <!-- Reviewable:end -->
|
|
|
@bors-servo retry Infrastructure |
|
|
|
|
|
@bors-servo retry |
|
cc me |
bors-servo
added a commit
that referenced
this pull request
Apr 29, 2016
Use byte indices instead of char indices for text runs Replace character indices with UTF-8 byte offsets throughout all code dealing with text runs. This eliminates a lot of complexity when converting from one to the other, and interoperates better with the rest of the Rust ecosystem. For most code this is just a simple replacement of char indices with byte indices. In a few places like glyph storage and text fragment scanning, it also lets us get rid of code that existed only to map between bytes and chars. Also includes some related fixes to text shaping, discovered while working on this conversion. See the commit messages for details. r? @pcwalton <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10895) <!-- Reviewable:end -->
|
|
This was referenced Apr 29, 2016
This was referenced May 20, 2016
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.
mbrubeck commentedApr 28, 2016
•
edited by larsbergstrom
Replace character indices with UTF-8 byte offsets throughout all code dealing with text runs. This eliminates a lot of complexity when converting from one to the other, and interoperates better with the rest of the Rust ecosystem.
For most code this is just a simple replacement of char indices with byte indices. In a few places like glyph storage and text fragment scanning, it also lets us get rid of code that existed only to map between bytes and chars.
Also includes some related fixes to text shaping, discovered while working on this conversion. See the commit messages for details.
r? @pcwalton
This change is