1355570 - Update nsCSSRenderingGradients for changes in WebRender

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ryan Hunt [:rhunt]]

There have been a number of upstream changes in WebRender that affect the display item generation.
1. Normalization and other fixes for degenerate gradients is now handled
2. Support was added for tiling in single display item
3. Premultiplied alpha is now done with a blend mode

Additionally, the original patch for building display items didn't use aSrc which is an error.

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 1355570 - Update bindings and WebRender display list building for gradient tiling

Review commit: https://reviewboard.mozilla.org/r/128994/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128994/

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1355570 - Premultiply WebRender gradient stop colors

Review commit: https://reviewboard.mozilla.org/r/128996/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128996/

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1355570 - Simplify nsCSSRenderingGradients

In nsCSSRenderingGradients the logic for handling many error conditions
and normalizing the gradient stops is shared between the paint path and
the WebRender path. WebRender now normalizes gradients and handles any
error conditions that it needs to. There are some conditions that are not
problems for it, like repeating radial gradients with stops below zero.

This commit undoes the work done in bug 1341101 to share this logic. Some
conditions were moved around in bug 1341101 to make things simpler, and
that has been undone. Now the paint path is identical to how it was
originally.

There is one exception, which is ResolveMidpoints which is kept between
both code paths. This should be safe.

Review commit: https://reviewboard.mozilla.org/r/128998/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128998/

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: Bug 1355570 - Respect aSrc when building WebRender gradients

Review commit: https://reviewboard.mozilla.org/r/129000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129000/

Comment 5

[Ryan Hunt [:rhunt]]

(In reply to Ryan Hunt [:rhunt] from comment #3)
> Created attachment 8857095 [details]
> Bug 1355570 - Simplify nsCSSRenderingGradients
> 
> In nsCSSRenderingGradients the logic for handling many error conditions
> and normalizing the gradient stops is shared between the paint path and
> the WebRender path. WebRender now normalizes gradients and handles any
> error conditions that it needs to. There are some conditions that are not
> problems for it, like repeating radial gradients with stops below zero.
> 
> This commit undoes the work done in bug 1341101 to share this logic. Some
> conditions were moved around in bug 1341101 to make things simpler, and
> that has been undone. Now the paint path is identical to how it was
> originally.
> 
> There is one exception, which is ResolveMidpoints which is kept between
> both code paths. This should be safe.
> 
> Review commit: https://reviewboard.mozilla.org/r/128998/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/128998/

I forgot to add that ResolvePremultipliedAlpha used to be shared by both code paths, but is now only done for the Paint path. WebRender handles this internally, and shouldn't need to create a bunch of intermediate colors stops to blend premultiplied.

ResolveMidpoints is similar to ResolvePremultipliedAlpha (creating extra color stops to fake nonlinear interpolation), and this could maybe some day be handled in WebRender.

Comment 6

[Ryan Hunt [:rhunt]]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4fb0b7eb8f348f2b8d7bc7f25d35ed8da0c4931

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8857094 [details]
Bug 1355570 - Premultiply WebRender gradient stop colors

https://reviewboard.mozilla.org/r/128996/#review131670

::: layout/painting/nsCSSRenderingGradients.cpp:1027
(Diff revision 1)
>    bool isRepeat = mGradient->mRepeating || mForceRepeatToCoverTiles;
>    aMode = isRepeat ? WrGradientExtendMode::Repeat : WrGradientExtendMode::Clamp;
>  
>    aStops.SetLength(mStops.Length());
>    for(uint32_t i = 0; i < mStops.Length(); i++) {
> -    aStops[i].color.r = mStops[i].mColor.r;
> +    float alpha = mStops[i].mColor.a * aOpacity;

Probably should use 'Float' here since that's what type ColorStops uses.

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8857095 [details]
Bug 1355570 - Simplify nsCSSRenderingGradients

https://reviewboard.mozilla.org/r/128998/#review131672

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8857096 [details]
Bug 1355570 - Respect aSrc when building WebRender gradients

https://reviewboard.mozilla.org/r/129000/#review131676

::: layout/painting/nsCSSRenderingGradients.cpp:1077
(Diff revision 1)
> +                                                   aDest.width / ((float)mPresContext->CSSPixelsToAppUnits(aSrc.width)),
> +                                                   aDest.height / ((float)mPresContext->CSSPixelsToAppUnits(aSrc.height)));
> +
>    if (mGradient->mShape == NS_STYLE_GRADIENT_SHAPE_LINEAR) {
> +    lineStart.x = (lineStart.x - srcTransform.x) * srcTransform.width;
> +    lineStart.y = (lineStart.y - srcTransform.y) * srcTransform.height;

These first two lines could be moved outside of the if.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8857093 [details]
Bug 1355570 - Update bindings and WebRender display list building for gradient tiling

https://reviewboard.mozilla.org/r/128994/#review132102

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/9d9628ffe7a0
Update bindings and WebRender display list building for gradient tiling r=jrmuizel
https://hg.mozilla.org/projects/graphics/rev/940396cc2852
Premultiply WebRender gradient stop colors r=mattwoodrow
https://hg.mozilla.org/projects/graphics/rev/30005fc4bb99
Simplify nsCSSRenderingGradients r=mattwoodrow
https://hg.mozilla.org/projects/graphics/rev/1732ef63e9c8
Respect aSrc when building WebRender gradients r=mattwoodrow

Comment 12

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/mozilla-central/rev/9d9628ffe7a0
https://hg.mozilla.org/mozilla-central/rev/940396cc2852
https://hg.mozilla.org/mozilla-central/rev/30005fc4bb99
https://hg.mozilla.org/mozilla-central/rev/1732ef63e9c8