Wrong rules generated in feature file when multiple lookups exist in contextual rule

[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

When you open an issue for a change/improvement/feature request:

• A description of the problem you're trying to solve, including why you think this is a problem
• If the feature changes current behavior, reasons why your solution is better
• (optional) Possible solution/fix/workaround

Describing the problem:

When Generating feature files for a font that has more than one lookup in one contextual rule, only the first lookup of that rule is written to the feature file, and all other lookups are discarded.
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 app and open a new font.
2. Create 4 dummy glyphs (I created A, B, C, and D).
   
3. Create 3 GSUB lookups (a single substitution 'Single 1' that replaces A by B, an other single substitution 'Single 2' that replaces C by D, and a contextual substitution By Glyphs that has the following rule: "A @<Single 1> C @<Single 2>", which will replace A by B and C by D if C camer after A.
   
   
4. Save the newly created lookups, and then try them in a metrics window, to ensure that it is correctly created (We can see that the sequence "ABACD" is getting substituted by "ABBDD" as expected.)
   
5. "Save Feature File..." and inspect its contents:

...
lookup caltContextualAlternatesinLatinlookup1 {
lookupflag 0;
sub \A'lookup Single1 \C' ;
} caltContextualAlternatesinLatinlookup1;
...

6. We can see that the rule omitted the second lookup (Single2). Furthermore, removing old lookups from fontforge completely, and then trying to "Merge Feature Info..." of our previously exported feature file would change our substitution rules, and would not produce the same expected result in the test of step 4.

Expected behavior:

1. The previously quoted contextual substitution lookup in the feature file should have the rule:

sub \A'lookup Single1  \C'lookup Single2 ;

Possible solution:

1. I think this problem is related to the lookup_in_rule() function, where the condition to check in the for loop should be seq > r->lookups[i].seq instead of seq < r->lookups[i].seq. Otherwise, the loop would never increment i.

[iorsh]

GW comments in lookup_in_rule():

    /* Suppose there are two lookups which affect the same sequence pos? */
    /* That's legal in otf, but I don't think it can be specified in the */
    /*  feature file. It doesn't seem likely to be used so I ignore it */

Could it be related to this case?

[NadAlaba]

GW comments in lookup_in_rule():

    /* Suppose there are two lookups which affect the same sequence pos? */
    /* That's legal in otf, but I don't think it can be specified in the */
    /*  feature file. It doesn't seem likely to be used so I ignore it */

Could it be related to this case?

No, not really. The comment is talking about having 2 lookups that affect the same sequence position, whereas the bug happens whenever there is more than one lookup in one rule, even if each lookup has a unique sequence position.

Looking at my example, the bug happened when the lookup Single 1 affected position 0 (the letter A), and lookup Single 2 affected position 1 (the letter C).