1442748 - Don't throttle frames if we haven't sent the root pipeline DL to WR yet

Status: RESOLVED FIXED

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

Description

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

The root cause of bug 1442608 appears to be this scenario:
- We create the WR instance and set the root pipeline id on it. This gets set in WR on the "pending" scene. However since there is no display list for that pipeline yet, it doesn't trigger a scene build
- Then we send a GenerateFrame request. Since there hasn't been a scene build, the root pipeline id was never copied from the "pending" scene to the "current" scene. Therefore the current scene has no root pipeline id set, and we skip the render/composite steps
- Since the render/composite never happens, the C++ RenderThread code doesn't get notified that the render is done, and so it forevermore denies WRBP [1] the ability to send more GenerateFrame requests. This results in nothing ever getting rendered or composited, and so we end up with a blank screen.

This is something of a race condition because if the root pipeline's display list does get to WR before the root pipeline id is set, then step 1 in the above sequence does cause a scene build and all is well.

This situation was introduced by the async scene building patches in WR. Prior to that, there was no "pending" scene vs "current" scene distinction. So in step 1 when we set the root pipeline id, it would get set on "the" scene, and the GenerateFrame would then render it even though it had no real content to render.

In bug 1442608 I plan to land a workaround where we detect the above scenario and force a scene build when we get the GenerateFrame request. That should work around the problem, but a better long-term fix is to have the frame throttling code only take effect once we've actually sent a display list for the root pipeline. Until that point WR is not guaranteed to render anything in response to the GenerateFrame requests.

This will need to be resolved before the next WR update, because the WR update will clobber the workaround and result in the bad situation again.

[1] https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/gfx/layers/wr/WebRenderBridgeParent.cpp#1209

Comment 1

[Nicolas Silva [:nical]]

Attachment: Don't generate frames if we haven't received a display list.

This is improves two things:
 - If we haven't received a display list, this WebRender bridge doesn't have content to render so generating a frame will waste CPU which is better used elsewhere when creating the widget, and avoid showing a blank frame.
 - This avoids getting the frame throttling mechanism to block subsequent frame because webrender declined to render the empty frame.

Comment 2

[Nicolas Silva [:nical]]

Attachment: Remove the workaround from bug 1442608.

The workaround isn't needed anymore.

With these two patches:

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

With the two patches + the commit in issue 2474 that fixes leaking pipeline epochs:

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

With the two patches + the two commits from issue 2474:

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

Comment 3

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

Comment on attachment 8956444 [details] [diff] [review]
Don't generate frames if we haven't received a display list.

Review of attachment 8956444 [details] [diff] [review]:
-----------------------------------------------------------------

Looks straightforward enough.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cf7f246faed
Don't generate frames if we haven't received a display list. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/31760a89ed73
Remove the workaround from 1442608. r=kats

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/4cf7f246faed
https://hg.mozilla.org/mozilla-central/rev/31760a89ed73