1339575 - Convert nsDisplaySolidColor to a WebRender Display Item

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jamie Nicol [:jnicol]]

Implement nsDisplaySolidColor::GetLayerState and return LAYER_ACTIVE when layers.force-active=true.

Comment 1

[Jamie Nicol [:jnicol]]

Attachment: Bug 1339575 - Add pref to create color layers for nsDisplaySolidColor.

Review commit: https://reviewboard.mozilla.org/r/113130/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/113130/

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8838133 [details]
Bug 1339575 - Add pref to create color layers for nsDisplaySolidColor.

https://reviewboard.mozilla.org/r/113130/#review115944

I don't see a BuildLayer method in this patch.

Comment 3

[Jamie Nicol [:jnicol]]

Comment on attachment 8838133 [details]
Bug 1339575 - Add pref to create color layers for nsDisplaySolidColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/113130/diff/1-2/

Comment 4

[Jamie Nicol [:jnicol]]

Comment on attachment 8838133 [details]
Bug 1339575 - Add pref to create color layers for nsDisplaySolidColor.

https://reviewboard.mozilla.org/r/113130/#review121538

::: layout/painting/nsDisplayList.cpp:2787
(Diff revision 2)
> +LayerState
> +nsDisplaySolidColor::GetLayerState(nsDisplayListBuilder* aBuilder,
> +                                   LayerManager* aManager,
> +                                   const ContainerLayerParameters& aParameters)
> +{
> +  if (ForceActiveLayers()) {

I'm not sure whether I should create a pref "layers.advanced.solid-color-layers" as an alternative way to enable this?

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 8838133 [details]
Bug 1339575 - Add pref to create color layers for nsDisplaySolidColor.

https://reviewboard.mozilla.org/r/113130/#review122124

::: layout/painting/nsDisplayList.cpp:2787
(Diff revision 2)
> +LayerState
> +nsDisplaySolidColor::GetLayerState(nsDisplayListBuilder* aBuilder,
> +                                   LayerManager* aManager,
> +                                   const ContainerLayerParameters& aParameters)
> +{
> +  if (ForceActiveLayers()) {

It's not worth adding a special pref for this if it doesn't cause test failures with webrender enabled. Thought I don't actually know if our Qr reftests have force-active-layers enabled.

Comment 6

[Jamie Nicol [:jnicol]]

We don't currently have it enabled. And enabling this does cause test failures, both with webrender and without. Probably some fuzzing is needed, which isn't that surprising.

Comment 8

[Jamie Nicol [:jnicol]]

I'm currently tracking down some reftest failures caused by this. Actually to do with image layers which are created as an optimisation by framelayerbuilder because of this, not because of this itself. I'll also update it to use DisplayItemLayer and have an advanced layers pref, similar to all other webrender display item conversions.

Comment 9

[Jamie Nicol [:jnicol]]

Comment on attachment 8838133 [details]
Bug 1339575 - Add pref to create color layers for nsDisplaySolidColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/113130/diff/2-3/

Comment 10

[Jamie Nicol [:jnicol]]

Okay I've switched this off behind a pref while I track down the reftest failures it uncovers.

Here's a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48119b038e98d3d1aca679037c385fd2e476faa1&selectedJob=87310712

Could this be checked in to the projects/graphics repo please?

Comment 11

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

I don't think the sheriffs are doing landings to the graphics repo. I can do it in a bit, once KWierso finishes pushing the current graphics tip over to m-c.

Comment 12

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

https://hg.mozilla.org/projects/graphics/rev/e11dc9ddab7a4dcbbda01bbbc8572bf8bfc0dbe3

I took the liberty of moving the pref in gfxPrefs.h up a line into alphabetical order :)

Comment 13

[Jamie Nicol [:jnicol]]

Thanks Kats. I should probably look into getting the rights to do it myself. Quite embarrassing that I actually was trying to put it in alphabetical order! :)

I'll file a new bug for switching the pref on.

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/e11dc9ddab7a