1702228 - Cache linear gradients by default

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Nicolas Silva [:nical]]

... except with SWGL, for which the gradient brush shader is faster than samping from the texture cache.

This is a followup from bug 1687977 which did the work for conic and radial gradients. Linear gradients are a bit trickier because they are used a lot more, so greater care must be taken in ensuring that we don't introduce regressions in some cases.

There are also more fast paths to take into account.

Comment 1

[Nicolas Silva [:nical]]

Attachment: WIP: Bug 1702228 - (WIP) cache linear gradients.

Comment 2

[Nicolas Silva [:nical]]

Attachment: WIP: Bug 1702228 - (WIP) linear gradient fast path

Depends on D110403

Comment 3

[Nicolas Silva [:nical]]

I've put the work-in-progress changes here, while I look into some higher priority things. Hopefully I'll get back to it. The remaining work is:

• make sure swgl still uses the non-cached linear gradient brush
• carefully look at the performance and make sure we don't regress important cases (since the fast paths change)
• adjust reftest references

Comment 4

[Nicolas Silva [:nical]]

Attachment: Bug 1702228 - Cache linear gradients by default. r=gw

This patch breaks linear gradients into two primitives:

• LinearGradient is always rendered via the brush shader. It is only used with SWGL.
• CachedLinearGradient is always rendered via a cached render task and the brush image shader. Its implementation is very similar to conic and radial gradients, with the addition of a fast-path for axis aligned gradients with two stops.

In addition the following changes are made:

• The gradient fast path is simpler (only deals with two gradient stops).
• Decomposing axis-aligned gradients into parts eligible for the fast path happens during scene building instead of frame building.

Comment 5

[Nicolas Silva [:nical]]

Attachment: WIP: Bug 1702228 - Render very small render tasks for the gradient fast path

Instead of rendering 1xN pixels for cached gradient segments, just render the color of the gradient stops in 1x2 render tasks. This requires adjusting the texture coordinates in the image brush shader which looks a bit hacky, however it has a few advantages:

• gradients occupy much less space in the texture cache because they are much smaller
• gradients that use the same colors are more likely to reuse the same render tasks even if rendered at different sizes, which further reduces the occupied space.
• the amount of rendered and sampled pixels is reduced as well.

Depends on D112018

Comment 6

[Nicolas Silva [:nical]]

Attachment: Bug 1702228 - Check gradient parameters. r=gw

Depends on D112018

Comment 7

[Nicolas Silva [:nical]]

Attachment: Bug 1702228 - Adjust reftests. r=gw

Depends on D112969

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e38dce4a5b06
Cache linear gradients by default. r=gw
https://hg.mozilla.org/integration/autoland/rev/0919633e25e9
Check gradient parameters. r=gw
https://hg.mozilla.org/integration/autoland/rev/f733b37f9626
Adjust reftests. r=gw

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e38dce4a5b06
https://hg.mozilla.org/mozilla-central/rev/0919633e25e9
https://hg.mozilla.org/mozilla-central/rev/f733b37f9626

Comment 10

[Richard Marti (:Paenglab)]

This bug seems to have regressed in Thunderbird. Back-out fixes the issue.

We use linear-gradient(to left, transparent, transparent 3px, var(--in-content-categories-background) 3px) which is no more shown. to right or 90deg works. I also tried -90deg which also didn't work but -89deg or -91deg are working.

Comment 11

[Liam England]

FYI, this bug caused a regression on the discord homepage (https://discord.com) as per this regression range.

Minimal test case: https://jsfiddle.net/g4sL2e5c/show

Comment 12

[Emilio Cobos Álvarez (:emilio)]

(In reply to LE from comment #11)
Thanks for the test-case! I filed bug 1707460 for it. Even if we revert this change, we should probably add that as an automated test somehow.