1885716 - Incorrect colors on a Codepen demo

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Mayank Bansal]

Attachment: Standalone_testcase.html

Go to https://codepen.io/ghaste/pen/oNOzRME

AR:

1. The colors are all in shades of red.

Bisection points to::
Bug 1852513 - Use more gradient color stops for interpolation r=tlouw,emilio
Differential Revision: https://phabricator.services.mozilla.com/D190903

Comment 1

[Mayank Bansal]

Attachment: Bug.png

Comment 2

[Mayank Bansal]

Attachment: about:support

Comment 3

[Mayank Bansal]

There are maybe simpler demos with the same issue: https://codepen.io/ghaste/pen/JjVGvQL

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

:ahale, since you are the author of the regressor, bug 1852513, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: reduced test-case

if I specify two stops like: background-image: linear-gradient(in hsl longer hue 45deg, #f66 0 0, #f66);, then it works as expected... So this check isn't quite correct, since the end stop might be implied from the start.,

Comment 6

[Tiaan Louw [:tlouw]]

Attachment: Firefox vs chrome with 100% color stop

It also looks like the interpolation is running in reverse. When i change the color stop to: #f66 0 100%, I get this result in firefox/chrome (chrome on the right. Looks like they are inverted on

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1852513

Comment 8

[Chris Bellini (:cbellini)]

Will this fix target 126?

Comment 9

[Glenn Watson [:gw]]

Ashley, looks like this is a regression based on comment 7. Redirecting question from comment 8 to you.

Comment 10

[Ashley Hale [:ahale]]

Ah, I looked into this bug when it was filed but got side-tracked before finishing the patch.

I'll write the patch now, it's not clear to me if this needs to block 126 or not because long-way-around interpolation methods on HSL are commonly used for colorwheel selectors in UI design, so it depends entirely if it breaks one that is significant, it is unfortunately not an edge-case in terms of impact.

From my recollection, the logic needs to be >=1 color stops when it's a hue interpolation method (HSL/HWB/LCH/OKLCH) because of the long-way-around case, and otherwise >=2 is correct in terms of detecting no-op gradients.

In future we plan to enable the pref which would trigger the gfxPlatform::GetCMSMode() == CMSMode::All check to always succeed, which will also obviate this issue, but that's on a longer timescale, so this needs to be fixed.

Comment 11

[Ashley Hale [:ahale]]

Attachment: Bug 1885716 - Fix HSL/HWB/LCH/OKLCH gradient longer hue interpolation 1-stop case

Comment 12

[Ashley Hale [:ahale]]

It turns out it's not the logic for whether to add extra stops that is to blame here, this is a bug in how the ColorStopInterpolator::CreateStops function handles the last stop, especially in the case of a single stop gradient with longer hue interpolation.

Patch is coming along well, and will add 2 tests for coverage of these issues.

Comment 13

[Ashley Hale [:ahale]]

While I have gotten it to render the gradients (it was just showing a single color for me in testing previously), the interpolation direction appears to be backwards on my test cases compared to other browsers, so we're not done here yet.

Comment 14

[Ashley Hale [:ahale]]

NeedInfo - What do we need to do to make StyleColorSpace interpolation go the other way around on a 360 degree case with longer?

Comment 15

[Tiaan Louw [:tlouw]]

Because we use weight based values to do interpolation, it is reversed from a lerp. That said it looks like we have a bug here:

https://searchfox.org/mozilla-central/rev/c09764753ea40725eb50decad2c51edecbd33308/servo/ports/geckolib/glue.rs#8814

We take the parameter progress which suggests progress from left to right, but this is not correct because of the weights thing. I filed bug 1890488 to fix this.

Comment 16

[Ashley Hale [:ahale]]

That isn't the interpolation bug I meant - I already worked around that in the code that calls it ( https://searchfox.org/mozilla-central/source/layout/painting/nsCSSRenderingGradients.cpp#521 ), I am saying that the 'longer hue' interpolation in Style when given the same color twice seems to go backwards around the color wheel compared to other browsers. I assume the bug is solely with same-color interpolation using longer, which may be picking increasing / decreasing incorrectly.

Comment 17

[Ashley Hale [:ahale]]

I've revised my diff to fix the longer hue interpolation direction for identical colors, and moved the tests to WPT.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/a65ef2dc959b
Fix HSL/HWB/LCH/OKLCH gradient longer hue interpolation 1-stop case r=emilio

Comment 19

[Iulian Moraru]

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

Comment 20

[Mayank Bansal]

This is fixed for me on the latest Nightly. Nightly shows the same colors as Chrome now.

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 22

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2024-03-15 Nightly build on macOS 12.
Issue is verified as fixed on Firefox Nightly 127.0a1 and Firefox 126.0 on macOS 12, Windows 10, Ubuntu 22.