1664804 - Dropping frames on Yeti Sensation with WebRender on MacOS

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Rares Doghi, Desktop QA]

Attachment: AboutSupportMAC.txt

[Affected platforms]:
Platforms: MAC OsX 10.15

[Preconditions]:
Ensure both gfx.webrender.all = true

Steps :

1. Launch the Firefox browser and reach https://www.crazygames.com/game/yeti-sensation
2. Start the game In Full screen mode.

Expected Results :
The gameplay experience should be smooth without lag or dropped frames.

Actual Results :
Playing the game in full screen a few times will cause frames to drop. (Noticeable when jumping or picking up the rocket).
With WebRender off the gameplay is a lot smoother.

Please note that I tried to get a screen recording of this issue but the quicktime player is recording with just a few fps and the dropping frames are not noticed in the recording.

Comment 1

[Rares Doghi, Desktop QA]

I think the Severity for this issue should be an S4 since it only happens on that specific game and the frame drop is not too noticeable.

Comment 2

[Kris Taeleman (:ktaeleman)]

@Matt: can you repro?

Comment 3

[Matt Woodrow (:mattwoodrow)]

I can!

It looks like we make the game fullscreen, but we still end up building the display list for all of the normal page behind it.

The fullscreen content has an opaque background, so I guess we're culling on the gpu, but only after we've paid significant costs for frame building and rendering of all that content.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1664804 - Build top layer content before we build scrolled content. r?miko,mstange

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1664804 - Compute whether top layer content is opaque. r?miko,mstange

The default styling for a ::backdrop pseudo element results in it being fully opaque and occluding all the rest of the page.
This allows us to detect that case early, and skip doing any work for the rest of the page.

Depends on D91668

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1664804 - Skip building RSF content if we have an opaque top layer occluding it. r?miko,mstange

Depends on D91669

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1664804 - Allow external surfaces for WebGL again. r?jgilbert

This check was previously added in bug 1628175, and then accidentally removed in bug 1632249.

Depends on D91670

Comment 8

[Timea Cernea [:tbabos][inactive]]

This is also reproducible on the latest Nightly 83.0a1 (2020-10-05) (64-bit) on Windows 7 x64 with Intel HD Graphics 4600 graphic card and Webrender enabled. Disabling webrender will make the game run smoother.

Comment 9

[Raluca Popovici, Desktop QA]

This is also reproducible on the latest Nightly 83.0a1 (2020-10-05) (64-bit) on Ubuntu 18.04 x64 with Intel HD Graphics 630 graphic card and Webrender enabled. Disabling webrender will make the game run smoother.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5ae04b52977
Build top layer content before we build scrolled content. r=miko
https://hg.mozilla.org/integration/autoland/rev/237da1bcdd52
Compute whether top layer content is opaque. r=miko
https://hg.mozilla.org/integration/autoland/rev/df9da74b1612
Skip building RSF content if we have an opaque top layer occluding it. r=miko
https://hg.mozilla.org/integration/autoland/rev/8afa0054e3cf
Allow external surfaces for WebGL again. r=jgilbert

Comment 11

[Narcis Beleuzu [:NarcisB]]

Backed out for assertion failures on AsyncCompositionManager.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/57186fcced4f868210fbff2849ebdd0193e52653
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=317703886&repo=autoland&lineNumber=5921

Please also check failures on dom/html/test/test_fullscreen-api.html -> https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=317703675&repo=autoland&lineNumber=10987

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17fef6ea08fe
Build top layer content before we build scrolled content. r=miko
https://hg.mozilla.org/integration/autoland/rev/f9c62853d8ba
Compute whether top layer content is opaque. r=miko
https://hg.mozilla.org/integration/autoland/rev/5e665bbcad8d
Skip building RSF content if we have an opaque top layer occluding it. r=miko
https://hg.mozilla.org/integration/autoland/rev/a54e27ab0e56
Allow external surfaces for WebGL again. r=jgilbert

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on browser_inspector_highlighter-geometry_06.js

