1459686 - Apply async images when sending display list to WR

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

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

When we send a new display list to WR at [1] that contains new async image pipeline references, the contents of those referenced pipelines don't actually get sent to WR until later. This can result in WR complaining about missing pipelines. What happens is that when WRBP gets the transaction from the content process, it updates the AsyncImagePipelineManager at [2], but it doesn't send that info to WR. That only happens later at [3] in a separate transaction. So the scene build that is triggered by [1] doesn't find the pipelines that are referenced in the DL.

[1] https://searchfox.org/mozilla-central/rev/b28b94dc81d60c6d9164315adbd4a5073526d372/gfx/layers/wr/WebRenderBridgeParent.cpp#651-655
[2] https://searchfox.org/mozilla-central/rev/b28b94dc81d60c6d9164315adbd4a5073526d372/gfx/layers/wr/WebRenderBridgeParent.cpp#621
[3] https://searchfox.org/mozilla-central/rev/b28b94dc81d60c6d9164315adbd4a5073526d372/gfx/layers/wr/WebRenderBridgeParent.cpp#1293

Comment 1

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

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

Comment 2

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

Attachment: Bug 1459686 - Remove WebRenderCommandBuilder::mParentCommands which is always empty.

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

Comment 3

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

Attachment: Bug 1459686 - Refactor to have the ApplyAsyncImages callsite provide the transaction.

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

Comment 4

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

Attachment: Bug 1459686 - Include async image updates in the displaylist transaction.

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

Comment 5

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8973799 [details]
Bug 1459686 - Remove WebRenderCommandBuilder::mParentCommands which is always empty.

https://reviewboard.mozilla.org/r/242168/#review248126

Yea, we do not need it anymore.

Comment 6

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8973800 [details]
Bug 1459686 - Refactor to have the ApplyAsyncImages callsite provide the transaction.

https://reviewboard.mozilla.org/r/242170/#review248132

Comment 7

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8973801 [details]
Bug 1459686 - Include async image updates in the displaylist transaction.

https://reviewboard.mozilla.org/r/242172/#review248134

You might want to add a comment why it is necessary.

Comment 8

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

(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> You might want to add a comment why it is necessary.

Will do. Also I just realized the part 1 patch seems to have a bunch of merge day goop in it, no idea how that happened. I'll clean it up.

Comment 9

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

Comment on attachment 8973799 [details]
Bug 1459686 - Remove WebRenderCommandBuilder::mParentCommands which is always empty.

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

Comment 10

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

Comment on attachment 8973800 [details]
Bug 1459686 - Refactor to have the ApplyAsyncImages callsite provide the transaction.

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

Comment 11

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

Comment on attachment 8973801 [details]
Bug 1459686 - Include async image updates in the displaylist transaction.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4869243efd9c
Remove WebRenderCommandBuilder::mParentCommands which is always empty. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/b27eadf1b8de
Refactor to have the ApplyAsyncImages callsite provide the transaction. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/25652d08b28e
Include async image updates in the displaylist transaction. r=sotaro

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/4869243efd9c
https://hg.mozilla.org/mozilla-central/rev/b27eadf1b8de
https://hg.mozilla.org/mozilla-central/rev/25652d08b28e