black-renderer
black-renderer copied to clipboard
Fix sweep gradients: they were buggy, and the spec is changing
In particular, extend modes seem to behave differently, in the spec, for sweep gradient than for linear and radial ones. For example, normalizing the colorLine seems unwated for sweeps, but more processing of the color linee be required, for simulating repeat, pad, mirror. Solutions will be different for Skia on one hand, and the other backends on the other hand.
Also: the Skia backend doesn't currently implement the sweep extend mode correctly: the extend mode should not extend before the start angle and after end angle (???).
First of all, big thanks @justvanrossum for the report on Chromium/Skia using the incorrect start axis (vertical up instead of horizontal right). I filed the following issues after further investigation: https://bugs.chromium.org/p/skia/issues/detail?id=13208, https://bugs.chromium.org/p/skia/issues/detail?id=13209, https://bugs.chromium.org/p/skia/issues/detail?id=13210. So the start angle was wrong, repeat modes didn't work, the sector was cut, and sectors crossing 0 did not work.
To fix this, I increased test coverage and improved the Skia implementation.
In the following I try to break down some observations of what I am seeing in BlackRenderer to hopefully get to a common understanding and help with debugging issues in BlackRenderer for sweeps.
New Test Glyphs
https://github.com/googlefonts/color-fonts/pull/98 contains additional test glyphs for sweep (now merged to main)
Skia Fix and Results
My fix CL for Skia is in: https://skia-review.googlesource.com/c/skia/+/529460
With that I get the following, in our opinion correct results (rows are extend modes pad, reflect, repeat respectively). Please do comment if you think some interpretations/renderings are still wrong as that may well be possible.
BlackRenderer results
Extend mode pad:
$ blackrenderer ~/dev/skia/resources/fonts/more_samples-glyf_colr_1.ttf "a456789α" sweeps.png --font-size=100
The tests glyphs are:
_sample_sweep(-360, 0, "pad", "narrow", next(access_chars)),
_sample_sweep(0, 90, "pad", "narrow", next(access_chars)),
_sample_sweep(45, 90, "pad", "narrow", next(access_chars)),
_sample_sweep(247.5, 292.5, "pad", "narrow", next(access_chars)),
_sample_sweep(90, 270, "pad", "narrow", next(access_chars)),
_sample_sweep(-270, 270, "pad", "narrow", next(access_chars)),
_sample_sweep(-45, 45, "pad", "narrow", next(access_chars)),
_sample_sweep(315, 45, "pad", "narrow", next(access_chars)),
This shows:
- Start color red is not drawn only from start angle, but from 0 degrees, but color stop 0 should be mapped to start angle.
- Issues when start Angle is negative.
- Issues when angle valus cross 0/360 degrees. (last three images)
Extend mode reflect
$ blackrenderer ~/dev/skia/resources/fonts/more_samples-glyf_colr_1.ttf "βγδεζηθι" sweeps_reflect.png --font-size=100
_sample_sweep(-360, 0, "reflect", "narrow", next(access_chars)),
_sample_sweep(0, 90, "reflect", "narrow", next(access_chars)),
_sample_sweep(45, 90, "reflect", "narrow", next(access_chars)),
_sample_sweep(247.5, 292.5, "reflect", "narrow", next(access_chars)),
_sample_sweep(90, 270, "reflect", "narrow", next(access_chars)),
_sample_sweep(-270, 270, "reflect", "narrow", next(access_chars)),
_sample_sweep(-45, 45, "reflect", "narrow", next(access_chars)),
_sample_sweep(315, 45, "reflect", "narrow", next(access_chars)),
Issues, as much as I can tell:
- Start angle (in first picture) starts from vertical position, not from horizontal
- The reflecting sector seems too small?
Extend mode repeat
$ blackrenderer ~/dev/skia/resources/fonts/more_samples-glyf_colr_1.ttf "κλμνξοπρ" sweeps_repeat.png --font-size=100
_sample_sweep(-360, 0, "repeat", "narrow", next(access_chars)),
_sample_sweep(0, 90, "repeat", "narrow", next(access_chars)),
_sample_sweep(45, 90, "repeat", "narrow", next(access_chars)),
_sample_sweep(247.5, 292.5, "repeat", "narrow", next(access_chars)),
_sample_sweep(90, 270, "repeat", "narrow", next(access_chars)),
_sample_sweep(-270, 270, "repeat", "narrow", next(access_chars)),
_sample_sweep(-45, 45, "repeat", "narrow", next(access_chars)),
_sample_sweep(315, 45, "repeat", "narrow", next(access_chars)),
Issues, as much as I can tell, similar to reflect:
- Start angle (in first picture) starts from vertical position, not from horizontal
- The reflecting sector seems too small?
I think I have the reflect
and repeat
modes wrong and the observation that the
- The reflecting sector seems too small? is incorrect.
For the
_sample_sweep(0, 90, "reflect", "narrow", next(access_chars)),
test shown in my second row, second image from the left, there's an implicit "pad" that fills the 0-90 degree angle, which is incorrect.
I'll update this report after fixing that.
Also, contrary to what I said earlier:
This shows:
Start color red is not drawn only from start angle, but from 0 degrees, but color stop 0 should be mapped to start angle.
Starting from start angle 0, and padding with red seems correct and is not an issue.
With WIP Skia CL #532057 we are getting closer but we still need some clarifications on what happens at the 0 / 360 degree boundary.
Extend mode pad:
Test glyphs
_sample_sweep(-360, 0, "pad", "narrow", next(access_chars)),
_sample_sweep(0, 90, "pad", "narrow", next(access_chars)),
_sample_sweep(45, 90, "pad", "narrow", next(access_chars)),
_sample_sweep(247.5, 292.5, "pad", "narrow", next(access_chars)),
_sample_sweep(90, 270, "pad", "narrow", next(access_chars)),
_sample_sweep(-270, 270, "pad", "narrow", next(access_chars)),
_sample_sweep(-45, 45, "pad", "narrow", next(access_chars)),
_sample_sweep(315, 45, "pad", "narrow", next(access_chars)),
$ blackrenderer ~/dev/skia/resources/fonts/more_samples-glyf_colr_1.ttf "a456789α" sweeps.png --font-size=100
Skia
-
_sample_sweep(-270, 270, "pad", "narrow", next(access_chars)),
does not match in BlackRenderer and Skia, even though this glyph IMO should be equivalent to_sample_sweep(90, 270, "pad", "narrow", next(access_chars)),
. - Also, not sure why we diverge on
_sample_sweep(315, 45, "pad", "narrow", next(access_chars)),
. - This result however is based on certain assumptions that are not 100% clear in the spec, see https://github.com/googlefonts/colr-gradients-spec/issues/361#issuecomment-1105250860
Extend mode reflect
Test glyphs
_sample_sweep(-360, 0, "reflect", "narrow", next(access_chars)),
_sample_sweep(0, 90, "reflect", "narrow", next(access_chars)),
_sample_sweep(45, 90, "reflect", "narrow", next(access_chars)),
_sample_sweep(247.5, 292.5, "reflect", "narrow", next(access_chars)),
_sample_sweep(90, 270, "reflect", "narrow", next(access_chars)),
_sample_sweep(-270, 270, "reflect", "narrow", next(access_chars)),
_sample_sweep(-45, 45, "reflect", "narrow", next(access_chars)),
_sample_sweep(315, 45, "reflect", "narrow", next(access_chars)),
$ blackrenderer ~/dev/skia/resources/fonts/more_samples-glyf_colr_1.ttf "βγδεζηθι" sweeps_reflect.png --font-size=100
Skia
Similarly quite well aligned, not sure what happens in BlackRenderer on the 6th and 8th glyph, which should IMO be equivalent to 5th and 7th respectively.
Extend mode repeat
Test glyphs
_sample_sweep(-360, 0, "repeat", "narrow", next(access_chars)),
_sample_sweep(0, 90, "repeat", "narrow", next(access_chars)),
_sample_sweep(45, 90, "repeat", "narrow", next(access_chars)),
_sample_sweep(247.5, 292.5, "repeat", "narrow", next(access_chars)),
_sample_sweep(90, 270, "repeat", "narrow", next(access_chars)),
_sample_sweep(-270, 270, "repeat", "narrow", next(access_chars)),
_sample_sweep(-45, 45, "repeat", "narrow", next(access_chars)),
_sample_sweep(315, 45, "repeat", "narrow", next(access_chars)),
$ blackrenderer ~/dev/skia/resources/fonts/more_samples-glyf_colr_1.ttf "κλμνξοπρ" sweeps_repeat.png --font-size=100
Skia
Similarly quite well aligned, not sure what happens in BlackRenderer on the 6th and 8th glyph, which should IMO be equivalent to 5th and 7th respectively.
Also relevant: https://github.com/fonttools/fonttools/pull/2743
New, wider set of tests in https://github.com/googlefonts/color-fonts/pull/119, Chrome rendering with https://skia-review.googlesource.com/c/skia/+/566416