Adjust fallback checks in nsDisplayBorder::GetLayerState

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ethlin, Assigned: ethlin)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Currently webrender doesn't support all border styles. If we want to turn on the pref 'layers.advanced.border-layers' by default, we should handle the falling back checks correctly.
On the call today, we discussed that we'd prefer to get the simple / common cases off to WR and just leave the complicated versions to a fallback path for now.
(Assignee)

Comment 2

2 years ago
Created attachment 8846996 [details] [diff] [review]
Create border renderer in nsDisplayBorder::GetLayerState

We should reuse the renderer created in nsDisplayBorder::GetLayerState. So we will do the fallback there.
Attachment #8846996 - Flags: review?(mchang)
(Assignee)

Comment 3

2 years ago
Created attachment 8846998 [details] [diff] [review]
Part2. Add CanCreateWebrenderCommands in BorderRenderer

The fallback checks include certain border styles, border width and border radius.
Attachment #8846998 - Flags: review?(mchang)
(Assignee)

Updated

2 years ago
Summary: Adjust falling back checks in nsDisplayBorder::GetLayerState → Adjust fallback checks in nsDisplayBorder::GetLayerState
Attachment #8846996 - Flags: review?(mchang) → review+
Comment on attachment 8846998 [details] [diff] [review]
Part2. Add CanCreateWebrenderCommands in BorderRenderer

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

::: layout/painting/nsCSSRenderingBorders.cpp
@@ +3545,5 @@
> +      return false;
> +    }
> +
> +    if (mBorderRadii[i].width != 0 || mBorderRadii[i].height != 0) {
> +      return false;

So does this mean we just never render any borders that have a border radii? Is that the intent?

@@ +3551,5 @@
> +  }
> +
> +  if (mBorderWidths[eSideTop] != mBorderWidths[eSideBottom] ||
> +      mBorderWidths[eSideBottom] != mBorderWidths[eSideLeft] ||
> +      mBorderWidths[eSideLeft] != mBorderWidths[eSideRight]) {

nit: Wrap this is a IsUniform().
Attachment #8846998 - Flags: review?(mchang) → review+
(Assignee)

Comment 5

2 years ago
(In reply to Mason Chang [:mchang] from comment #4)
> Comment on attachment 8846998 [details] [diff] [review]
> Part2. Add CanCreateWebrenderCommands in BorderRenderer
> 
> Review of attachment 8846998 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/painting/nsCSSRenderingBorders.cpp
> @@ +3545,5 @@
> > +      return false;
> > +    }
> > +
> > +    if (mBorderRadii[i].width != 0 || mBorderRadii[i].height != 0) {
> > +      return false;
> 
> So does this mean we just never render any borders that have a border radii?
> Is that the intent?

There are many reftest failures are about radii problem. Actually webrender supports border radii. I think we can remove this check for now and see how to fix those failures.  

> @@ +3551,5 @@
> > +  }
> > +
> > +  if (mBorderWidths[eSideTop] != mBorderWidths[eSideBottom] ||
> > +      mBorderWidths[eSideBottom] != mBorderWidths[eSideLeft] ||
> > +      mBorderWidths[eSideLeft] != mBorderWidths[eSideRight]) {
> 
> nit: Wrap this is a IsUniform().

After the webrender update in bug 1346915, I think we can skip the width check.

Comment 6

2 years ago
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/3a6924086fdb
Part1. Create border renderer in nsDisplayBorder::GetLayerState. r=mchang
https://hg.mozilla.org/projects/graphics/rev/864a80a4fc13
Part2. Add CanCreateWebrenderCommands for checking if the render should create WR commands. r=mchang
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.