Sync with 2.8.2 - Githubissues

bluebear94 commented 9 months ago

Troublesome commits so far:

[X] harfbuzz/harfbuzz@4f1e8d2, mostly because the upstream code is structured differently from ours

[X] harfbuzz/harfbuzz@5585ea0 – the in_house::cluster_004 test fails:

thread 'in_house::cluster_004' panicked at tests/shaping/in_house.rs:532:5:
assertion `left == right` failed
left: "uni0D4E=0+0|uni25CC=0+418|uni0D4D=1+0|space=2+0"
right: "uni0D4E=0+0|uni25CC=0+418|uni0D4D=0+0|space=0+0"

~~Could this be another rounding error?~~ Not likely, the things that are wrong are the cluster indices.

bluebear94 commented 9 months ago

It seems that the if (indic_plan->is_old_spec || end - start > 127) branch in initial_reordering_consonant_syllable is taken by HarfBuzz in the cluster_004 test, but the equivalent branch in RustyBuzz is not.

bluebear94 commented 9 months ago

In HB’s data_create_indic, plan->map.chosen_script[0] turns out to be 0xFFFF. Why this value, I don’t know, but changing line 479 of our complex/indic.rs to

.map_or(true, |tag| tag.to_bytes()[3] != b'2');

makes cluster_004 pass.

RazrFalcon commented 9 months ago

In HB’s data_create_indic, plan->map.chosen_script[0] turns out to be 0xFFFF.

0xFFFF is a special value in GSUB/GPOS. Maybe this.

RazrFalcon commented 9 months ago

Also:

/**
 * HB_OT_LAYOUT_NO_SCRIPT_INDEX:
 *
 * Special value for script index indicating unsupported script.
 */
#define HB_OT_LAYOUT_NO_SCRIPT_INDEX        0xFFFFu

RazrFalcon commented 9 months ago

In rustybuzz we use None instead of 0xFFFF. Which leads to this confusion. But also, I have no idea what (plan->map.chosen_script[0] & 0x000000FFu) != '2' suppose to check.

RazrFalcon commented 9 months ago

@behdad Would you mind clarifying this line of code? I know it's 11 years old, but still.

(plan->map.get_chosen_script (0) & 0x000000FF) != '2'

https://github.com/harfbuzz/harfbuzz/blob/f3584d3a3a627e38dfd7769975a670db340d2a48/src/hb-ot-shape-complex-indic.cc#L332

Why we're checking the last byte and why it must not be '2'/0x32?

RazrFalcon commented 9 months ago

@bluebear94 https://github.com/harfbuzz/harfbuzz/commit/855a3f478eea5b770e64611d09fd347336c56b67 doesn't affect us, right?

Or you haven't finished 2.8.2 yet?

bluebear94 commented 9 months ago

@bluebear94 harfbuzz/harfbuzz@855a3f4 doesn't affect us, right?

I don’t believe it does.

Or you haven't finished 2.8.2 yet?

No, I still have some commits to go through.

bluebear94 commented 9 months ago

I think these cover all the relevant changes.

RazrFalcon commented 9 months ago

All good. Thanks!

RazrFalcon commented 9 months ago

5257 commits left...

khaledhosny commented 9 months ago

@behdad Would you mind clarifying this line of code? I know it's 11 years old, but still.
(plan->map.get_chosen_script (0) & 0x000000FF) != '2'
https://github.com/harfbuzz/harfbuzz/blob/f3584d3a3a627e38dfd7769975a670db340d2a48/src/hb-ot-shape-complex-indic.cc#L332

Why we're checking the last byte and why it must not be '2'/0x32?

This is checking for old Indic script tags (deva vs dev2) the old tags trigger an old (and obsolete) processing model for backward compatibility with fonts designed against the old OpenType Indic spec.

RazrFalcon commented 9 months ago

Thanks. Not it makes sense.

behdad commented 9 months ago

(plan->map.get_chosen_script (0) & 0x000000FF) != '2'

The Indic scripts have two tags. Old and new. The old tag for Devanagari is deva whereas the new tag is dev2. All the new tags end with 2.

RazrFalcon commented 9 months ago

@behdad Thanks!

RazrFalcon / rustybuzz

Sync with 2.8.2 #82