Closed Bug 1852513 Opened 9 months ago Closed 7 months ago

Render gradients with different color interpolation methods

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
121 Branch
Tracking Status
firefox121 --- fixed

People

(Reporter: tlouw, Assigned: ahale)

References

Details

Attachments

(7 files)

CSS now supports specifying a color interpolation method for gradients (linear, radial and conical)

See:
https://drafts.csswg.org/css-images-4/#linear-gradients
https://drafts.csswg.org/css-images-4/#radial-gradients
https://drafts.csswg.org/css-images-4/#conic-gradients

Currently WebRender only supports rendering gradients with linear sRGB interpolation.

Interpolation is still linear for each of the source components, but once converted to sRGB, they might not be linear any more.

How does the gfx code receive this new mode setting? Or do we need to do the plumbing for that through the Gecko to WebRender interface?

This sounds like it can either be a LUT for several stops along the gradient (to fit the curve closely) or new shader code converting the color space for each pixel (which may be slower than the LUT for SWGL on old devices that need software rendering).

The new data is stored inside the StyleGradient struct and the color interpolation is stored here:
https://searchfox.org/mozilla-central/source/__GENERATED__/layout/style/ServoStyleConsts.h#13578
Interpolation method is stored for each gradient type: linear, radial, conical.

AFAICT the data ends up here: https://searchfox.org/mozilla-central/rev/699a7704521354cac1e6a0679029c89ca899e25c/layout/painting/nsCSSRenderingGradients.cpp#1212 where the display lists are created for gradients. From there to WR, I'm not entirely sure.

As for rendering method, I think checking on how other browsers handle it might be worth it.

For an idea of what the color space conversions look like, have a look here:
https://searchfox.org/mozilla-central/rev/699a7704521354cac1e6a0679029c89ca899e25c/servo/components/style/color/convert.rs

Some of the conversions are just a few math operations, but worst case the color components go through 3-4 matrix transforms.

Assignee: nobody → ahale
Status: NEW → ASSIGNED
Depends on: 1860018
Blocks: 1861363

Note for those following along: I'm primarily using https://codepen.io/web-dot-dev/pen/JjBXEQL as a test for the new layout.css.gradient-color-interpolation-method.enabled code path, because it hits the most edge cases in my testing (as shown above in previous attachments).

Possibly related bugs that should benefit from more color stops but are out of scope for the current enablement criteria:

Attached the current way my patch renders the same test, almost correct.

Pushed by tlouw@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d55b7b5e620c
Use more gradient color stops for interpolation r=tlouw,emilio

Backed out for causing gradient-eval-* failures

[task 2023-11-02T11:33:45.710Z] 11:33:45     INFO - TEST-START | /css/css-images/gradient/gradient-eval-002.html
[task 2023-11-02T11:33:45.713Z] 11:33:45     INFO - PID 18211 | 1698924825712	Marionette	INFO	Testing http://web-platform.test:8000/css/css-images/gradient/gradient-eval-002.html == http://web-platform.test:8000/css/css-images/gradient/gradient-eval-002-ref.html
[task 2023-11-02T11:33:45.925Z] 11:33:45     INFO - PID 18211 | 1698924825923	Marionette	INFO	Allowed 0-10000 pixels different, maximum difference per channel 1-2
[task 2023-11-02T11:33:45.967Z] 11:33:45     INFO - TEST-UNEXPECTED-PASS | /css/css-images/gradient/gradient-eval-002.html | Testing http://web-platform.test:8000/css/css-images/gradient/gradient-eval-002.html == http://web-platform.test:8000/css/css-images/gradient/gradient-eval-002-ref.html
Flags: needinfo?(ahale)

Technically not causing failures - it caused tests to pass that previously failed, per discussion with tlouw I've removed the meta files for those tests which had been expecting failure, since we now expect them to pass.

I've refined the patch slightly to correct one other test which had pixel colors that deviated just a little too much, so it now uses 128 color stops instead of 32 to more closely follow the correct color curves, this might increase cpu usage a little on load but I doubt it will even show up on perf tests.

Flags: needinfo?(ahale)
Pushed by tlouw@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/910d9a3cb77f
Use more gradient color stops for interpolation r=tlouw,emilio
Pushed by ahale@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/269afd5c7d29
Use more gradient color stops for interpolation r=tlouw,emilio

Backed out for causing wr failures in oklab-gradient.html.

I've updated the tests to tolerate slightly more color deviation because on MOZ_HEADLESS=1 they deviate a bit more than the current limits, but without MOZ_HEADLESS they are well within tolerances already, which is why my local test results did not match try as it uses MOZ_HEADLESS=1.

Flags: needinfo?(ahale)
Pushed by ahale@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48de4f46ad5b
Use more gradient color stops for interpolation r=tlouw,emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43070 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 121 Branch
Upstream PR merged by moz-wptsync-bot

Hi there. Is it expected that interpolation using longer cycles 360 degrees if the start and stop colors are identical? For example:

Is it expected that the following is solid red or a gradient that cycles 360 degrees?

background: conic-gradient(in hsl longer hue, 
  hsl(0, 100%, 50%), 
  hsl(0, 100%, 50%)
);

This came up because of doc fixes prompted by Nightly changes. Context here: https://github.com/mdn/content/pull/30275#issuecomment-1812051921

Flags: needinfo?(ahale)

Thanks for reporting the issue. That is indeed a bug, but predates the gradient rendering in different color spaces, so I filed a bug for the fix here in bug 1864851

Flags: needinfo?(ahale)

Super, thank you for the clarification.

Regressions: 1885716
Regressions: 1885717
No longer regressions: 1885717
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: