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

Status: RESOLVED FIXED

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

Description

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 1

[Glenn Watson [:gw]]

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.

Comment 2

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 1 - Support prefer compositor flag in wrench.

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.

Comment 3

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 2 - Refactor some of the tile map handling code.

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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e1ffcbadfb2
Part 1 - Support prefer compositor flag in wrench. r=nical

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1e1ffcbadfb2

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c0eb80974b0
Part 2 - Refactor some of the tile map handling code. r=nical,Bert

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0c0eb80974b0

Comment 8

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 3 - Abstract some parts of yuv shader logic.

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bde7c898937d
Part 3 - Abstract some parts of yuv shader logic. r=nical

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/bde7c898937d

Comment 11

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 4 - Refactor more parts of brush yuv shader.

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

Comment 12

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 5 - Support compositor surfaces for draw compositor mode.

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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b22c07ea7c72
Part 4 - Refactor more parts of brush yuv shader. r=nical

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b22c07ea7c72

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/238379209ef4
Part 5 - Support compositor surfaces for draw compositor mode. r=Bert

Comment 16

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 6 - Support an opaque/alpha native surface per slice.

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.

Comment 17

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/238379209ef4

Comment 18

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 7 - Fix UV rect calculation for external textures.

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.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6814870342ef
Part 6 - Support an opaque/alpha native surface per slice. r=Bert

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f267ed8f2e3
Part 7 - Fix UV rect calculation for external textures. r=Bert

Comment 21

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 8 - Remove overlay tiles, they can be alpha tiles instead.

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.

Comment 22

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 9 - Optimize compositor surface overlays.

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.

Comment 23

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/6814870342ef
https://hg.mozilla.org/mozilla-central/rev/8f267ed8f2e3

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38b7f4761f50
Part 8 - Remove overlay tiles, they can be alpha tiles instead. r=nical

Comment 25

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/38b7f4761f50

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6dc6b38288c
Part 9 - Optimize compositor surface overlays. r=Bert

Comment 27

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/f6dc6b38288c

Comment 28

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 10 - Fix incorrect skipping of composites.

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.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f2a8eb49897
Part 10 - Fix incorrect skipping of composites. r=Bert

Comment 30

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 11 - Refactor how external surfaces are composited.

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.

Comment 31

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/5f2a8eb49897

Comment 32

[Glenn Watson [:gw]]

Attachment: Bug 1579235 - Part 12 - Support native compositor surfaces.

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

Comment 33

[Glenn Watson [:gw]]

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.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4bbb79cbf9cc
Part 11 - Refactor how external surfaces are composited. r=Bert,sotaro

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c3a689bf10e1
Part 12 - Support native compositor surfaces. r=Bert,sotaro

Comment 36

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/4bbb79cbf9cc
https://hg.mozilla.org/mozilla-central/rev/c3a689bf10e1

Comment 37

[Glenn Watson [:gw]]

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.