1439960 - Remove DisplayItemLayer

Status: RESOLVED FIXED

(Core :: Graphics: Layers, enhancement, P3)

Description

[Ryan Hunt [:rhunt] (on leave until early May)]

Now that layers-full webrender has been removed, DisplayItemLayer should be unused.

Comment 1

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, )

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

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8952744 [details]
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, )

https://reviewboard.mozilla.org/r/221978/#review228470

::: layout/painting/nsDisplayList.cpp:4935
(Diff revision 1)
>                                            const StackingContextHelper& aSc,
>                                            mozilla::layers::WebRenderLayerManager* aManager,
>                                            nsDisplayListBuilder* aDisplayListBuilder)
>  {
>    ContainerLayerParameters parameter;
>    if (GetLayerState(aDisplayListBuilder, aManager, parameter) != LAYER_ACTIVE) {

You're changing behavior here. There might be more implementations of CreateWebRenderCommands that call GetLayerState; you'll probably need to move the relevant checks from GetLayerState to CreateWebRenderCommands for all of them.

Comment 3

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8952744 [details]
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, )

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

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8952744 [details]
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, )

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb91b80515941e1e940a8ec937e86735ee6ba69e

There's still some reftest failures here on the retrigger, I'm not able to reproduce it locally so I'll continue investigating.

Comment 5

[Ryan Hunt [:rhunt] (on leave until early May)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=81786dcf770b59b8ed146e74032b776739bc0937

Green. The problem was in the removal of a call to GetLayerState from nsDisplayBackgroundImage, causing us to not request the image container with SYNC_DECODE when we need to.

Comment 6

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Respect SYNC_DECODE when deciding to use Webrender for nsDisplayBackgroundImage. (bug 1439960, )

Previously CreateWebrenderCommands would use GetLayerState to determine whether to use Webrender or
take the fallback path. GetLayerState would then under some cases call CanOptimizeToImageLayer()
which would get the image container using the appropriate flags for sync decoding.

Now nsDisplayBackgroundImage only uses CanCreateWebrenderCommands, which doesn't pass the correct
flags to image container, leading to reftest failures in some cases. This commit fixes that.

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

Comment 7

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8952744 [details]
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, )

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

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8960755 [details]
Respect SYNC_DECODE when deciding to use Webrender for nsDisplayBackgroundImage. (bug 1439960, )

https://reviewboard.mozilla.org/r/229496/#review235222

::: layout/painting/nsCSSRendering.cpp:2057
(Diff revision 1)
>  nsCSSRendering::CanBuildWebRenderDisplayItemsForStyleImageLayer(LayerManager* aManager,
>                                                                  nsPresContext& aPresCtx,
>                                                                  nsIFrame *aFrame,
>                                                                  const nsStyleBackground* aBackgroundStyle,
> -                                                                int32_t aLayer)
> +                                                                int32_t aLayer,
> +                                                                uint32_t mPaintFlags)

rename to aPaintFlags

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 8952744 [details]
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, )

https://reviewboard.mozilla.org/r/221978/#review235226

Nice!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6bd6ebc8597
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, r=mstange)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8d6b0fa1447
Respect SYNC_DECODE when deciding to use Webrender for nsDisplayBackgroundImage. (bug 1439960, r=mstange)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8057c06fc4c
Fix unified build bustage for missing include. (bug 1439960, r=me) on a CLOSED TREE

Comment 12

[Cosmin Sabou [:CosminS]]

Backed out for build bustages on APZInputBridgeChild.cpp and FrameBuilder.cpp

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&fromchange=c8d6b0fa14478a42379e2314eb7ce3783a09d9f6&tochange=b8057c06fc4c3e0943dc77fd897318c1000bfae2&selectedJob=169287275

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=169287275&repo=mozilla-inbound&lineNumber=18264
https://treeherder.mozilla.org/logviewer.html#?job_id=169293310&repo=mozilla-inbound

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/d732294871dc34b0481c40200691e29f7c8c0292

Comment 13

[Ryan Hunt [:rhunt] (on leave until early May)]

Try run with the fix for the unified bustage.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=67f088dd915d12dfc48a826e79c24518f5108f14

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62432cc896ee
Remove DisplayItemLayer and corresponding layers.advanced prefs. (bug 1439960, r=mstange)
https://hg.mozilla.org/integration/mozilla-inbound/rev/19ad67bba591
Respect SYNC_DECODE when deciding to use Webrender for nsDisplayBackgroundImage. (bug 1439960, r=mstange)

Comment 15

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/62432cc896ee
https://hg.mozilla.org/mozilla-central/rev/19ad67bba591