Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Range selectors for ligatures does not work properly with aat fonts applying ligatures via morx table #4020

Closed
behdad opened this issue Jan 10, 2023 Discussed in #4019 · 6 comments · Fixed by #4023
Closed

Range selectors for ligatures does not work properly with aat fonts applying ligatures via morx table #4020

behdad opened this issue Jan 10, 2023 Discussed in #4019 · 6 comments · Fixed by #4023

Comments

@behdad
Copy link
Member

behdad commented Jan 10, 2023

Discussed in #4019

Originally posted by DeependraKhetwal January 9, 2023
Hi , I am facing an issue with aat fonts where on disabling ligatures for a particular range the ligatures gets disabled for the whole line. The issue does not happen with fonts that apply ligature substitutions via opentype 'liga' feature but which apply ligature substitutions via 'morx' table. In the attached example I am disabling ligatures for 'ffi' (using -liga[3:]) but ligature is getting turned off for 'ff' also.
Screenshot 2022-12-05 at 2 02 42 PM (1)
@behdad
Copy link
Member Author

behdad commented Jan 10, 2023

To implement this properly we need to treat runs with different features as independent runs for running the state machine, as the subtable flags might be different. That would be a significant change to our internal implementation.

I have a hacky implementation instead, which seems to work for simple cases. I'm going to push that instead.

@behdad behdad closed this as completed in 24a4d39 Jan 10, 2023
@behdad
Copy link
Member Author

behdad commented Jan 10, 2023

That broke tests. Reverting while I investigate whether it can be fixed.

behdad added a commit that referenced this issue Jan 10, 2023
@behdad
Revert "Revert "[aat] Allow disable feature ranges""
5202053 
This reverts commit 82b3e8a.

Another try.

Fixes #4020
@behdad
Copy link
Member Author

behdad commented Jan 10, 2023

Okay. "Fixed" it. I really don't like this hack, but I think it will do the job for now. It doesn't have any adverse effect if user doesn't specify any features. For fully specified features I might have broken things. To be determined.

@behdad
Copy link
Member Author

behdad commented Jan 10, 2023

Humm, actually my change probably breaks some fonts. I'm uncomfortable with it. Let me think more. Might revert it.

behdad added a commit that referenced this issue Jan 10, 2023
@behdad
[aat] Support feature ranges
1b7994c 
The hard way...

A bit uglier than I liked it to be, but is proper at least.

Fixes #4020
@behdad
Copy link
Member Author

behdad commented Jan 10, 2023

Okay. I fixed it properly. Please test @DeependraKhetwal.

@behdad
Copy link
Member Author

behdad commented Jan 10, 2023

Humm. It broke more things. Debugging still :(

@behdad behdad reopened this Jan 10, 2023
@behdad behdad mentioned this issue Jan 10, 2023
Merged
@khaledhosny khaledhosny linked a pull request Jan 10, 2023 that will close this issue
[aat] Support feature ranges #4023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[aat] Support feature ranges harfbuzz/harfbuzz
1 participant
@behdad