Closed Bug 1337715 Opened 3 years ago Closed 3 years ago

Add WrBorderRadius structure for border layer

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54

People

(Reporter: ethlin, Assigned: ethlin)

Details

Attachments

(2 files, 4 obsolete files)

This is a kind of refactoring bug. I will add WrBorderRadius type as the parameter to WR.
I add WrBorderRadius in this patch and also use 'WrColor' for wr_push_rect since we already have it.
Attachment #8834820 - Flags: review?(bugmail)
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
Attachment #8834820 - Flags: review?(bugmail) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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!
Addressed kats' comments. This part is for WrBorderRadius.
Attachment #8834820 - Attachment is obsolete: true
This part is about using WrColor in wr_push_rect.
The patch was wrong. Correct it.
Attachment #8835312 - Attachment is obsolete: true
Attachment #8835314 - Attachment is obsolete: true
Pushed by ethlin@mozilla.com:
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
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.