backout: https://hg.mozilla.org/integration/autoland/rev/2c446d9b60d1a8707c8c03cdd24400a1da41e827

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-devtools-chrome-e10s%2Cdt9&revision=a54e27ab0e567307c795d58bf9525f5b9e684459&selectedTaskRun=fm5LnhG0T7GmtlbWKTBc7g.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=317971780&repo=autoland&lineNumber=5989

[task 2020-10-08T00:07:01.623Z] 00:07:01 INFO - TEST-PASS | devtools/client/inspector/test/browser_inspector_highlighter-geometry_06.js | top handler's x axis unchanged. -
[task 2020-10-08T00:07:01.623Z] 00:07:01 INFO - Buffered messages finished
[task 2020-10-08T00:07:01.624Z] 00:07:01 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/test/browser_inspector_highlighter-geometry_06.js | top handler's y axis updated. - Got 32, expected 42
[task 2020-10-08T00:07:01.626Z] 00:07:01 INFO - Stack trace:
[task 2020-10-08T00:07:01.627Z] 00:07:01 INFO - chrome://mochikit/content/browser-test.js:test_is:1332
[task 2020-10-08T00:07:01.629Z] 00:07:01 INFO - chrome://mochitests/content/browser/devtools/client/inspector/test/browser_inspector_highlighter-geometry_06.js:isHandlerPositionUpdated:132
[task 2020-10-08T00:07:01.629Z] 00:07:01 INFO - chrome://mochitests/content/browser/devtools/client/inspector/test/browser_inspector_highlighter-geometry_06.js:areElementAndHighlighterMovedCorrectly:110
[task 2020-10-08T00:07:01.629Z] 00:07:01 INFO - chrome://mochitests/content/browser/devtools/client/inspector/test/browser_inspector_highlighter-geometry_06.js:executeTest:87
[task 2020-10-08T00:07:01.630Z] 00:07:01 INFO - chrome://mochitests/content/browser/devtools/client/inspector/test/browser_inspector_highlighter-geometry_06.js:null:75
[task 2020-10-08T00:07:01.630Z] 00:07:01 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1069
[task 2020-10-08T00:07:01.630Z] 00:07:01 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1109
[task 2020-10-08T00:07:01.631Z] 00:07:01 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:932
[task 2020-10-08T00:07:01.632Z] 00:07:01 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:1037
[task 2020-10-08T00:07:01.632Z] 00:07:01 INFO - Checking element's sides are correct after drag & drop

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aba3cff2d5dd
Compute whether top layer content is opaque. r=miko
https://hg.mozilla.org/integration/autoland/rev/cdc14c572652
Skip building RSF content if we have an opaque top layer occluding it. r=miko
https://hg.mozilla.org/integration/autoland/rev/3d03dbc5ab3f
Allow external surfaces for WebGL again. r=jgilbert

Comment 15

[Razvan Maries]

Backed out for high frequent failure at APZCTreeManager.cpp.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Cdebug%2Cmochitests%2Ctest-linux1804-64-qr%2Fdebug-mochitest-plain-e10s%2C12&revision=3d03dbc5ab3f861f202fb18619a2daff0d818a2d&selectedTaskRun=P48404L6SImSvoOoNUuyOA.0

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318674806&repo=autoland&lineNumber=4490

Backout: https://hg.mozilla.org/integration/autoland/rev/d9d8f3ad8ed75f88bb3f67627bee8dcd0771104f

Comment 16

[Matt Woodrow (:mattwoodrow)]

Botond, could you please help me understand what that assertion is testing?

I managed to dump the display list and APZC state when it happens: https://paste.mozilla.org/47dczqZQ

Comment 17

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

General context first:

The patches here are introducing another scenario along the lines of https://bugzilla.mozilla.org/show_bug.cgi?id=1634763#c10 - APZ is getting a hit-test result from WR that has a specific scrollId, but APZ has no data for that scrollId. APZ's knowledge of the different scrollIds comes from the WebRenderLayerScrollData instances, which contain ScrollMetadata instances populated here. The WebRenderLayerScrollData instances in turn are built as we turn the gecko DL into a WR DL, mostly in this branch.

