-
Notifications
You must be signed in to change notification settings - Fork 16
VF-first feature writers #30
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.
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
Conversation
Adapt tests for new code
|
@frankrolf is it time to merge this? |
|
Some more patience needed. :-) |
|
I am a bit unhappy about the “downgrading” of many pairs to “normal”, while they are clearly exceptions. I compared the old and new kern features for https://github.com/adobe-fonts/source-serif/tree/main/Roman/Instances/Text/Regular. While the kerndumps for both .fea files are equivalent, I wonder if we could reinstate the current behavior. |
|
Broadly speaking the reason for the reduced list of exceptions is that the new code isn't judging the cases to be exceptions. That's not your assessment, so let's talk about one: iotadieresistonos lambda from these files. iotadieresistonos occurs in @MMK_L_GRK_alpha and @MMK_R_GRK_iotadieresis. lambda is in no classes. These are the occurrences of those two classes: lambda only occurs once here in the fifth line. But the entries are reversed compared with the entry in question. Unless ordered pairs and their opposite pairs interact, there's no reason for iotadieresistonos lambda to be an exception. So the questions are:
|
|
Oh, maybe I misunderstood what you were posting. Let me dig more into this. |
|
I just ran the new code and only saw the two differences. I'm not sure what 7 pairs you're referring to. |
|
Sorry, let me clarify: When I run the current (static) code, there are two differences -- pairs sorted from glyph-glyph to exceptions. |
|
I'm still not quite following this thread. You mention kern_ss4_exceptions.ufo -- is that linked somewhere? |
There may just be an aspect of the overall system that I'm not comprehending. What specifically does it mean that "iota is grouped with iotadieresistonos"? I know that in the files generated from Text/Regular/font.ufo they're both in the @MMK_L_GRK_alpha group but you seem to mean something more fundamental than that. What other grouping is there and how is it represented/implemented? Is it in some other feature file? |
|
kern_ss4_exceptions.ufo is the test case you modified the expected output for. I think this modification is not desirable. The file is in this repo, at tests/kern_ss4_exceptions.ufo |
|
OK, looking at the kern_ss4_exceptions test case there is a single difference, in that These are the kerning statements involving lambda: Other than its kerning with itself, its only the first element in the pair except for the line in question. Presumably the worry is the last line, since that kerns lambda with iotadieresistonos, but there its kerned in the other order. Why would that line make |
|
Maybe it’ll help to focus on how kerning is usually authored, rather than this output (which already is an abstraction). Let’s take our lambda, iota, iotadieresistonos example again: In any kerning tool I’ve used, group kerning is the normal interaction – if a group for the given glyph exists. Any kerning that involves a special decision for one member of the group has to be created as an exception. In our case, this means:
(for visuals, see #30 (comment)) This means that a font with only a single kerning pair can still have a kerning exception :-) I understand that it may not make any functional difference where that pair “lives” in the kern feature hierarchy (in the one-pair case, especially) – but I hope it’s clear why I don’t want a choice the user made to become “invisible” (while other pairs, which necessarily have to be among the exceptions aren’t). |
|
OK, I'll think about this. |
|
Are you saying that:
That's all there is to it? |
|
Alright, I pushed a commit that should do what I think you're describing. |
|
Thank you! |
|
I will close this PR since I made a few changes to main since it has been opened (sorry about that). |



The existing tests should pass. I had to do a bit of restructuring in the kern tester.
The new code classes some cases as "normal" when the old code classes them as "exceptions", so two of the test outputs have changed. Take a look and see if you agree with the new classification.