1793105 - Update COLRv1 radial gradient rendering as discussed in spec issue

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

A lengthy discussion about radial gradients in https://github.com/googlefonts/colr-gradients-spec/issues/367 is finally converging on a result.

At issue is what it means for the radii of the circles defining a radial gradient to be negative. This doesn't occur in HTML or CSS gradients, as such values would simply be an error, but in COLRv1 fonts it is possible for variation deltas to make one or both of the radii negative, even though the default values (before variations are applied) must be non-negative because they are unsigned in the font table.

The rendering algorithm described in the spec didn't consider negative radii, but it turns out to handle this just fine, and the resulting behavior makes geometric sense. (Details in the spec issue.) And following discussion and experiments, we have concluded that it's reasonable to implement this using the available shader APIs (which was in doubt for a while), even though they don't directly accept negative geometry.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1793105 - Update COLRv1 radial-gradient handling of negative radii. r=#gfx-reviewers

This implements the adjusted behavior agreed with Dominik in
https://github.com/googlefonts/colr-gradients-spec/issues/367
and email discussion. Dominik is updating Blink to have the
same behavior.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1793105 - Add a reftest for COLRv1 radial gradients with variations that involve negative radii. r=#gfx-reviewers

THe last two lines in the testcase fail badly with current trunk code, and are fixed by the patch here.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db9c185baf7a
Update COLRv1 radial-gradient handling of negative radii. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/d1d1d489003c
Add a reftest for COLRv1 radial gradients with variations that involve negative radii. r=gfx-reviewers,lsalzman

Comment 4

[Jonathan Kew [:jfkthame]]

A quick followup: Dominik's testing revealed an error in the edge case where there are co-located color stops at an end of the color line, and we end up collapsing to a single "padding" color because the defined range of the color line is outside the visible cone.

I have a simple patch to fix this, which I'll post for review as a followup here, and add an example of this edge-case to the reftest.

Comment 5

[Jonathan Kew [:jfkthame]]

Try run in progress to check if fuzz numbers need tweaking... https://treeherder.mozilla.org/jobs?repo=try&revision=aacf7fdf417debb7714bb16eb10abbc2581c4958

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1793105 - followup, fix for the edge-case of collapsing the color line to one end, with co-located stops. r?lsalzman

Rather than calling TruncateGradientStops, which doesn't handle this case properly (it leaves multiple
color stops because they're at the same offset), we can just delete all but the required color directly.

Added an example of this scenario to the reftest.

Comment 7

[Jonathan Kew [:jfkthame]]

Marking bug as leave-open until the followup is reviewed/landed.

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/db9c185baf7a
https://hg.mozilla.org/mozilla-central/rev/d1d1d489003c

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe373509557f
followup, fix for the edge-case of collapsing the color line to one end, with co-located stops. r=lsalzman

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/fe373509557f