1363683 - Update webrender to 7f37799d63ed80a07675ad599526290843394c99

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

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

+++ This bug was initially created as a clone of Bug #1361751 +++

I'm filing this as a placeholder bug for the next webrender update. I may be running a cron script [1] that does try pushes with webrender update attempts, so that we can track build/test breakages introduced by webrender on a rolling basis. This bug will hold the try push links as well as dependencies filed for those breakages, so that we have a better idea going into the update of what needs fixing. I might abort the cron job because once things get too far out of sync it's hard to fully automate fixing all the breakages.

When we are ready to actually land the update, we can rename this bug and use it for the update, and then file a new bug for the next "future update".

[1] https://github.com/staktrace/moz-scripts/blob/master/try-latest-webrender.sh

Comment 1

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

WR @ ea21ca3bf9eb56948efe7349c56120c4c8019c2c

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

Busted, because of changes to set_display_list to take a DisplayListBuilder instead of a BuiltDisplayList. This is hard to fix in gecko without reverting the API change.

I'm also trying WR @ 005eb39d79afca7ea0dc140d5cf7b809be06589b:

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

That's looking better, but it's an older WR.

Comment 2

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

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #1)
> I'm also trying WR @ 005eb39d79afca7ea0dc140d5cf7b809be06589b:
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=40a7eaf0bf1f3f98537c7bfb870973e860262194
> 
> That's looking better, but it's an older WR.

This ended up with a bunch of failures. The regression range is pretty big, I'll kick off some bisections.

*   005eb39 Auto merge of #1205 - glennw:fix-subpixel-aa, r=kvark
|\
| * 7d9fd04 Fix interaction between selecting subpixel AA and disabling text AA.
* |   6fe4fb4 Auto merge of #1204 - glennw:xform-fixes, r=kvark
|\ \
| |/
|/|
| * a8ab93f Make the check for a matrix preserving a 2d axis aligned rect more robust.
* |   c661afa Auto merge of #1200 - glennw:remove-old-borders, r=kvark
|\ \
| * | 88ab4bc Remove the old border code path.
* | |   e6c970c Auto merge of #1203 - nical:fuzzy-wrench, r=glennw
|\ \ \
| * | | 276e0b2 Add --fuzzy=<tolerance> option to wrench reftest.
* | | |   c84d649 Auto merge of #1193 - JerryShih:yuv-interleaved-format, r=kvark
|\ \ \ \
| |_|/ /
|/| | |
| * | | 49ce12b Add a single channel interleaved YCbCr image format.
| * | | 6180ac8 Add #[repr(u32)] for ExternalImageType enum.
|  / /
* | |   e2e07a0 Auto merge of #1199 - glennw:corners5, r=kvark
|\ \ \
| |_|/
|/| |
| * | c1efd22 Support border corners with differing styles.
* | |   7599f25 Auto merge of #1201 - rlhunt:composite/large, r=kvark
|\ \ \
| |_|/
|/| |
| * | 05f5d67 Read the correct render task for composite backdrops
|/ /
* |   254779a Auto merge of #1198 - JerryShih:resolve-ext-image, r=glennw
|\ \
| |/
|/|
| * b0bdc52 Resolve the ext-image again even though it's resolved in previous frame.
|/
* e44a079 Auto merge of #1194 - glennw:dots-7, r=kvark
* 24bf170 Add dotted border support to new path.

Comment 3

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

WR @ 254779aff04f475275966478a888385782031308

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

R7 failure is expected, R4 failure is new. The test in question (column-balancing-overflow-005.html) is already marked as fuzzy-if(webrender,255,12), and it looks like we can now change it to fuzzy-if(webrender,126,216). Not sure if that's an improvement or not :)

Comment 4

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

WR @ c84d649fa3abee170a158267473ad8b40e37bdda

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

This has all sorts of failures, so it narrows the regression range to this:

