Backporting

[RazrFalcon]

A place to discuss the backporting of harfbuzz changes.

[RazrFalcon]

@bluebear94 @notgull Here we can discuss backporting process, if needed.

[RazrFalcon]

I have invited you both to the repo. It should speed up process a bit by avoiding needless pull request.
You can still create pull requests for review purposes.

[bluebear94]

Thanks for the invitation!

[notgull]

The next step is the commits between 2.7.4 and 2.8.0, i.e. #38. See here: harfbuzz/harfbuzz@2.7.4...2.8.0

[RazrFalcon]

Currently stuck at harfbuzz/harfbuzz@d8ea552
The harfbuzz code, as usual, is pretty insane. AAT in general is mind-boggling. Will have to figure out how to humanize it first in C++, test it and then port to Rust.

Also, we do not test Apple fonts at all, due to licensing issues, therefore it would be hard to prove that the change actually works.

Otherwise this commit is the last big change before finishing 2.8. Hopefully will finish it tomorrow.

[bluebear94]

Also, we do not test Apple fonts at all, due to licensing issues, therefore it would be hard to prove that the change actually works.

Couldn’t we generate the AAT tests but put them behind a feature flag, so that these tests can at least be run by people using macOS?

[RazrFalcon]

We can. But not only you need macOS, you would need a specific version of a font as well. What HB does is that it check font's hash in macOS tests. Meaning we have to:

1. Update the scripts/gen-shaping-tests.py or port those test manually. Figure out how to check hashes and all of this stuff.
2. Update the CI to run those tests on different macOS versions.

I simply hadn't had time to do all of this during the initial rustybuzz release. And Apple/AAT fonts aren't that common, outside mac especially, therefore it's was a low priority task.

[RazrFalcon]

I've added #80

[behdad]

Currently stuck at harfbuzz/harfbuzz@d8ea552
The harfbuzz code, as usual, is pretty insane. AAT in general is mind-boggling. Will have to figure out how to humanize it first in C++, test it and then port to Rust.

At least that commit is well-commented. The logic is complex. I'm curious to see your humanized C++ version.

[RazrFalcon]

Welp, I've ended up with:

const auto is_safe_to_break_extra = [&]() {
    /* 2c. */
    const auto wouldbe_entry = machine.get_entry(StateTableT::STATE_START_OF_TEXT, klass);

    /* 2c'. */
    if (c->is_actionable (this, wouldbe_entry))
        return false;

    /* 2c". */
    return next_state == machine.new_state(wouldbe_entry.newState)
        && (entry.flags & context_t::DontAdvance) == (wouldbe_entry.flags & context_t::DontAdvance);
};

const auto is_safe_to_break = [&]() {
    /* 1. */
    if (c->is_actionable (this, entry))
        return false;

    /* 2. */
    // This one is meh, I know...
    const auto ok =
           state == StateTableT::STATE_START_OF_TEXT
        || ((entry.flags & context_t::DontAdvance) && next_state == StateTableT::STATE_START_OF_TEXT)
        || is_safe_to_break_extra();
    if (!ok)
        return false;

    /* 3. */
    return !c->is_actionable (this, machine.get_entry (state, StateTableT::CLASS_END_OF_TEXT));
};

if (!is_safe_to_break() && buffer->backtrack_len () && buffer->idx < buffer->len)
    buffer->unsafe_to_break_from_outbuffer (buffer->backtrack_len () - 1, buffer->idx + 1);

Is it more read-able? Debatable. But it's definitely more portable and straight-forward.
For example, there are no C-style assignments in conditions in Rust (afaik). Aka if (a = b)

PS: And I would also embed docs directly, for readability, but my English-foo isn't that good.

[behdad]

Thanks. Updated.

[behdad]

FWIW, lambdas in C++ are not free and don't get inlined and optimized properly. This had regressed LucidaGrande shaping time by over 50%. Fixing in harfbuzz/harfbuzz#5031 (comment)

[RazrFalcon]

Oh wow, that's pretty bad and unexpected. Those lamdas look trivial enough to be inlined.

[RazrFalcon]

2.8.0 is done. Will be able to work on it again in a couple of weeks, so feel free to send patches in the mean time.

[bluebear94]

Working on 2.8.2 right now.

[bluebear94]

Working on 2.9.0 right now.

[bluebear94]

2.9.1 is a pretty small update, but it adds tests that specify pre- and post-context, so we’ll eventually have to add support for that in Rustybuzz.

There’s also a change to an hb_buffer_append function that we’ve yet to port.

[RazrFalcon]

If it affects a missing feature you can easily skip it.

[CraftSpider]

I'm interested in potentially contributing some work towards this crate - however, I have no clue how to start with setting up an environment to run tests for a given HB version, or what the process is in general. If someone could give me some pointers, I'd love to help give this crate some love.

[bluebear94]

RazrFalcon has written instructions on how to generate tests from HarfBuzz’s test suite.

[RazrFalcon]

I will write a porting guide in the next couple of days. This should simplify things quite a bit.

[RazrFalcon]

Added the initial draft: https://github.com/RazrFalcon/rustybuzz/blob/master/docs/backporting.md

As always, it turned out to be harder then I was expecting. I think if you simply start asking questions it would be much easier for me. And I would update the docs accordingly.

@bluebear94 if you have anything to contribute/suggest - please do. I'm not really sure how to ease the involvement process.

[Keavon]

@RazrFalcon I have one potential suggestion that could augment what you've written in that guide. It might be worthwhile to set up a .devcontainer/devcontainer.json (here's ours for Graphite just for reference) with the necessary development environment setup, especially for the Harfbuzz side, so it can be easy for interested contributors to get up-and-running. It also means people can boot up a free GitHub Codespaces environment even if they're not on Linux, for example. Then you could update the backporting writeup to show how to use it with that preconfigured environment (like the harfbuzz tests). Just an idea!

[RazrFalcon]

Thanks, but I'm not sure it would help much. It's not that hard to build harfbuzz.
And I never used those containers, so it only will add complexity.