4.4.1 next attempt

[LaurenzV]

Status	Commit message	HB
🟡	[indic-like] Move allocation of syllable() buffer var to shapers that use it	Link	apart from the one change in hb-ot-layout, the rest isn't relevant for us right?
🟡	[layout-cache] Cache lookahead	Link	can ignore, right?
🟡	[layout] Rename apply_recurse_func to specialization of dispatch_recurse_func	Link	do we have that?
🟡	[indic-like] Remove category duplication	Link	Unfortunately, it's not really possible to deduplicate like they did, because of shortcomings of ragel rust... I added comments to hint to the lack of deduplication, I hope it won't cause problems in the future, but there isn't much else we can do I think...
🟡	[ucd] Update	Link	relevant?
🟡	[indic] Disable vowel-constraints under uniscribe-bug-compatible	Link	seems like we don't have this flag?
🟡	[indic] Clear syllables at the end of GSUB	Link	Seems like we don't have the clear_syllables_function... but it does seem relevant and it's also used in two other places, do you know what the rust equivalent would look like? I'm having a hard time understanding the C++ macro...
🟡	[indic] Remove remnants of Sinhala	Link	hb-ot-shaper contains a change in the end that is alsoa bout the "uniscribe compatibility bug"
🟡	[gdef] Minor harmless use of HB_OT_LAYOUT_GLYPH_CLASS_UNCLASSIFIED	Link	how do I translate this? or is that a ttf-parser change?

[LaurenzV]

@RazrFalcon arabic-fallback commits are not relevant for us, right?

[RazrFalcon]

Yep.

[LaurenzV]

@RazrFalcon not done yet, but getting close I think. There were however a couple of things I wasn't sure about, see above, would be great if you could take a look.

[LaurenzV]

Also, another question (which is not relevant for 4.4.1 but for later): The boring expansion spec is out of scope right now, right? i.e. I can ignore commits about >64k glyphs / avar2 / VARC?

[LaurenzV]

Alright, everything up to 4.4.1 is ported. So the only thing I need help with is with the orange points mentioned above.

I tried adding a CI check with no-default-features, but for some reason, it doesn't work... Am I understanding correctly that I always need to pass std/libm?

[RazrFalcon]

The boring expansion spec is out of scope right now, right? i.e. I can ignore commits about >64k glyphs / avar2 / VARC?

Yep.

Am I understanding correctly that I always need to pass std/libm?

It should be --no-default-features --features=libm.

So the only thing I need help with is with the orange points mentioned above.

Will take a look.

[RazrFalcon]

[indic-like] Move allocation of syllable() buffer var to shapers that use it | apart from the one change in hb-ot-layout, the rest isn't relevant for us right?

Yes, we do not use HB_BUFFER_ALLOCATE_VAR and friends. I still have no idea what they do in the first place. They do not affect shaping output.

[layout-cache] Cache lookahead | can ignore, right?

I think so. We do not cache those values to begin with.

[layout] Rename apply_recurse_func to specialization of dispatch_recurse_func | do we have that?

Probably not. Hard to say.

[indic-like] Remove category duplication

A strange one, indeed. But I guess it doesn't affect us much.

[ucd] Update | relevant?

We already have SCRIPT_MATH somehow... And we use unicode-script crate for low-level stuff anyway.

[indic] Disable vowel-constraints under uniscribe-bug-compatible | seems like we don't have this flag?

Yep, out of scope.

[indic] Clear syllables at the end of GSUB | Seems like we don't have the clear_syllables_function... but it does seem relevant and it's also used in two other places, do you know what the rust equivalent would look like? I'm having a hard time understanding the C++ macro...

Yes, we do not use HB_BUFFER_ALLOCATE_VAR and friends. See above.

[indic] Remove remnants of Sinhala | Link | hb-ot-shaper contains a change in the end that is alsoa bout the "uniscribe compatibility bug"

Yes, we do not care about uniscribe_bug_compatible.

[gdef] Minor harmless use of HB_OT_LAYOUT_GLYPH_CLASS_UNCLASSIFIED | how do I translate this? or is that a ttf-parser change?

This is ttf_parser::gdef::GlyphClass and in ttf-parser/rustybuzz HB_OT_LAYOUT_GLYPH_CLASS_UNCLASSIFIED would be None. This is how ttf_parser::gdef::Table::glyph_class works.

Also, that change in harfbuzz is no-op anyway, since HB_OT_LAYOUT_GLYPH_CLASS_UNCLASSIFIED is 0 to begin with. So it's just a magic number was replaced with a named one. No effect to rustybuzz.

---

Hopefully that's all.

[RazrFalcon]

To clarify HB_BUFFER_ALLOCATE_VAR and stuff a bit, the way I initially wrote rustybuzz is by forking harfbuzz, removing everything that doesn't affects shaping results; removed all magic macros and stuff; and only then started incrementally porting C++ to Rust.

And if I remember correctly, as it was 4 years ago, I removed HB_BUFFER_ALLOCATE_VAR macros. Run tests. All of them were still passing. Job done. Not second thoughs.

[LaurenzV]

Okay, thanks! I will still try to look into the clear_syllable thing since I do think it is relevant, but other than that I think it should be ready to merge.

[RazrFalcon]

Thanks again!

[LaurenzV]

@behdad Could you maybe clarify what hb_syllabic_clear_var does? Does it really just deallocate something or does it also change any values somehow?

[behdad]

The boring expansion spec is out of scope right now, right? i.e. I can ignore commits about >64k glyphs / avar2 / VARC?

avar2 is a bit different: It's shipped by Apple, and is enabled by default in HB. So I prefer if you implement it.

[behdad]

@behdad Could you maybe clarify what hb_syllabic_clear_var does? Does it really just deallocate something or does it also change any values somehow?

No it doesn't. The whole buffer variable allocation is just sanity checking to make sure the same buffer variable is not used for two different things at the same time. Some of the caching query whether a certain variable is free to use. But if you are not porting the layout caching things, you can ignore the buffer var allocations and just use them as long as they match the HB use patterns.

[LaurenzV]

Got it, thanks!

[LaurenzV]

Okay sorry, but I need to clarify again. Since we are not porting layout caching, should the following code snippet:

static void
override_features_indic (hb_ot_shape_planner_t *plan)
{
  plan->map.disable_feature (HB_TAG('l','i','g','a'));
  plan->map.add_gsub_pause (hb_syllabic_clear_var); // Don't need syllables anymore, use stop to free buffer var
}

be translated as (the way it currently is):

fn override_features(planner: &mut hb_ot_shape_planner_t) {
    planner
        .ot_map
        .disable_feature(hb_tag_t::from_bytes(b"liga"));
}

or should we still add the gsub pause, so something like:

fn override_features(planner: &mut hb_ot_shape_planner_t) {
    planner
        .ot_map
        .disable_feature(hb_tag_t::from_bytes(b"liga"));
   planner.ot_map.add_gsub_pause(None);
}

[behdad]

Add a None pause so behavior remains exactly the same as HB.