*   c84d649 Auto merge of #1193 - JerryShih:yuv-interleaved-format, r=kvark
|\
| * 49ce12b Add a single channel interleaved YCbCr image format.
| * 6180ac8 Add #[repr(u32)] for ExternalImageType enum.
*   e2e07a0 Auto merge of #1199 - glennw:corners5, r=kvark
|\
| * c1efd22 Support border corners with differing styles.
* 7599f25 Auto merge of #1201 - rlhunt:composite/large, r=kvark
* 05f5d67 Read the correct render task for composite backdrops

Comment 5

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

WR @ 7599f25fc5cb6eddcaee5df0c875c83587ed537e

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

Looking fine, this narrows the regression range to:

*   c84d649 Auto merge of #1193 - JerryShih:yuv-interleaved-format, r=kvark
|\
| * 49ce12b Add a single channel interleaved YCbCr image format.
| * 6180ac8 Add #[repr(u32)] for ExternalImageType enum.
* e2e07a0 Auto merge of #1199 - glennw:corners5, r=kvark
* c1efd22 Support border corners with differing styles.

And WR @ e2e07a05c65466406969947c2a8a6f675f9a1737

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

Which has the bustage, so the regression range is:

* e2e07a0 Auto merge of #1199 - glennw:corners5, r=kvark
* c1efd22 Support border corners with differing styles.

Comment 6

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

A quick investigation of the failure shows that the test is timing out and getting killed by the harness. The main thread is stuck in SendDPGetSnapshot, blocked on the compositor thread. The compositor thread has dutifully gone into WebRenderAPI::Readback and is waiting for the SynchronousTask to be completed on the render thread. The render thread seems to be just waiting for commands.

So most likely this has nothing to do with gw's patch, but is a pre-existing bug/race condition in our snapshotting code path.

Comment 7

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

I did a try push with some logging, and it looks like WebRenderAPI::Readback starts, but the render thread never runs the task it posted. See log at [1]. The stack dump for thread 47 below (at [2]) seems to indicate the render thread is idle and waiting for tasks. So most likely there's a race condition somewhere in the machinery to post tasks to the render thread and the task is getting dropped.

I can continue investigation tomorrow. Nical, if you have some cycles (and since you wake up before me), do you mind taking a look as well? IIRC you were involved with the threading machinery here.

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=98175948&repo=try&lineNumber=20189
[2] https://treeherder.mozilla.org/logviewer.html#?job_id=98175948&repo=try&lineNumber=24082

Comment 8

[Sotaro Ikeda [:sotaro]]

Readback event is sent via render backend thread by using ApiMsg::ExternalEvent(evt). From it, it seems that render backend thread becomes much more busy than before.

  https://dxr.mozilla.org/mozilla-central/source/gfx/webrender/src/render_backend.rs#422

Comment 9

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

Ok, some more logging shows that the render backend thread does get the message and processes it, but I guess it never sends the notification back to the compositor thread, or the notification is getting lost somewhere.

https://treeherder.mozilla.org/logviewer.html#?job_id=98367455&repo=try&lineNumber=76347

Comment 10

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

More logging [1] shows we're going into RenderThread::RunEvent from a non-render thread, it redispatches to the renderthread Loop(), and... disappears.

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=98391655&repo=try&lineNumber=82229

Comment 11

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

Swapping the chromium thread out for an xpcom thread didn't help. I think it's actually a deadlock but I don't know what the RenderThread is busy doing to create the deadlock. Will try to figure that out.

Comment 12

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

More logging seems to indicate that the renderthread goes into an UpdateAndRender operation and doesn't return for a long time. It starts at [1] and 5 minutes later it's still not done, and the test gets killed.

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=98445793&repo=try&lineNumber=109140

Comment 13

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

