1337715 - Add WrBorderRadius structure for border layer

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ethan Lin[:ethlin]]

This is a kind of refactoring bug. I will add WrBorderRadius type as the parameter to WR.

Comment 1

[Ethan Lin[:ethlin]]

Attachment: Add WrBorderRadius and apply WrColor for wr_push_rect

I add WrBorderRadius in this patch and also use 'WrColor' for wr_push_rect since we already have it.

Comment 2

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

Comment on attachment 8834820 [details] [diff] [review]
Add WrBorderRadius and apply WrColor for wr_push_rect

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

It would have been nice to split the WrColor changes into a separate patch from the WrBorderRadius changes. Probably not worth doing it now, but try to do that in the future. r=me with the comments below addressed.

::: gfx/layers/wr/WebRenderBorderLayer.cpp
@@ +57,5 @@
>                     wr::ToWrBorderSide(mWidths[0], mColors[0], mBorderStyles[0]),
>                     wr::ToWrBorderSide(mWidths[1], mColors[1], mBorderStyles[1]),
>                     wr::ToWrBorderSide(mWidths[2], mColors[2], mBorderStyles[2]),
>                     wr::ToWrBorderSide(mWidths[3], mColors[3], mBorderStyles[3]),
> +                   wr::ToWrBorderRadius(mCorners[0], mCorners[1], mCorners[2], mCorners[3])));

The order here needs to be 0, 1, 3, 2 to match the existing behaviour

::: gfx/webrender_bindings/src/bindings.rs
@@ +826,5 @@
> +impl WrBorderRadius
> +{
> +    pub fn to_border_radius(&self) -> BorderRadius
> +    {
> +        BorderRadius { top_left: self.top_left.to_layout_size(), top_right: self.top_right.to_layout_size(), bottom_left: self.bottom_left.to_layout_size(), bottom_right: self.bottom_right.to_layout_size() }

nit: please break this up over multiple lines to make it more readable

Comment 3

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> Comment on attachment 8834820 [details] [diff] [review]
> Add WrBorderRadius and apply WrColor for wr_push_rect
> 
> Review of attachment 8834820 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It would have been nice to split the WrColor changes into a separate patch
> from the WrBorderRadius changes. Probably not worth doing it now, but try to
> do that in the future. r=me with the comments below addressed.

It's fine. I'll split the WrColor changes into another patch.

> 
> ::: gfx/layers/wr/WebRenderBorderLayer.cpp
> @@ +57,5 @@
> >                     wr::ToWrBorderSide(mWidths[0], mColors[0], mBorderStyles[0]),
> >                     wr::ToWrBorderSide(mWidths[1], mColors[1], mBorderStyles[1]),
> >                     wr::ToWrBorderSide(mWidths[2], mColors[2], mBorderStyles[2]),
> >                     wr::ToWrBorderSide(mWidths[3], mColors[3], mBorderStyles[3]),
> > +                   wr::ToWrBorderRadius(mCorners[0], mCorners[1], mCorners[2], mCorners[3])));
> 
> The order here needs to be 0, 1, 3, 2 to match the existing behaviour

Sorry, I should follow the original order.

> 
> ::: gfx/webrender_bindings/src/bindings.rs
> @@ +826,5 @@
> > +impl WrBorderRadius
> > +{
> > +    pub fn to_border_radius(&self) -> BorderRadius
> > +    {
> > +        BorderRadius { top_left: self.top_left.to_layout_size(), top_right: self.top_right.to_layout_size(), bottom_left: self.bottom_left.to_layout_size(), bottom_right: self.bottom_right.to_layout_size() }
> 
> nit: please break this up over multiple lines to make it more readable

Okay!

Comment 4

[Ethan Lin[:ethlin]]

Attachment: Part1. Add WrBorderRadius for WebRenderBorderLayer. r=kats

Addressed kats' comments. This part is for WrBorderRadius.

Comment 5

[Ethan Lin[:ethlin]]

Attachment: Part2. Use WrColor as parameter for wr_push_rect. r=kats

This part is about using WrColor in wr_push_rect.

Comment 6

[Ethan Lin[:ethlin]]

Attachment: Part1. Add WrBorderRadius for WebRenderBorderLayer. r=kats

The patch was wrong. Correct it.

Comment 7

[Ethan Lin[:ethlin]]

Attachment: Part1. Add WrBorderRadius for WebRenderBorderLayer. r=kats

Comment 8

[Ethan Lin[:ethlin]]

Attachment: Part1. Add WrBorderRadius for WebRenderBorderLayer. r=kats

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/a15741e12268
Part1. Add WrBorderRadius for WebRenderBorderLayer. r=kats
https://hg.mozilla.org/projects/graphics/rev/6acfa0db546d
Part2. Use WrColor as parameter for wr_push_rect. r=kats