1430660 - Move APZ updates to scrollframes in WR into a transaction

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P1)

Description

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

APZ currently updates WR scrollframe offsets directly via WebRenderAPI at [1], which is invoked from [2]. We should move this into the transaction at [3] so that it happens along with the OMTA updates for that frame and isn't happening randomly outside a transaction.

[1] https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/gfx/layers/apz/src/APZCTreeManager.cpp#516
[2] https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/gfx/layers/wr/WebRenderBridgeParent.cpp#1232
[3] https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/gfx/layers/wr/WebRenderBridgeParent.cpp#1243

Comment 1

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

Attachment: Bug 1430660 - Move the APZ update of scroll layer positions into a transaction.

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

Comment 2

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

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

Comment 3

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

Comment on attachment 8942947 [details]
Bug 1430660 - Move the APZ update of scroll layer positions into a transaction.

That try push is most definitely not green.

Comment 4

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

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

Comment 5

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

Comment on attachment 8942947 [details]
Bug 1430660 - Move the APZ update of scroll layer positions into a transaction.

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

Comment 6

[Nicolas Silva [:nical]]

Comment on attachment 8942947 [details]
Bug 1430660 - Move the APZ update of scroll layer positions into a transaction.

https://reviewboard.mozilla.org/r/213210/#review219282

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21f6b322e9a2
Move the APZ update of scroll layer positions into a transaction. r=nical

Comment 8

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 21f6b322e9a2 (bug 1430660) for build bustage on webrender_bindings/RenderThread.cpp(475)


Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=21f6b322e9a29a5938193369a890415742bf4081&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success

Failure log: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=21f6b322e9a29a5938193369a890415742bf4081&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success

Backout: https://hg.mozilla.org/integration/autoland/rev/1e723381da62b08b15147357610b989aecc853e4

Comment 9

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

Thanks, looks like the windows compiler doesn't like having the extern declaration in webrender_ffi_generated.h as well as the impl in RenderThread.cpp. I'll fix.

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=65e020cf7a1d961b8472361f71da9a53048bde86 is looking good. I just extracted a helper method so we aren't calling wr_notifier_new_frame_ready directly in RenderThread.cpp. That works around the compile error.

Comment 11

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

Comment on attachment 8942947 [details]
Bug 1430660 - Move the APZ update of scroll layer positions into a transaction.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96e818315d57
Move the APZ update of scroll layer positions into a transaction. r=nical

Comment 13

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/96e818315d57