Closed Bug 1339575 Opened 3 years ago Closed 3 years ago

Convert nsDisplaySolidColor to a WebRender Display Item

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jnicol, Assigned: jnicol)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Implement nsDisplaySolidColor::GetLayerState and return LAYER_ACTIVE when layers.force-active=true.
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.
Attachment #8838133 - Flags: review?(mstange)
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 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.
Attachment #8838133 - Flags: review?(mstange) → review+
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.
Duplicate of this bug: 1351510
Summary: Allow nsDisplaySolidColor to be active → Convert nsDisplaySolidColor to a WebRender Display Item
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.
Component: Graphics: Layers → Graphics: WebRender
See Also: → 1351511
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?
Keywords: checkin-needed
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.
Keywords: checkin-needed
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 :)
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
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.
See Also: → 1352034
You need to log in before you can comment on or make changes to this bug.