1346702 - Adjust fallback checks in nsDisplayBorder::GetLayerState

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Ethan Lin[:ethlin]]

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.

Comment 1

[Mason Chang [Inactive] [:mchang]]

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.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Create border renderer in nsDisplayBorder::GetLayerState

We should reuse the renderer created in nsDisplayBorder::GetLayerState. So we will do the fallback there.

Comment 3

[Ethan Lin[:ethlin]]

Attachment: Part2. Add CanCreateWebrenderCommands in BorderRenderer

The fallback checks include certain border styles, border width and border radius.

Comment 4

[Mason Chang [Inactive] [:mchang]]

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().

Comment 5

[Ethan Lin[:ethlin]]

(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

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/3a6924086fdb
https://hg.mozilla.org/mozilla-central/rev/864a80a4fc13