Closed Bug 1664804 Opened 1 year ago Closed 11 months ago

Dropping frames on Yeti Sensation with WebRender on MacOS

Categories

(Core :: Graphics: WebRender, defect)

Desktop
macOS
defect

Tracking

()

VERIFIED FIXED
83 Branch
Tracking Status
firefox80 --- disabled
firefox81 --- disabled
firefox82 --- disabled
firefox83 --- verified

People

(Reporter: rares.doghi, Assigned: mattwoodrow)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files, 1 obsolete file)

Attached file 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.

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.

Blocks: wr-mac
Blocks: gfx-triage
See Also: → 1665030

@Matt: can you repro?

Flags: needinfo?(matt.woodrow)

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.

Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)

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

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

Depends on D91670

Blocks: 1665030
No longer blocks: gfx-triage

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.

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.

Pushed by mwoodrow@mozilla.com:
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
Flags: needinfo?(matt.woodrow)
Pushed by mwoodrow@mozilla.com:
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

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

Flags: needinfo?(matt.woodrow)
Attachment #9178343 - Attachment is obsolete: true
Pushed by mwoodrow@mozilla.com:
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
Flags: needinfo?(matt.woodrow)

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

Flags: needinfo?(matt.woodrow) → needinfo?(botond)

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.

  1. 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.

Flags: needinfo?(botond)

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).

Pushed by mwoodrow@mozilla.com:
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

== 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

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.

Status: RESOLVED → VERIFIED
Regressions: 1679681
You need to log in before you can comment on or make changes to this bug.