Closed Bug 1538969 Opened 6 months ago Closed 5 months ago

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

Categories

(Core :: Graphics: Layers, defect)

68 Branch
Unspecified
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: jesup, Assigned: mattwoodrow)

References

(Depends on 1 open bug)

Details

(Whiteboard: [gvtv:p1] [media-q2])

Attachments

(3 files)

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

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

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

OS: Unspecified → Android
See Also: → 1535511
Whiteboard: [gvtv] → [gvtv:p1]
Version: 52 Branch → 68 Branch
Depends on: 1539673
Assignee: nobody → matt.woodrow
Whiteboard: [gvtv:p1] → [gvtv:p1] [media-q2]
Pushed by mwoodrow@mozilla.com:
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
Pushed by mwoodrow@mozilla.com:
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
Flags: needinfo?(matt.woodrow)

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.

Regressions: 1543944
You need to log in before you can comment on or make changes to this bug.