Old kern writer: register lookups under DFLT, too #844
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
madig
changed the title
madig
force-pushed
the
upgrade-old-kern-writer
branch
from
5d9ec42
to
b072905
Compare
madig
force-pushed
the
old-kern-register-lookups-dflt
branch
from
54bbd2b
to
5f17e66
Compare
anthrotype
reviewed
May 21, 2024
anthrotype
reviewed
May 21, 2024
madig
force-pushed
the
upgrade-old-kern-writer
branch
from
b072905
to
0d15b13
Compare
madig
force-pushed
the
old-kern-register-lookups-dflt
branch
from
5f17e66
to
e36d9c1
Compare
anthrotype
reviewed
May 24, 2024
| @@ -694,6 +694,7 @@ def test_kern_LTR_and_RTL(self, FontClass): | |||
|
|
|||
| feature kern { | |||
| lookup kern_dflt; | |||
| lookup kern_ltr; | |||
There was a problem hiding this comment.
hm this looks suspicious..
the global languagesystems contain both LTR and RTL scripts in this particular test font (I see both latn and arab), if you register kern_ltr in the top part of the feature block before any explicit script/language declarations then it will be registered for all the globally defined languagesystems, but do we actually want to register the kern_ltr under arab?! Probably not. I don't know if it is benign or just plain wrong
There was a problem hiding this comment.
If kern_ltr includes kerning between common characters (e.g. punctuation or digits) it would be wrong to register it for arab, otherwise it should be redundant and tiny bit slow but otherwise harmless.
madig
force-pushed
the
old-kern-register-lookups-dflt
branch
from
e36d9c1
to
27af8ea
Compare
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 ports #787 to the old kern writer.
Contrary to the new writer, here we don't try to register languages for DFLT.