1337681 - Support border radius for WebRenderBorderLayer

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ethan Lin[:ethlin]]

We should pass the radius info to WebRenderBorderLayer as well.

Comment 1

[Ethan Lin[:ethlin]]

Attachment: Part1. Support border radius for WebrenderBorderLayer

This patch will be pushed to m-c. I'm not sure if there is a convert function for Size to LayerSize.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Part2. Turn on pref for a test about border radius

I turn on the pref 'layers.advanced.border-layers' for a reftest. I think there are some rendering bugs in Webrender, like layout/reftests/border-radius/border-ellips.html. WR seems to not support a radius with different width/height.

Comment 3

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

Comment on attachment 8834801 [details] [diff] [review]
Part1. Support border radius for WebrenderBorderLayer

Review of attachment 8834801 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/painting/nsDisplayList.cpp
@@ +4410,5 @@
>      }
> +    mCorners[0] = LayerSize(br->mBorderRadii.TopLeft().width, br->mBorderRadii.TopLeft().height);
> +    mCorners[1] = LayerSize(br->mBorderRadii.TopRight().width, br->mBorderRadii.TopRight().height);
> +    mCorners[2] = LayerSize(br->mBorderRadii.BottomLeft().width, br->mBorderRadii.BottomLeft().height);
> +    mCorners[3] = LayerSize(br->mBorderRadii.BottomRight().width, br->mBorderRadii.BottomRight().height);

I would prefer to see mCorners here keep the same order as the rest of gecko [1] - topLeft, topRight, bottomRight, bottomLeft, because this code is really in gecko. The bottomRight <-> bottomLeft ordering is different in webrender, but I think that the mapping should be done in the WebRenderBorderLayer, hence my comment about ordering in bug 1337715 comment 2. You can even use NS_FOR_CSS_FULL_CORNERS here, I think.

[1] http://searchfox.org/mozilla-central/rev/672c83ed65da286b68be1d02799c35fdd14d0134/gfx/2d/Types.h#405

@@ +4447,5 @@
>    }
> +  mCorners[0] = LayerSize(br->mBorderRadii.TopLeft().width, br->mBorderRadii.TopLeft().height);
> +  mCorners[1] = LayerSize(br->mBorderRadii.TopRight().width, br->mBorderRadii.TopRight().height);
> +  mCorners[2] = LayerSize(br->mBorderRadii.BottomLeft().width, br->mBorderRadii.BottomLeft().height);
> +  mCorners[3] = LayerSize(br->mBorderRadii.BottomRight().width, br->mBorderRadii.BottomRight().height);

ditto

Comment 4

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8834801 [details] [diff] [review]
Part1. Support border radius for WebrenderBorderLayer

Review of attachment 8834801 [details] [diff] [review]:
-----------------------------------------------------------------

As Kats said, please keep gecko ordering for display items. r=me with that changed.

Comment 5

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Comment on attachment 8834801 [details] [diff] [review]
> Part1. Support border radius for WebrenderBorderLayer
> 
> Review of attachment 8834801 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/painting/nsDisplayList.cpp
> @@ +4410,5 @@
> >      }
> > +    mCorners[0] = LayerSize(br->mBorderRadii.TopLeft().width, br->mBorderRadii.TopLeft().height);
> > +    mCorners[1] = LayerSize(br->mBorderRadii.TopRight().width, br->mBorderRadii.TopRight().height);
> > +    mCorners[2] = LayerSize(br->mBorderRadii.BottomLeft().width, br->mBorderRadii.BottomLeft().height);
> > +    mCorners[3] = LayerSize(br->mBorderRadii.BottomRight().width, br->mBorderRadii.BottomRight().height);
> 
> I would prefer to see mCorners here keep the same order as the rest of gecko
> [1] - topLeft, topRight, bottomRight, bottomLeft, because this code is
> really in gecko. The bottomRight <-> bottomLeft ordering is different in
> webrender, but I think that the mapping should be done in the
> WebRenderBorderLayer, hence my comment about ordering in bug 1337715 comment
> 2. You can even use NS_FOR_CSS_FULL_CORNERS here, I think.
> 
> [1]
> http://searchfox.org/mozilla-central/rev/
> 672c83ed65da286b68be1d02799c35fdd14d0134/gfx/2d/Types.h#405
> 
> @@ +4447,5 @@
> >    }
> > +  mCorners[0] = LayerSize(br->mBorderRadii.TopLeft().width, br->mBorderRadii.TopLeft().height);
> > +  mCorners[1] = LayerSize(br->mBorderRadii.TopRight().width, br->mBorderRadii.TopRight().height);
> > +  mCorners[2] = LayerSize(br->mBorderRadii.BottomLeft().width, br->mBorderRadii.BottomLeft().height);
> > +  mCorners[3] = LayerSize(br->mBorderRadii.BottomRight().width, br->mBorderRadii.BottomRight().height);
> 
> ditto

Okay! I'll change the order.

Comment 6

[Ethan Lin[:ethlin]]

Attachment: Part1. Support border radius for BorderLayer. r=mattwoodrow

Addressed kats' comments. I use NS_FOR_CSS_FULL_CORNERS for the radius assignment. And split the m-c patch from the original patch.

Comment 7

[Ethan Lin[:ethlin]]

Attachment: Part2. Support border radius for WebRenderBorderLayer. r=mattwoodrow

This patch is for WebRenderBorderLayer and it will be pushed to graphics branch.

Comment 8

[Ethan Lin[:ethlin]]

Attachment: Part3. Turn on pref for a test about border radius. r=mattwoodrow

This patch should be part3 now.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19199c39b67e
Part1. Support border radius for BorderLayer. r=mattwoodrow

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/19199c39b67e

Comment 11

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

Comment on attachment 8835265 [details] [diff] [review]
Part1. Support border radius for BorderLayer. r=mattwoodrow

This got merged back to the graphics branch in e79bf3dc8926

Comment 12

[Ethan Lin[:ethlin]]

try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=98c587822c121aec4fc9d724802e15d3210118df&selectedJob=76253261

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/661f8aa1e17f
Part2. Support border radius for WebRenderBorderLayer. r=mattwoodrow
https://hg.mozilla.org/projects/graphics/rev/44155de565e7
Part3. Turn on pref layers.advanced.border-layers for WebRenderBorderLayer's border radius. r=mattwoodrow