Reordering fails when GDEF table is absent

[simoncozens]

(See this typedrawers thread)

I can see there's code in the USE shaper to note down which glyphs are involved in pref substitutions and then reorder them, but I can't get it to fire. I created a font by adding this feature code:

feature pref {
    script java;
    sub uniA9BF by glyph01;
} pref;

to my Fallback Plus font, and tried to reorder using the pref feature, but this was the output:

$ hb-shape Fallback-Javanese.otf -u 'A995 A9BF'
[uniA995=0+600|glyph01=0+600]

I would have expected glyph 01 uniA995. Adding some instrumentation (OK, print statements) into _set_glyph_props and record_pref_use I got this:

$ ./util/hb-shape --script='java' --shapers=ot ~/hacks/typography/test-fonts/Fallback-Javanese.otf -u 'A995 A9BF'
Choosing...
Choose USE.
Setting glyph props
Checking pref use...
Checking glyph 591 (2)...
Checking glyph 101 (2)...
[uniA995=0+600|glyph01=0+600]

So when the substitution is done, the HB_OT_LAYOUT_GLYPH_PROPS_SUBSTITUTED is added to the second glyph, but by the time record_pref_use is called, the flag has been cleared.

I feel like this is a bug, but I don't have enough USE experience to be confident.

[punchcutter]

I downloaded that font and there's no GSUB in it. Which means there's also no script/lang tags to trigger USE shaping.

[simoncozens]

There’s no features in that font. I added the feature above to it.

[simoncozens]

Font with added feature:
Test-Javanese.zip

[punchcutter]

That zip gives me a 0kb file, but I added the feature to the other font and everything works fine here.
hb-shape (HarfBuzz) 2.6.4
[glyph01=0+600|uniA995=0+600]

[simoncozens]

I’m glad it works for you! I wonder why it doesn’t for me... (Using harfbuzz HEAD.)

[punchcutter]

It works for me with HEAD. Here's the font where I added the feature and script/lang tag.
FallbackPlus-Regular.otf.zip

[punchcutter]

We've done the same thing with other scripts and it's always been working fine. I can't tell why your font might not work because that zip file doesn't open for me. Can you try to attach it again?

[simoncozens]

Thanks for having another look. Here's the zip file again. Looking at the differences between your font and mine, it may be that the lack of GDEF table is part of the problem.

Test-Javanese.zip

[punchcutter]

Yes, seems like missing GDEF is a problem. I added GDEF just for these two glyphs and then it seems to work.

[simoncozens]

OK. That seems buggy - this behaviour shouldn't depend on the GDEF table AIUI.

If I'm reading hb_ot_layout_glyph_props_flags_t right, Harfbuzz is stashing internal information in a GDEF-related structure. I don't know how that works when the GDEF table isn't present, but from this example I think the answer is "not quite right".

[simoncozens]

As a further data point, this is different behaviour to CoreText:

simon@upo ~/hacks/typography/test-fonts .*$ hb-shape --shaper=coretext Fallback-Javanese.otf -u 'A995 A9BF'
[glyph01=0+600|uniA995=0+600]
simon@upo ~/hacks/typography/test-fonts .*$ hb-shape --shaper=ot Fallback-Javanese.otf -u 'A995 A9BF'
[uniA995=0+600|glyph01=0+600]

[punchcutter]

In this particular case the GDEF shouldn’t matter, but as a general rule I expect a GDEF for correct GPOS/GSUB because those both rely on glyph classes, and/or mark attachment or mark filter classes to function properly. It’s only a simple example like this where the default lookup flags (don’t ignore marks, etc) won’t matter, but any other flags rely on GDEF to know what to do so I don’t think it’s necessarily a bug to fail when no GDEF exists. CoreText might be more forgiving, but we can’t see what magic is inside to confirm.

[khaledhosny]

If I'm reading hb_ot_layout_glyph_props_flags_t right, Harfbuzz is stashing internal information in a GDEF-related structure. I don't know how that works when the GDEF table isn't present, but from this example I think the answer is "not quite right".

HarfBuzz synthesizes glyph classes when GDEF table is missing, so I don’t think this is why the reordering is not working when GDEF is missing.

[khaledhosny]

The root cause is this condition:

harfbuzz/src/hb-ot-layout-gsubgpos.hh

Lines 669 to 672 in 1b64b73

	if (likely (has_glyph_classes))
	_hb_glyph_info_set_glyph_props (&buffer->cur(), add_in | gdef.get_glyph_props (glyph_index));
	else if (class_guess)
	_hb_glyph_info_set_glyph_props (&buffer->cur(), add_in | class_guess);

has_glyph_classes is false, and class_guess is 0 (it is always 0 for single single substitutions). I’m not sure why the check for class_guess is needed. May be @behdad can comment.

[lianghai]

Forwarding my test files (the two OTF files only differ in the existence of an empty GDEF) from n8willis/opentype-shaping-documents#94:

DummyKnda.zip

~ % hb-shape DummyKnda-Regular-without-GDEF.otf ರ್ಕ
[repha..Knda=0+0|ka..Knda=2+0]
~ % hb-shape DummyKnda-Regular-with-GDEF.otf ರ್ಕ 
[ka..Knda=0+0|repha..Knda=0+0]

[behdad]

has_glyph_classes is false, and class_guess is 0 (it is always 0 for single single substitutions). I’m not sure why the check for class_guess is needed. May be @behdad can comment.

If we don't have a guess (eg in a SingleSubst), we just retain whatever class previous glyph had. That's the only sensical thing to do.

[behdad]

Anyone wants to explain to me in simple terms, what's the observed "bug"?

Maybe pref lookup has lookupflags that make it not work when GDEF is synthesized. That's the kind of thing that can explain this.

In general, a complex-shaper font without GDEF is asking for trouble.

[khaledhosny]

Which is fine when the font has a GDEF table, but if not then gdef.get_glyph_props () will return empty props and previously synthesized props will be dropped.

[khaledhosny]

Ignore that. If no GDEF and no guess _hb_glyph_info_set_glyph_props will not be called.

[behdad]

Ignore that. If no GDEF and no guess _hb_glyph_info_set_glyph_props will not be called.

Exactly.

[khaledhosny]

Which has the effect of HB_OT_LAYOUT_GLYPH_PROPS_SUBSTITUTED not being set, which causes the reordering to not happen.

[behdad]

Which has the effect of HB_OT_LAYOUT_GLYPH_PROPS_SUBSTITUTED not being set, which causes the reordering to not happen.

Oooh. Is the add_in we are not adding. Yeah?

[khaledhosny]

Yes.

[behdad]

/me fixes that.

[simoncozens]

Just change the else if (class_guess) for an else.

[behdad]

No no. Is more. Am fixing.

[behdad]

Fixed. WOuld be great if someone can add a test for it.