Skip to content

Wrong feature file generated when there's pair positioning in contextual lookups #5383

@nadalaba

Description

@nadalaba

When reporting a bug/issue:

  • Screenshot
  • The FontForge version and the operating system you're using
  • The behavior you expect to see, and the actual behavior
  • Steps to reproduce the behavior
  • (optional) Possible solution/fix/workaround

Describing the problem:

When Generating feature files for a font that has pair positioning by glyphs (not by coverage or by class) in a contextual lookup rule, the value record of the first glyph (where the kerning data is stored) gets discarded, leaving only the value record of the second glyph (which is most of the time zero).
This problem affects the UFO generated with FontForge, too.

FontForge Version: 20230101 a1dad3e Built: 2023-01-01 05:31 UTC-ML-TtfDb-D-GDK3
OS: Windows 10 Pro 22H2

Steps to reproduce:

  1. Start FontForge and open a new font.
  2. Create 2 dummy glyphs (I created A and B).
    1
  3. Create 2 GPOS lookups (a pair positioning 'pair1' that is not linked to any feature and kerns the pair A B, and a contextual chaining position associated with 'kern' feature, that has the following rule A | A @<pair1> B |, which will kern the pair A B closer whenever it comes after A.
    2
    2 5
  4. Save the newly created lookups, and then try them in a metrics window, to ensure that they are correctly created (We can see that the sequence "AAB" causes the pair "AB" to kern closer as expected).
    3
  5. "Save Feature File..." and inspect its contents:

...
lookup kernHorizontalKerninginLatinlookup1 {
lookupflag 0;
pos \A \A' \B'<0 0 0 0> ;
} kernHorizontalKerninginLatinlookup1;
...

  1. We can see that the rule discarded the value record of the first glyph, which holds the kerning data (-532 in that example). Furthermore, trying to "Merge Feature Info..." of our exported feature file would cause an error parsing contextual sequence.

Expected behavior:

  1. The previously quoted contextual chaining position lookup in the feature file should have the rule:
    pos \A \A'<0 0 -532 0> \B'<0 0 0 0>;, or
    pos \A \A' -532 \B;

Possible solution:

  • The issue is related to dump_contextpstglyphs() in featurefile.c, where next_start always starts with '\0' because of the line *pt = '\0'; earlier, so the condition if ( *next_start!='\0' ) is always wrong and we will never get pst from pst_from_pos_pair_lookup() on pos == 0, so dump_valuerecord() only gets called on pos == 1.
  • This can be solved by reverting pt to its original value, just until next_start points to what's after the spaces after the while loop, and then nuling it again, so that start will end on '\0'.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions