1731136 - WebRender should use one large tile for flat color fills in native layers

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Brad Werth [:bradwerth]]

Bug 1653417 created a heuristic to detect a case when a video power optimization is possible. This heuristic would be greatly simplified or even eliminated if WebRender consistently used a single large native layer to represent a flat color fill (as opposed to a patchwork of memory-optimized tiles).

Comment 1

[Brad Werth [:bradwerth]]

Attachment: Bug 1731136 Part 2: Allow native compositors to handle surface backdrops.

Comment 2

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1731136 Part 2: Make RenderCompositorNative handle flat color on macOS.

Depends on D127175

Comment 3

[Brad Werth [:bradwerth]]

Attachment: Bug 1731136 Part 1: Extend compositor to manage backdrop surfaces.

Comment 4

[Brad Werth [:bradwerth]]

Attachment: Bug 1731136 Part 3: Make the tile_cache track compositor backdrop surfaces.

Depends on D127175

Comment 5

[Brad Werth [:bradwerth]]

Attachment: Bug 1731136 Part 5: Make macOS native compositor and NativeLayerCA handle backdrop

Depends on D128131

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b408a3230170
Part 1: Extend compositor to manage backdrop surfaces. r=gw
https://hg.mozilla.org/integration/autoland/rev/7afa3f4b5a85
Part 2: Allow native compositors to handle surface backdrops. r=gw
https://hg.mozilla.org/integration/autoland/rev/6b5b4b5eb1e9
Part 3: Make the tile_cache track compositor backdrop surfaces. r=gw
https://hg.mozilla.org/integration/autoland/rev/c333a19d8712
Part 4: Make macOS native compositor and NativeLayerCA handle backdrop layers. r=mstange

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/b408a3230170
https://hg.mozilla.org/mozilla-central/rev/7afa3f4b5a85
https://hg.mozilla.org/mozilla-central/rev/6b5b4b5eb1e9
https://hg.mozilla.org/mozilla-central/rev/c333a19d8712

Comment 8

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing Bug 1771465 . CLOSED TREE (backed out on dev's request )

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

Comment 9

[Brad Werth [:bradwerth]]

I've updated D128131 to fix the issue noted in Bug 1771465. I'll wait to re-land the patches until after the merge date.

Comment 10

[Cosmin Sabou [:CosminS]]

Backout reached mozilla-central: https://hg.mozilla.org/mozilla-central/rev/21fcc945a41df3d003af0911451d9c7fc99e48ba

Comment 11

[Brad Werth [:bradwerth]]

Attachment: Bug 1731136 Part 4: Limit backdrops to the visible area.

This calculates the visible area of the TileCache, and compares against that area
for three purposes:

1. Backdrops must cover only the visible area, not the tile coverage area.
2. Primitives only obstruct the backdrop if they appear in the visible area.
3. Backdrop candidates opaque rects are clipped to the visible area.

All these changes ensure that if a tile coverage area is larger than the window,
backdrops are only considered and created for the visible area.

Comment 12

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/ef61304adce3

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b5626a7b82d
Part 1: Extend compositor to manage backdrop surfaces. r=gw
https://hg.mozilla.org/integration/autoland/rev/7dfdcb74d1c1
Part 2: Allow native compositors to handle surface backdrops. r=gw
https://hg.mozilla.org/integration/autoland/rev/e127e41e12da
Part 3: Make the tile_cache track compositor backdrop surfaces. r=gw
https://hg.mozilla.org/integration/autoland/rev/a24467137e9b
Part 4: Limit backdrops to the visible area. r=gw
https://hg.mozilla.org/integration/autoland/rev/d2a56d51c65b
Part 5: Make macOS native compositor and NativeLayerCA handle backdrop r=mstange

Comment 14

[Sandor Molnar[:smolnar]]

Backed out 5 changesets (bug 1731136) for causing reftest failures in layout/reftests/bugs/299136-1.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/47b031489c06cd0c1f48652c4d6d262d400218eb

Push with failures

Failure log

Comment 15

[Brad Werth [:bradwerth]]

This is a surprising failure, with test layout/reftests/bugs/299136-1.html on Windows software WebRender. The failure appears to be in differences in text anti-aliasing. When I run the test locally on macOS, it passes. When I manually load the test page and the ref page, neither of them actually invoke a native color layer. The only changed code that could be getting run on Windows is in D128131 where the empty tile rects are not sent to the compositor. I'm surprised this would make a difference since it is intended to only be an optimization.

I'll run a general try run of all the reftests and see what I can replicate on treeherder. Depending on how widespread these failures are, the fix will either be an update to test expectations, or to remove the "optimization" of omitting empty tile rects in cases where the native color layers won't be used anyway.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d74eb0203332e3ee09ea6107a34c0f2f4feea305

Comment 16

[Brad Werth [:bradwerth]]

(In reply to Brad Werth [:bradwerth] from comment #15)

This is a surprising failure, with test layout/reftests/bugs/299136-1.html on Windows software WebRender. The failure appears to be in differences in text anti-aliasing.

Specifically, the difference is in whether or not the text is eligible for subpixel-AA. With the patches applied, the test file is eligible and the ref file is not. I'm not sure which of the two is changed by the patches. My guess is that we want both to be eligible and the patches have made the ref file ineligible somehow. This doesn't appear to be tied to software WebRender.

Comment 17

[Brad Werth [:bradwerth]]

New theory, with these code changes calculate_subpixel_mode is checking a slightly smaller backdrop.opaque_rect that doesn't completely cover the tiling area, and so it doesn't turn on subpixel mode for the text. This should be fixable by making the comparison only check against the visible portion of the tiling area.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9de0a9fc0a0e
Part 1: Extend compositor to manage backdrop surfaces. r=gw
https://hg.mozilla.org/integration/autoland/rev/215d2e4b1833
Part 2: Allow native compositors to handle surface backdrops. r=gw
https://hg.mozilla.org/integration/autoland/rev/05d7f11bf1c9
Part 3: Make the tile_cache track compositor backdrop surfaces. r=gw
https://hg.mozilla.org/integration/autoland/rev/06c57c0af21d
Part 4: Limit backdrops to the visible area. r=gw
https://hg.mozilla.org/integration/autoland/rev/ebe5a430029f
Part 5: Make macOS native compositor and NativeLayerCA handle backdrop r=mstange

Comment 19

[bszekely]

https://hg.mozilla.org/mozilla-central/rev/9de0a9fc0a0e
https://hg.mozilla.org/mozilla-central/rev/215d2e4b1833
https://hg.mozilla.org/mozilla-central/rev/05d7f11bf1c9
https://hg.mozilla.org/mozilla-central/rev/06c57c0af21d
https://hg.mozilla.org/mozilla-central/rev/ebe5a430029f