Adjust fallback checks in nsDisplayBorder::GetLayerState

RESOLVED FIXED in Firefox 55

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: ethlin, Assigned: ethlin)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

10 months 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

10 months 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

10 months 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

10 months 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

10 months 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

10 months 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: 10 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.