More specific:

Looking at the pastebin from the above comment, we can see (on line 39) that APZ got a hit-test result from WR that had scrollId=65. From the display list we can see there's a couple of fixed-pos items (lines 12 and 15) that have scrollTarget 65, so most likely the hit-test was done at a point that landed on one of those fixed-pos items (or more precisely, landed on one of the hit-test rectangles from lines 13 and 16, which would have the that scrollId as the target). Presumably scrollId 65 refers to the root scroll frame of the enclosing document, on line 10. But there are no display items in the display list that have an ASR pointing to that scrollId (in fact all the display items have null ASRs). I guess normally there's at least a background item or similar which will have an ASR with that scrollId, and that would trigger the creation of the WebRenderLayerScrollData instance. In this case I guess your patch optimized away all those display items, and so there's no WebRenderLayerScrollData instance, and therefore there's no HitTestingTreeNode (lines 31-37) with v=65 (i.e. APZ doesn't know about it).

I'm not sure as to what the right solution here. Seems like if you really want to make it so that all the display items inside the RSF are removed, then we should add some other mechanism for triggering the creation of the WebRenderLayerScrollData and populating it with the appropriate ScrollMetadata instances. If this is the approach you want to take, then off the top of my head I think we would need two changes:

1. Around this line we'd want to add something like:

      if (!forceNewLayerData &&
          item->GetType() == DisplayItemType::TYPE_SUBDOCUMENT &&
          item->AllMyChildrenGotOptimizedAway()) {
        forceNewLayerData = true;
        asr = item->AnASRThatPointsToMyRSF();
      }

This would make it go into the relevant codepaths to trigger the creation of the WebRenderLayerScrollData and insert it into the right place.

2. This line would need to be modified so that in the case of this special subdocument display item, we use AnASRThatPointsToMyRSF() instead of the ASR of the item itself.

There might be extra trickiness to deal with if AnASRThatPointsToMyRSF()->mParent is not the ASR of the subdocument item itself. In that case walking up the ASR chain might result in building a structure that violates assumptions elsewhere. But we can deal with that if it becomes a problem - hopefully this at least gives you some idea of what's going on and what might be done to tackle it.

Comment 18

[Matt Woodrow (:mattwoodrow)]

The testcase here had a subdocument as the top-layer of the root document, and then the <body> element of the subdocument as the top-layer of the subdoc.

The fixed position items created for the top-layer in the root document had a scrolltarget referencing the root ASR of the root document. Despite not being in any display items, we create WebRenderLayerScrollData for the root ASR implicitly, so these work correctly.

The fixed position items created for the top-layer in the subdoc had a scrolltarget refererencing the root ASR of their document. The only display items for that ASR got removed as part of the cull, so we never got WebRenderLayerScrollData for them, and we fail in this way.

I think the easiest thing to do for now is to only allow this optimization for top-layer content in the root content document, which should fix most cases we care about (including the original test case in this bug).

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/600b4f1152e6
Compute whether top layer content is opaque. r=miko
https://hg.mozilla.org/integration/autoland/rev/ce6494510851
Skip building RSF content if we have an opaque top layer occluding it. r=miko
https://hg.mozilla.org/integration/autoland/rev/c0f6b814ea1d
Allow external surfaces for WebGL again. r=jgilbert

Comment 20

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/600b4f1152e6
https://hg.mozilla.org/mozilla-central/rev/ce6494510851
https://hg.mozilla.org/mozilla-central/rev/c0f6b814ea1d

Comment 21

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #27255 (as of Sun, 18 Oct 2020 07:47:16 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
9%	glterrain		macosx1014-64-shippable-qr	e10s stylo	4.31 -> 3.91

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=27255

Comment 22

[Rares Doghi, Desktop QA]

This issue is verified as Fixed on Mac Osx 10.15, Ubuntu 18.04, Windows 7 as well as Windows 8.1 in our latest Beta build 83.0b4.