Fix font feature range #172
Merged
Conversation
This file contains 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
The documentation says that every syntax supported by harfbuzz is accepted, but the harfbuzz buffer raqm builds is always in UTF32, while raqm itself keeps track of UTF8 or UTF16 ranges. This converts the range from the other encoding to UTF32, like is done for every other range-based function.
therahedwig
force-pushed
the
fix-font-feature-range
branch
from
75bc5fa
to
7a36efc
Compare
|
Also rebased this one into a single commit, so it's easier to review. |
khaledhosny
reviewed
May 5, 2022
| if (fea.start != HB_FEATURE_GLOBAL_START) | ||
| { | ||
| if (rq->text_utf8) | ||
| { |
There was a problem hiding this comment.
The curly brackets should be dropped for one line body.
khaledhosny
reviewed
May 5, 2022
| { | ||
| if (rq->text_utf8) | ||
| { | ||
| fea.start = _raqm_u8_to_u32_index (rq, fea.start); |
There was a problem hiding this comment.
We probably need a helper function or a macro that encapsulates this logic, something like _rq_encoding_to_u32_index() that would check the encoding and do the mapping if necessary.
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.
This fixes #170, applies on to of #169 and #171
I was wrong, it's not per run, just an issue of the encoding being different between the text and the harfbuzz buffer, which makes this fix a lot simpler. This should mean that ranges used for font_feature won't need to be converted to UTF32 beforehand.