Broken results when setting variations

[MoSal]

Hello. I wasn't sure if I should report this considering the deprecation status of this crate. But anyway, here it goes..

---

Running the example with this monospace variable font:

First, test with wght=<default> and wdth=<default>:

% ./shape MartianMonoVF.ttf --variations=wght=400 ' 0123456789'
space=0+750|zero=1+750|one=2+750|two=3+750|three=4+750|four=5+750|five=6+750|six=7+750|seven=8+750|eight=9+750|nine=10+750

% ./shape MartianMonoVF.ttf --variations=wdth=112.5 ' 0123456789'
space=0+750|zero=1+750|one=2+750|two=3+750|three=4+750|four=5+750|five=6+750|six=7+750|seven=8+750|eight=9+750|nine=10+750

All good.

Now, let's variate:

% ./shape MartianMonoVF.ttf --variations=wdth=100 ' 0123456789'
space=0+0|zero=1+699|one=2+739|two=3+703|three=4+699|four=5+702|five=6+712|six=7+699|seven=8+686|eight=9+699|nine=10+699

% ./shape MartianMonoVF.ttf --variations=wght=300 ' 0123456789'
space=0+0|zero=1+749|one=2+789|two=3+755|three=4+749|four=5+745|five=6+768|six=7+749|seven=8+733|eight=9+749|nine=10+749

Not so mono anymore, and that space=0+0 is even more problematic.

harfbuzz reports correct results.

[RazrFalcon]

Interesting. Will take a look.

[LaurenzV]

Seems like there are two issues at play here:

1. The glyph_advance function currently looks as follows:

fn glyph_advance(&self, glyph: GlyphId, is_vertical: bool) -> u32 {
    let face = &self.ttfp_face;
    if face.is_variable()
        && face.has_non_default_variation_coordinates()
        && face.tables().hvar.is_none()
        && face.tables().vvar.is_none()
    {
        return match face.glyph_bounding_box(glyph) {
            Some(bbox) => {
                (if is_vertical {
                    bbox.y_max + bbox.y_min
                } else {
                    bbox.x_max + bbox.x_min
                }) as u32
            }
            None => 0,
        };
    }

In the case of this font, hvar doesn't exist, so we use the bounding box of the glyph to determine the advance... Which is 0 in case of the space character.

Seems the reason that this is done is that in ttf-parser, we ignore it if the hvar table doesn't exist:

pub fn glyph_hor_advance(&self, glyph_id: GlyphId) -> Option<u16> {
        #[cfg(feature = "variable-fonts")]
        {
            let mut advance = self.tables.hmtx?.advance(glyph_id)? as f32;

            if self.is_variable() {
                // Ignore variation offset when `hvar` is not set.
                if let Some(hvar) = self.tables.hvar {
                    if let Some(offset) = hvar.advance_offset(glyph_id, self.coords()) {
                        // We can't use `round()` in `no_std`, so this is the next best thing.
                        advance += offset + 0.5;
                    }
                }
            }

            u16::try_num_from(advance)
        }

        #[cfg(not(feature = "variable-fonts"))]
        {
            self.tables.hmtx?.advance(glyph_id)
        }
    }

So I presume using other tables has just not been implemented yet? @RazrFalcon since you seem to have written the code in ttf-parser, do you know what's up with that?

[LaurenzV]

harfbuzz seems to do something different here using something called "phantom points".

[behdad]

harfbuzz seems to do something different here using something called "phantom points".

Correct.

[RazrFalcon]

ttf_parser::Face::glyph_hor_advance looks fine to me. It doesn't have to be 1 to 1 with harfbuzz. If someone doesn't like ttf-parser behaviour they can easily overwrite it. Just like we do in rustybuzz.
Afaik, the TrueType spec doesn't define any of this. It defines the binary structure of the file, but not how to use parsed values.

As for phantom points, yep, not implemented. RazrFalcon/ttf-parser#27
That's a quite bizarre feature. Will take a look later.

Overall, the problem here is that in TrueType there are a billion ways to do exactly the same thing. And this is one of those edge cases.

[LaurenzV]

No worries, at least we know the "problem" now.

[asibahi]

Hello.

Came here to create an issue and found this one already. I have the same rust program implemented twice, once with harfbuzz_rs and once with rustybuzz. It is an Arabic font with variations, and the two libraries are giving visibly different results.

This with harfbuzz_rs: https://github.com/asibahi/nun/blob/e9e0cbb30d4113c45421a174fbb27f81e02be42b/images/noor_50.png

And this is with rustybuzz, same text and same initial settings: https://github.com/asibahi/nun/blob/1a995f5d10c1bd61425edb8b89f7ec049c43bdbd/images/noor_50.png

There is a rustybuzz branch. Running that and running master on the same text and settings will show you the different results.

There is a lot of other weird behaviors. Compare the images with the same names in both branches to see what I mean.

[behdad]

Do you know which one HarfBuzz produces? You probably can recreate using hb-view and a bunch of options.

[asibahi]

I am using my fork/PR of harfbuzz_rs which uses Harfbuzz 8.4.0 bindings

[RazrFalcon]

Yes, there is still a lot of work to do. rustybuzz tries to produce the same output as harfbuzz, but we're not there yet.
Also remember that rustybuzz matches harfbuzz 4.1 right now, not 8.4.

[LaurenzV]

@RazrFalcon I'm afraid it's still not working correctly, at least for this test case :(

cargo run --example shape -- MartianMonoVF.ttf --variations=wdth=100 ' 0123456789'
space=0+700|zero=1+748|one=2+703|two=3+703|three=4+709|four=5+750|five=6+702|six=7+742|seven=8+736|eight=9+702|nine=10+750

[RazrFalcon]

Yes, this is expected. My fix mostly affects spaces advance calculation. So they are no longer 0.

[RazrFalcon]

Stupid mistake... How about now?

[LaurenzV]

Will try again later.

[asibahi]

Over Eid vacation I have been working on a tiny toy project to compare HarfBuzz and Rustybuzz output for variable fonts (specifically Raqq). I came here to post it on this issue only to find it closed :) (I am using my fork of harfbuzz-rs and the crates.io version of rustybuzz.

Here it is anyway: https://github.com/asibahi/hb_vs_rb . I haven't (yet?) tried it with the input from my previous comment but for every small phrase I tried they matched perfectly. I even ran a loop over every permutation of the two axis to find discrepancies.

[LaurenzV]

@RazrFalcon Yep, seems to work prefectly now! Thanks a lot!

[LaurenzV]

@asibahi I suspect that your issue is not directly related to this, so if it is still reproducible with the newest main, I will try to look into it.

[asibahi]

@RazrFalcon small update: I refactored the code so I can switch between the two shapers with a trait, assuring I am calling both shapers exactly the same way.

I found no differences whatsoever. The earlier difference. must've been a bug in my code when I moved it to rusty buzz the first time.

[LaurenzV]

Great to hear! :)