1538969 - High Fill Ratio (3.0) playing back YouTube video on FireTV 4k stick

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

In a local firefox-for-firetv build with a local GeckoView viewing a 1920x1080p 30fps youtube vp9 video (HW decode), I enabled draw-fps. the Fill Ratio was 3.0/3.0 (I saw one blip of 3.5/3.5 I think, but it was pretty stable). Build was ~1.5ms, Exec was ~1.5-2.0ms generally.

This may be related to why composite's are often taking >33ms as seen in https://perfht.ml/2UaN9zg

Comment 1

[Markus Stange [:mstange]]

The high fill ratio can be also reproduced by running a video in https://www.youtube.com/tv in Firefox on Desktop.

Comment 2

[Matt Woodrow (:mattwoodrow)]

I can reproduce a value of 1.9 when playing on desktop (with the controls overlay hidden, 2.4 with it visible), which is still bad, but closer.

If anyone could grab a layer tree dump (layers.dump=true, writes to stdout) from the firetv case that would be super useful.

For the desktop case, it appears that the problem is that the page is using opacity:0 to hide the controls overlay, but we're still creating a transparent layer for them. The layer ends up being entirely transparent, but the compositor doesn't know that, so it's doing a draw call with a texture containing all 0s.

We build display items within opacity:0 containers, since we need the hit testing info for APZ, and then we cull out normal items during FrameLayerBuilder.

The problem is that the various nsDisplayItem::GetBounds implementations aren't aware that some of their descendants might get removed, so they're returning the bounds including all items. This is what FrameLayerBuilder uses to decide how much space to allocate within a layer (and happens before we descend into a container item to look at the actual children).

In this case the outermost item of the overlay is an inactive transform, so we're making space in the PaintedLayer for the transform, and then descending into to it to handle the actual contents, and find out there are none.

Some possible solutions:

• Refactor FLB to handle inactive layers differently, such that we adjust the visible region of the PaintedLayer after we've handled inner items, and use the bounds of those inner items, not the display-list bounds. Not sure how hard this is yet.

• Make nsDisplayItem::GetBounds aware of opacity:0 culling, and have a parameter specifying if we're inside a container that needs it. Lots of code to change, including items that compute their bounds at initialization and return a cached result.

• Move the opacity:0 culling to item time creation, so that everything is correct from the start. Probably the nicest end-state, but we'd have to touch every DL creation call sites (hundreds!) to handle fallible item creation.

• Implement nsDisplayItem::IsUniform() to return true for opacity:0 such that this code path [1] catches it. We'd also need to implement it for nsDisplayTransform to detect the case where the transform contains only opacity:0 children (as is the case here). This is a simple patch to write, but unless we write complex code for every possible container type, then it probably just fixes this specific example.

As a side note, it looks like the culling code will remove containers within opacity:0. That seems buggy if there were hit-test items within the inner container.

[1] https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/layout/painting/FrameLayerBuilder.cpp#3838

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1538969 - Report an opaque surface format for AndroidSurfaceTextureData when it's being used for an opaque video. r?jya

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1538969 - Handle non-integer transforms when doing occlusions in PostProcessLayers. r?mstange

Depends on D25185

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1538969 - Report nsDisplayVideo as being opaque when possible so that we can occlude content behind it. r?jya

Depends on D25186

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2332a2fb02fa
Report an opaque surface format for AndroidSurfaceTextureData when it's being used for an opaque video. r=jya
https://hg.mozilla.org/integration/autoland/rev/911386e724bc
Handle non-integer transforms when doing occlusions in PostProcessLayers. r=mstange
https://hg.mozilla.org/integration/autoland/rev/c8e85a802c77
Report nsDisplayVideo as being opaque when possible so that we can occlude content behind it. r=jya

Comment 7

[Cosmin Sabou [:CosminS]]

Backed out 3 changesets (Bug 1538969) for wpt reftests failures on transform3d-backface-visibility-001,2,3.

Push with failures:https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=c8e85a802c77d13a8dc2f20414a0fd5ab608e04a&searchStr=windows%2C10%2Cx64%2Cdebug%2Cweb%2Cplatform%2Ctests%2Cwith%2Ce10s%2Ctest-windows10-64%2Fdebug-web-platform-tests-reftests-e10s-1%2Cw-e10s%28wr1%29&selectedJob=238984620

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=238984620&repo=autoland&lineNumber=30268

Backout link: https://hg.mozilla.org/integration/autoland/rev/5d179e192915637fd52b5cbb786b9348d74c6b82

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c8055acb67b
Report an opaque surface format for AndroidSurfaceTextureData when it's being used for an opaque video. r=jya
https://hg.mozilla.org/integration/autoland/rev/3f2ce3a4c4e6
Handle non-integer transforms when doing occlusions in PostProcessLayers. r=mstange
https://hg.mozilla.org/integration/autoland/rev/c2b1753a416b
Report nsDisplayVideo as being opaque when possible so that we can occlude content behind it. r=jya

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4c8055acb67b
https://hg.mozilla.org/mozilla-central/rev/3f2ce3a4c4e6
https://hg.mozilla.org/mozilla-central/rev/c2b1753a416b

Comment 10

[Chris Peterson [:cpeterson]]

67=wontfix? If this graphics problem is only known to affect youtube.com/tv, then I assume we don't need to uplift it to Fennec 67 Beta. Firefox for Fire TV is not using GeckoView yet.