Ok, so glennw put together another PR (servo/webrender#1242) that seems to resolve this issue. I did a try push of WR @ c47ccf8deb8cc4dfd0852e0cd2a35e43cd924f9a without [1] and with [2] his PR applied, and the PR definitely makes things better. There are still 5 reftest failures, all related to box-shadow, and all of which are already marked with some sort of fuzz annotation (not necessarily for webrender though).

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=f46bcb059cdd2ff1e3ec4256de0e83746bee3c4b
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=96fabc04fb70d7f8d47c8de0f8424665386fb211

Comment 14

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

WR @ df12cbea6a0a5ede244c89398ec2055464ef2a0f

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

Back to bustage, more API changes.

Comment 15

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

WR @ 7f37799d63ed80a07675ad599526290843394c99

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

Looking mostly green. The mda* jobs are basically permafailing on graphics right now anyway. There's a couple of R8 intermittents but they don't seem permafail. I think we can push this update, I'll get the patches ready.

Comment 16

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

Attachment: Bug 1363683 - Update webrender to cset 7f37799d63ed80a07675ad599526290843394c99.

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

Comment 17

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

Attachment: Bug 1363683 - Cargo.toml/Cargo.lock updates.

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

Comment 18

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

Attachment: Bug 1363683 - Revendor third-party rust dependencies.

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

Comment 19

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

Attachment: Bug 1363683 - A fuzzy test changed in behaviour slightly, so adjust the fuzziness parameters.

This changed in WR cset 24bf170.

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

Comment 20

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

Attachment: Bug 1363683 - Update webrender bindings for change to display list construction and finalization API.

We now have to pass around a content size parameter to construct a display list,
and we get back a content size from finalizing the display list. Since we pass
the finalization results over IPC to WebRenderBridgeParent, we need to update
the IPDL as well to pass this around.

This changed in WR csets 425155a and 1eb84eb.

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

Comment 21

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

Attachment: Bug 1363683 - Some box shadow tests changed in behaviour, need fuzzing.

Unknown WR cset.

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

Comment 22

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

Attachment: Bug 1363683 - Update for scroll_node_with_id API change.

This changed in WR cset 48a098f.

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

Comment 23

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

Attachment: Bug 1363683 - Update for change in push_text API.

This changed in WR cset 3287c15.

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

Comment 24

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867814 [details]
Bug 1363683 - Update webrender to cset 7f37799d63ed80a07675ad599526290843394c99.

https://reviewboard.mozilla.org/r/139332/#review142676

Comment 25

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867816 [details]
Bug 1363683 - Revendor third-party rust dependencies.

https://reviewboard.mozilla.org/r/139336/#review142678

Comment 26

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867817 [details]
Bug 1363683 - A fuzzy test changed in behaviour slightly, so adjust the fuzziness parameters.

https://reviewboard.mozilla.org/r/139338/#review142680

Comment 27

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867818 [details]
Bug 1363683 - Update webrender bindings for change to display list construction and finalization API.

https://reviewboard.mozilla.org/r/139340/#review142682

Comment 28

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867819 [details]
Bug 1363683 - Some box shadow tests changed in behaviour, need fuzzing.

https://reviewboard.mozilla.org/r/139342/#review142684

Comment 29

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867820 [details]
Bug 1363683 - Update for scroll_node_with_id API change.

https://reviewboard.mozilla.org/r/139344/#review142686

Comment 30

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8867821 [details]
Bug 1363683 - Update for change in push_text API.

https://reviewboard.mozilla.org/r/139346/#review142688

Comment 31

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

In the interests of getting this landed sooner, I'm gonna assume you meant to r+ part 2 as well. :)

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/af0bdad77eac
Update webrender to cset 7f37799d63ed80a07675ad599526290843394c99. r=jrmuizel
https://hg.mozilla.org/projects/graphics/rev/575a2f6c3f68
A fuzzy test changed in behaviour slightly, so adjust the fuzziness parameters. r=jrmuizel
https://hg.mozilla.org/projects/graphics/rev/f396ec61fedd
Some box shadow tests changed in behaviour, need fuzzing. r=jrmuizel

Comment 33

[Jeff Muizelaar [:jrmuizel]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #31)
Yes

Comment 34

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/af0bdad77eac
https://hg.mozilla.org/mozilla-central/rev/575a2f6c3f68
https://hg.mozilla.org/mozilla-central/rev/f396ec61fedd