-
Notifications
You must be signed in to change notification settings - Fork 283
Port resvg to harfrust and fontations
#922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
02f55d3 to
62c028b
Compare
|
Yeah I think we first have to figure out how to deal with fontdb before doing anything with resvg. I don’t think RazrFalcon would want to switch it, and I’m not sure that fontique supports everything we need? |
|
It's an interesting experiment, but I don't see a point in it. Is there a reason in switching to PS: even if |
|
Oh wait, it depends on proc-macros/syn?! Burn it with fire! No proc-macros in |
harfrust is rustybuzz, just with ttf-parser switched out by read-fonts. It passes all the same tests, so it should be no less robust than then rustybuzz.
The reason is that going forward, (most likely) no one will be continually developing ttf-parser/rustybuzz, while harfrust will be actively maintained. |
|
I would suggest to come back to it in a year. No rush. And once again, proc-macros are no go. We have to figure out that one as well. |
Not sure about 'beta' type labels, but the first release (0.1.0) of HarfRust happened on June 10, marking a significant milestone.
For an immediate effect, it seems to be far more compliant when tested against the HarfBuzz test suite. To quote Chad:
And the HarfRust introduction post by Behdad contains the following claim:
Moving forward, HarfRust will receive active development and continuous sync with HarfBuzz, while RustyBuzz is likely to only see critical fixes. |
Apples and oranges, but sure. Either way, proc-macros is the current major roadblock. |
|
@RazrFalcon Seems like the proc-macros are coming from font-types which uses bytemuck and serde derive macros. Do we want to talk to them about converting to manual implementations? |
|
@mattfbacon if it's an easy fix - sure. |
|
No it wouldn't be at all, since the bytemuck derive macros generate unsafe code so they would need to allow unsafe code back into the codebase. But what is your plan here? Earlier you said wait, but for what? |
I would call it easy. It's not like the code deeply depends on proc-macros.
Adoption, etc. I don't see a point/reason in switching to 0.1. Especially if we would keep |
|
There are derive helpers based on It is likely possible to implement an alternative |
e29cfbe to
0973b97
Compare
harfrustharfrust and fontations
3c5c998 to
c05e2fd
Compare
As some months have passed another benefit has appeared - performance. In the last month the HarfRust vs HarfBuzz OpenType shaping performance difference has gone from ~4x to ~1.2x, which is a massive boost. Not quite on par with HarfBuzz yet, but getting a lot closer. More importantly for this PR here though, HarfRust is now a lot faster than RustyBuzz. |
|
The geometric mean of the last result is ~1.16, meaning Harfrust is now only 16% slower than Harfbuzz. |
It's even better than that. See the comment: In short, there's some hb-harfrust overhead included in the spreadsheet number. |
da2754d to
7e549e1
Compare
|
Diff for the colv1 test https://www.diffchecker.com/V8j95jlx with some manual fixups (removing groups - although it could also be those causing the differences?). I'm seeing some negations of y-coordinates (which possibly cancel each other out?), some rounding differences, and also that skrifa is applying some opacities to groups whereas ttf-parser applies them directly to the paths. |
|
As long as the COLRv1 test looks visually the same, it should be fine. |
|
@LaurenzV check out the visual diff on github. It's not quite the same. It's subtle, but one is "bolder" than the other (for lack of a better term). The diff produced by rhe test runner looks like the outline of each part of each glyph. |
|
It's possible the difference stems from the fact that the opacities are applied differently. When applying directly to a path, it will simply draw a path with that, while when applying to a group resvg will do layer isolation and apply the opacity that way instead, so you could try if this can be fixed by applying it directly. |
|
But if I remember COLRv1 might actually define opacities in terms of layer, in which case the previous behavior was simply incorrect. In any case, it still looks about the same so I wouldn't be too worried. |
e0b6593 to
8b2d26d
Compare
|
Ok, I've managed to fix the CBDT offset issue (I admit more by trial-and-error than by principle), and I've also managed to fix to fix the COLRv0 missing glyph parts (this was a silly bug where string was being thrown away rather than persisted). That leaves: The COLRv1 and Zalgo tests. Diff output from the test runner for these tests is as below:
For the COLRv1 tests, see also the diff in the generated "lowered" SVG output (https://www.diffchecker.com/V8j95jlx) @dfrg Do you have an opinion on whether these represent a genuine bug, a fixing of a bug, or a valid difference in interpretation? Y-coordinate negationsThere seems to be negated output for each y-coordinate in a path, which is then cancelled out by a negated y coordinate in a transform matrix. This is probably fine? But also, maybe we should fix it? Missing path segmentsSome of the text-related
The issue seems to be that the last segment of some (but not all) paths is being dropped in the Skrifa implementation. Although this seems to make no visual difference to the rendered SVG (see github diff), so perhaps that's also fine? |
|
As long as they visually appear the same, it’s fine. It’s not expected that we achieve pixel-level accuracy. |
These are equivalent but negating the y scale in the matrix rather than the y coords will generate smaller SVGs so I imagine that would be preferred.
Skrifa omits the final line segment if it matches the beginning of the subpath because this is effectively the same as the close command. I’m not familiar enough with SVG to understand the differences between group and path opacity so I can’t speak to those diffs. The output of both looks reasonable to me. |
https://razrfalcon.github.io/notes-on-svg-parsing/isolated-groups.html |
Good. It would not affect |
Signed-off-by: Nico Burns <[email protected]>
Signed-off-by: Nico Burns <[email protected]>
Signed-off-by: Nico Burns <[email protected]>
Signed-off-by: Nico Burns <[email protected]>
|
Harfrust 0.3.2 is now out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rendering on the right appears slightly thicker, especially in the eye area of the last emoji.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's a difference in opacity


Changes made
rustybuzzwithharfrustttf_parserwithskrifaNotes
1.65to1.80resvg.Production profile
The analysis below uses the following "production" profile:
Binary size analysis:
According to
cargo-bloat(not the most accurate):harfrustandrustybuzzare both ~250kbttf_parserandread-fontsare both ~100kbttf_parserincludes the metrics outlining functionality ofskrifa. And skrifa is another ~250kb.Compiling the CLI (which is a much more useful test) with the production profile above (
cargo build --profile production --bin resvg) I get:2.50mbformain2.85mbfor this PRSo this PR causes a binary increase of around
350kbCompile time analysis
Compile times on an M1 Pro (8+2 cores) are shown below (
cargo build):maindebugreleaseproductionSo this PR roughly doubles compile times at regular opt settings
(with the effect being less pronounced with LTO and other production optimisation settings enabled)