Closed Bug 1579235 Opened 7 months ago Closed 22 days ago

YouTube is drawn into the picture cache and then onto the screen

Categories

(Core :: Graphics: WebRender, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: jrmuizel, Assigned: gw)

References

(Blocks 7 open bugs)

Details

Attachments

(12 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

During video playback we end wastefully drawing into the picture cache and then drawing to the screen. This wastes our precious bandwidth.

The current plan is to fix this by separating out video into a separate layer that doesn't have picture caching.

Blocks: wr-intel
Blocks: 1569767
Assignee: nobody → gwatson
Blocks: 1460499
Blocks: 1536360
Whiteboard: [wr-q41]
See Also: → 1594454

Implementing this should be a reasonably small patch (in theory, just need to add picture cache slice tags to primitive clusters where there are video elements).

However, since it has the potential to cause some unexpected regressions (most likely, losing subpixel AA on some pages where we don't expect to), I am planning to implement this in the first week after the next soft freeze, so it has time to bake in nightly for a few weeks.

Blocks: 1601297
Depends on: 1610738
Whiteboard: [wr-q41]
Blocks: wr-win7
Keywords: leave-open

Add support to the yaml reader and writer to be able to specify
that a primitive should set the PREFER_COMPOSITOR_SURFACE flag.

This flag is not currently used, but in future will signal the
picture caching code to promote a primitive to draw on a native
compositor surface where possible.

Simplify some of the logic related to handling multiple
compositor surfaces in future, specifically:

  • Only rebuild the tiles map when the tile rect changes.
  • Remove need for tiles_to_draw array.
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e1ffcbadfb2
Part 1 - Support prefer compositor flag in wrench. r=nical
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c0eb80974b0
Part 2 - Refactor some of the tile map handling code. r=nical,Bert

Factor some parts of the YUV brush shader out into a shared
yuv.glsl shader include.

In future, this shader code will also be referenced by the
composite.glsl shader when using the simple (Draw) compositing
mode, to composite video surfaces directly into the framebuffer
where possible.

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bde7c898937d
Part 3 - Abstract some parts of yuv shader logic. r=nical
See Also: → 1616185

Move the UV writing function into the shared yuv.glsl include so
that the YUV compositing shader will be able to access it.

This patch adds support for YUV images to be promoted to compositor
surfaces when using the simple (draw) compositor mode. A follow up
to this will extend support to native compositor implementations.

We rely on only promoting compositor surfaces that are opaque
primitives. With this assumption, the tile(s) that intersect the
compositor surface get a 'cutout' in the location where the
compositor surface exists, allowing that tile to be drawn as
an overlay after the compositor surface.

Tiles are only drawn in overlay mode if there is content that
exists on top of the compositor surface. Otherwise, we can draw
the tiles in the normal fast path before the compositor surface
is drawn.

The patch also introduces a new subpixel AA mode, which allows
subpixel rendering to be enabled conditionally as long as the
text run does not intersect some number of excluded rectangle
regions. In this way, subpixel text remains on most of the page,
but is disabled for elements that are drawn into transparent
regions of the tile where the compositor surface exists.

This allows video playback to be composited directly into the
framebuffer, without invalidation of content tiles, which can
save a significant amount of battery power and improve performance.

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b22c07ea7c72
Part 4 - Refactor more parts of brush yuv shader. r=nical
Depends on: 1617808
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/238379209ef4
Part 5 - Support compositor surfaces for draw compositor mode. r=Bert

Previously, a native compositor surface was considered to be
completely opaque, or completely translucent. This is due to
a limitation in how alpha is handled in the DirectComposition
API level.

With this patch, each picture cache slice maintains both an
opaque and translucent native surface handle. Tiles are assigned
to one of those surfaces based on their current opacity.

This is a performance optimization in some cases, since:

  • Even if part of a cache is translucent, opaque tiles can
    still participate in occlusion at the compositor level.
  • If a tile is changing from opaque to translucent, it now
    invalidates only that tile, rather than the entire surface.

The primary benefit of this patch is that it allows compositor
surfaces to be drawn sliced in between the opaque surface and
any overlay / alpha tiles.

This patch fixes an oversight in part 5 of this patch series that
could result in an incorrect UV rect being used for an external
texture that uses a custom UV rect.

When the texture is an external texture, the UV rect is not known when
the external surface descriptor is created, because external textures
are not resolved until the lock() callback is invoked at the start of
the frame render. To handle this, query the texture resolver for the
UV rect if it's an external texture, otherwise use the default UV rect.

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6814870342ef
Part 6 - Support an opaque/alpha native surface per slice. r=Bert
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8f267ed8f2e3
Part 7 - Fix UV rect calculation for external textures. r=Bert

A previous patch in this series introduced overlay tiles. However,
now that native surfaces exist for for the opaque and alpha tiles
within a slice, we can remove the overlay tiles array and add
these special tiles to the alpha surface.

This patch improves the performance of compositor surfaces in
two ways:

(1) Ignore primitives behind the first compositor surface when
determining whether a tile needs to be moved to the overlay
(alpha) pass. This means WR only moves a tile to the alpha
pass when it has primitives that overlap with the compositor
surface bounding rect, and are ordered after that compositor
surface. In practice, this means most tiles are able to
remain in the fast (opaque) path. Typically, a small number
of tiles that contain overlay video controls are moved to the
alpha pass.

(2) Register the opaque compositor surfaces as potential occluders.
This allows tiles that are completely covered by a compositor
surface to be removed from the compositor visual tree, which
helps both the simple and native compositor modes.

Between them, these optimizations typically mean that when watching
video in full-screen, nothing is composited except the video surface
itself, and some small region(s) where video overlay controls are
currently active.

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/38b7f4761f50
Part 8 - Remove overlay tiles, they can be alpha tiles instead. r=nical
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f6dc6b38288c
Part 9 - Optimize compositor surface overlays. r=Bert

Ensure that the image keys and image generations for external
compositor surfaces are included in the composite descriptor,
which is used to determine if a composite is required or can
be skipped.

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f2a8eb49897
Part 10 - Fix incorrect skipping of composites. r=Bert

This patch refactors how external surfaces are stored in the
CompositeState structure. This is primarily to simplify integration
with native compositor mode, but also simplifies the Draw compositor
path.

Previously, the ResolvedExternalSurface struct contained information
that was used to rasterize the external surface (YUV planes etc) and
also the information to composite it (device rect, clip rect, z_id).

Now, ResolvedExternalSurface contains just the information required
to rasterize the external surface, while the compositing information
is handled by adding the external surface as a regular tile. This
makes it possible to unify how external surfaces are drawn, via the
common draw_tile_list method.

This patch adds support for external compositor surfaces when
using native compositor mode.

With patch 12, that should complete everything required to close this bug (except flicking the switch to enable it).

I'd like to revisit some of these patches as a follow up and restructure them a bit, but this should cover all the functionality for the initial version of video compositor surfaces.

There is still future work for more performance / optimizations, such as (1) adding support for hardware decoded native surfaces directly into a DC swapchain (2) disable z-buffer when updating native compositor surfaces.

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bbb79cbf9cc
Part 11 - Refactor how external surfaces are composited. r=Bert,sotaro
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c3a689bf10e1
Part 12 - Support native compositor surfaces. r=Bert,sotaro

This is fixed now that https://bugzilla.mozilla.org/show_bug.cgi?id=1617808 has landed, which enables the functionality in this patch series. There are likely to be some regressions to fix, but we can open them as separate bugs.

Status: NEW → RESOLVED
Closed: 22 days ago
Resolution: --- → FIXED
Duplicate of this bug: 1594454
You need to log in before you can comment on or make changes to this bug.