Skip to content

Conversation

@skef
Copy link

@skef skef commented Dec 5, 2024

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.

Adapt tests for new code
@skef
Copy link
Author

skef commented Mar 27, 2025

@frankrolf is it time to merge this?

@frankrolf
Copy link
Member

Some more patience needed. :-)

@frankrolf
Copy link
Member

I am a bit unhappy about the “downgrading” of many pairs to “normal”, while they are clearly exceptions.
What causes this?

I compared the old and new kern features for https://github.com/adobe-fonts/source-serif/tree/main/Roman/Instances/Text/Regular.
The UFO file contains 34 exception pairs, which (using the current writer) are represented as such in the “exception” area.
With the new writer, we have 7 pairs less classified as exceptions.

While the kerndumps for both .fea files are equivalent, I wonder if we could reinstate the current behavior.
Full disclosure – when using the current writer, the attached file comes out, which is slightly different from the file found in the repo – not sure why.

kern.fea.zip

@skef
Copy link
Author

skef commented Mar 28, 2025

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:

pos Chi @MMK_R_GRK_iotadieresis 10;
pos Kappa @MMK_R_GRK_iotadieresis 20;
pos Psi @MMK_R_GRK_iotadieresis 30;
pos chi @MMK_R_GRK_iotadieresis -10;
pos lambda @MMK_R_GRK_iotadieresis -10;
pos xi @MMK_R_GRK_iotadieresis 20;
pos zeta @MMK_R_GRK_iotadieresis 20;
pos @MMK_L_GRK_Alpha @MMK_R_GRK_iotadieresis -20;
pos @MMK_L_GRK_Gamma @MMK_R_GRK_iotadieresis 11;
pos @MMK_L_GRK_Upsilon @MMK_R_GRK_iotadieresis 30;
pos @MMK_L_GRK_epsilon @MMK_R_GRK_iotadieresis -10;
pos @MMK_L_GRK_alpha @MMK_R_GRK_Mu 20;
pos @MMK_L_GRK_alpha @MMK_R_four.lf -10;
pos @MMK_L_GRK_alpha @MMK_R_parenright 20;
pos @MMK_L_GRK_alpha @MMK_R_quoteleft 10;
pos @MMK_L_GRK_alpha @MMK_R_quoteright 10;
pos @MMK_L_GRK_alpha @MMK_R_registered 21;
pos @MMK_L_GRK_alpha @MMK_R_sups_corner 21;
pos @MMK_L_GRK_alpha @MMK_R_sups_desc 21;
pos @MMK_L_GRK_alpha @MMK_R_sups_diag 40;
pos @MMK_L_GRK_alpha @MMK_R_sups_figures 21;
pos @MMK_L_GRK_alpha @MMK_R_sups_j 41;
pos @MMK_L_GRK_alpha @MMK_R_sups_round 21;
pos @MMK_L_GRK_alpha Chi 10;
pos @MMK_L_GRK_alpha Tau 30;
pos @MMK_L_GRK_alpha Zeta 30;
pos @MMK_L_GRK_alpha anoteleia -20;
pos @MMK_L_GRK_alpha backslash 21;
pos @MMK_L_GRK_alpha beta -5;
pos @MMK_L_GRK_alpha tau -20;

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:

  1. For this case, is that not your understanding?
  2. Are there other cases where the new code makes a different judgement about exceptions but this analysis doesn't apply?

@skef
Copy link
Author

skef commented Mar 28, 2025

Oh, maybe I misunderstood what you were posting. Let me dig more into this.

@skef
Copy link
Author

skef commented Mar 28, 2025

I just ran the new code and only saw the two differences. I'm not sure what 7 pairs you're referring to.

@frankrolf
Copy link
Member

frankrolf commented Mar 28, 2025

Sorry, let me clarify:
We are talking about this file: https://github.com/adobe-fonts/source-serif/tree/main/Roman/Instances/Text/Regular/font.ufo
I am running

kernFeatureWriter -s source-serif/Roman/Instances/Text/Regular/font.ufo

When I run the current (static) code, there are two differences -- pairs sorted from glyph-glyph to exceptions.
I don’t know the reason for this, but the result seems right.
kern.fea_static.zip

When I run the new code (vffirst), I see this:
image

kern.fea_vffirst.zip

@frankrolf
Copy link
Member

Explaining my view based on the test case kern_ss4_exceptions.ufo:

  • the pair iota lambda is unkerned
  • lambda is in no group, but iota is grouped with iotadieresistonos
  • the pair iotadieresistonos lambda is kerned
  • therefore, iotadieresistonos lambda is an exception (-60) from the default value of iota lambda (None)
Screenshot 2025-03-28 at 09 33 32 Screenshot 2025-03-28 at 09 33 35

@skef
Copy link
Author

skef commented Mar 28, 2025

I'm still not quite following this thread. You mention kern_ss4_exceptions.ufo -- is that linked somewhere?

@skef
Copy link
Author

skef commented Mar 28, 2025

  • lambda is in no group, but iota is grouped with iotadieresistonos

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?

@frankrolf
Copy link
Member

frankrolf commented Mar 29, 2025

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

@skef
Copy link
Author

skef commented Mar 29, 2025

OK, looking at the kern_ss4_exceptions test case there is a single difference, in that pos iotadieresistonos lambda 60; moves from the exceptions list to the "normal" glyph, glyph list.

These are the kerning statements involving lambda:

pos iotadieresistonos lambda 60;
pos lambda lambda 5;
pos lambda @MMK_R_GRK_iota -10;
pos lambda @MMK_R_GRK_iotadieresis -10;

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 pos iotadieresistonos lambda 60; an exception? Or if that's not the line, what other line in the file makes it an exception?

@frankrolf
Copy link
Member

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:

  • iota, iotadieresistonos are grouped in one left-side group, @kern1.GRK_alpha (this is because alpha has the same “tail”)
  • iota lambda sets fine, so it does not need any kerning
  • iotadieresistonos lambda crashes, so I make an exception (note that this is an exception from “no kerning at all”)

(for visuals, see #30 (comment))

This means that a font with only a single kerning pair can still have a kerning exception :-)
Since all exceptions are a deliberate user decision, I’d like to see them represented as such.

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).

@skef
Copy link
Author

skef commented Mar 30, 2025

OK, I'll think about this.

@skef
Copy link
Author

skef commented Mar 30, 2025

Are you saying that:

  • A kerning pair of glyphs is a glyph, glyph exception if the left glyph is in a left group or the right glyph is in a right group
  • A kerning pair of group, glyph is a group_glyph_exception if the right glyph is in a right group?
  • A kerning pair of glyph, group is a glyph_group_exception if the left glyph is in a left group?

That's all there is to it?

@skef
Copy link
Author

skef commented Mar 30, 2025

Alright, I pushed a commit that should do what I think you're describing.

@frankrolf
Copy link
Member

Thank you!
I think we should update SHORTINSTNAMEKEY from com.adobe.shortInstanceName to com.adobe.type.shortInstanceName – this would make it consistent with glyphs.com.adobe.type.processedglyphs.

@frankrolf
Copy link
Member

I will close this PR since I made a few changes to main since it has been opened (sorry about that).
I cherry-picked all your commits into a new vffirst2 branch, let’s continue in #33.

@frankrolf frankrolf closed this Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants