1664719 - Optimize handling of shared / compositor clips for picture caching

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task)

Comment 1

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 1 - Select outermost scroll root if no real scroll roots found.

Previously, if we encounter a page where there are no real scroll
roots, WR would attach the picture cache slices to the root reference
frame.

This means that in some cases, no shared clips will be found for the
content slices. This is not currently a performance issue, since the
content slices are fixed and don't scroll. However, future patches
for this bug will only select shared clips that are ancestors of
the picture cache scroll root.

This patch ensures that in these cases, we still select the outermost
defined scroll frame, allowing shared clips to be correctly found
for these pages with a single scroll frame that doesn't scroll.

Comment 2

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 2 - Select ancestor clips as shared clips only.

Instead of selecting any rectangle clips as potential shared clips
for content in a picture cache slice, only select clips that are
positioned by an ancestor spatial node of the picture cache
scroll root.

This ensures we don't incorrectly select rectangle clips on the
primitive as potential shared clips, which will be important in
the follow up patches for this bug.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f66cec1aee94
Pt 1 - Select outermost scroll root if no real scroll roots found. r=nical

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ea4a897ea2f
Pt 2 - Select ancestor clips as shared clips only. r=nical

Comment 5

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/f66cec1aee94
https://hg.mozilla.org/mozilla-central/rev/0ea4a897ea2f

Comment 6

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 3 - Ensure that we don't use nested iframes as scroll roots.

This is a rare edge case, but it affects which clips are able to be considered
as shared clips. Fixing this makes the shared clips more likely to be consistent,
which will be important for the follow up patches in this bug. This mostly
only affects Gecko when running with Fission enabled.

Comment 7

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 4 - Create a new slice if shared clips are incompatible.

This patch changes the picture cache logic to create a new slice
if it finds a primitive with an incompatible set of shared clips.

In testing, I have been unable to find any real world content that
hits this case. There is a single test case in CI that hits this
case, which confirms that a new slice is created correctly. Due
to this, it should have minimal effect on real world browsing.

The benefit of this is that we know the set of shared clips won't
change as we continue to build the scene. The follow up patch to
this can then filter out and remove shared clips from primitives
during scene building, rather than frame building.

Comment 8

[Florin Strugariu [:Bebe]]

== Change summary for alert #26948 (as of Wed, 16 Sep 2020 15:23:49 GMT) ==

Improvements:

5% raptor-tp6-twitter-firefox-cold fcp linux64-shippable-qr opt webrender 213.71 -> 203.67
3% raptor-tp6-yandex-firefox-cold fcp linux64-shippable-qr opt webrender 315.50 -> 306.25
3% raptor-tp6-youtube-firefox-cold fcp linux64-shippable-qr opt webrender 456.21 -> 443.00
3% raptor-tp6-bing-firefox-cold fcp linux64-shippable-qr opt webrender 287.75 -> 279.50
2% raptor-tp6-yahoo-mail-firefox-cold fcp linux64-shippable-qr opt webrender 400.00 -> 391.17

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26948

Comment 9

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 2a - Re-enable is_ancestor check.

This was disabled temporarily to fix a regression. It was resolved
by 1665805, which fixes an unrelated bug in the CoreAnimation
implementation due to coordinate accuracy.

Comment 10

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 5 - Remove check for fixed clips when creating slices.

This is no longer useful since we create a separate slice when the
shared_clip config changes. Users who want to prefer subpixel AA
over (significant) performance improvements can enable the config
value `gfx.webrender.quality.force-subpixel-aa-where-possible'.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8102686489e
Pt 2a - Re-enable is_ancestor check. r=nical

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c90418894bcf
Pt 3 - Ensure that we don't use nested iframes as scroll roots. r=nical

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/c8102686489e
https://hg.mozilla.org/mozilla-central/rev/c90418894bcf

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/994c13da52dc
Pt 4 - Create a new slice if shared clips are incompatible. r=nical

Comment 15

[Natalia Csoregi [:nataliaCs]]

Backed out for wrench failures on text-fixed-slice.yaml

backout: https://hg.mozilla.org/integration/autoland/rev/d4bf04fc963a58831b9a6b220cee493493446eaf

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=994c13da52dc80ad8996c059ab052e8b3cf56797&selectedTaskRun=fqXg80bPQyW118q_UMTlBA.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=316444169&repo=autoland&lineNumber=2558

[task 2020-09-23T03:21:48.922Z] REFTEST reftests/text/raster-space-snap.yaml == reftests/text/raster-space-snap-ref.yaml
[task 2020-09-23T03:21:49.205Z] REFTEST reftests/text/text-fixed-slice.yaml == reftests/text/text-fixed-slice-slow.png
[task 2020-09-23T03:21:49.307Z] REFTEST TEST-UNEXPECTED-FAIL | reftests/text/text-fixed-slice.yaml == reftests/text/text-fixed-slice-slow.png | image comparison, max difference: 96, number of differing pixels: 4077 | 4077 num_differences > 0 and <= 255 (allowed 0);

Comment 16

[Glenn Watson [:gw]]

Fixed up the wrench test case that failed. Doing another try push (https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd78efa78c9a5d04c1b0275f41ef8d69060994f9) before re-landing. Thanks!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80bd0d474dbb
Pt 4 - Create a new slice if shared clips are incompatible. r=nical

Comment 18

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/80bd0d474dbb

Comment 19

[Glenn Watson [:gw]]

Attachment: Bug 1664719 - Pt 4 - Refactor complex stacking context clip detection.

Previously, we relied on walking the built clip-chain for a
stacking context to determine if it needed an intermediate surface
due to presence of a complex clip.

However, future patches in this bug need to delay clip-chain creation
for a primitive until the tile-cache slice for the primitive is
known. This change means we don't rely on the clip-chain early
during push_stacking_context (but should have no functional change).

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/329e0bb968c1
Pt 4 - Refactor complex stacking context clip detection. r=nical

Comment 21

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/329e0bb968c1

Comment 22

[Andrew McCreight [:mccr8]]

Should this bug get closed now, or is there further remaining work? Thanks.

Comment 23

[Glenn Watson [:gw]]

It can be closed - there is still more work to be done here, but we can open other bugs for that at the time.