1686654 - Reenable active SVG images

Status: RESOLVED FIXED

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

Description

[Robert Longson [:longsonr]]

Originally implemented in bug 1555356 but preffed off in bug 1684625 under the pref gfx.webrender.svg-images

Bug 1684625 includes reftests for a couple of the regressions caused by bug 1555356 (those regressions have been marked fixed by bug 1555356) but the others ought to be looked into too before the pref is set to true.

Comment 1

[Nicolas Silva [:nical]]

Attachment: Bug 1686654 - Ensure SwitchItem is called at the start of each blob group. r=jrmuizel

It was previously only called where we create a stacking context helper: before entering the grouping code and before pushing an active item into the wr display list, and not between the active items and the next blob group. This caused the leaf clip rect of the active item to leak into the next blob group.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2233117f1193
Ensure SwitchItem is called at the start of each blob group. r=jrmuizel

Comment 3

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/2233117f1193

Comment 4

[Nicolas Silva [:nical]]

The patch that landed was just dependency to reenabling active svg images.

Comment 5

[Nicolas Silva [:nical]]

Attachment: Bug 1686654 - Reenable active SVG images. r=#gfx-reviewers

Flipping this pref lets us use WebRender display items for a subset of SVG images and rectangles.

Comment 6

[Nicolas Silva [:nical]]

Attachment: Bug 1686654 - Adjust reftest expectations. r=#gfx-reviewers

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60d43ba90696
Adjust reftest expectations. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/9af11a2b5add
Reenable active SVG images. r=gfx-reviewers,jrmuizel

Comment 8

[Sandor Molnar[:smolnar]]

Backed out 2 changesets (bug 1686654) for causing mochitest and reftest failures.

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

Push with failures

Failure log mochitest

Failure log reftest

Comment 9

[Nicolas Silva [:nical]]

The failing mochitest appears to be caused by the fact that we end up ignoring the HitTestInfo display item just before the active rectangle because we act on it during the PaintItemRange loop but we don't get to PaintItemRange in an empty item group (which is the case here the item group before the active rectangle only contains the hit test info item so it has an empty painted rect.

I'm not sure how this is supposed to work but I suspect that the hit test info item applies to the actually painted item directly after it and so we probably have to check when making an item active whether the previous item is a hittest info item and if so push a wr hittest display item.

Comment 10

[Nicolas Silva [:nical]]

Attachment: Bug 1686654 - Push hit test items for active items that are split off a group. r=jrmuizel

... if the group had hit test flags set.

This is more of a workaround than a proper fix. The general idea is that if we hadn't made the item acitve, its bound would have included in the group's hit tested bounds. Making the item active reduces the area that is covered by hit test items and we don't want that.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/235f5e5c65f8
Push hit test items for active items that are split off a group. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/1e888c5964a4
Adjust reftest expectations. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/414066984e1e
Reenable active SVG images. r=gfx-reviewers,jrmuizel

Comment 12

[Noemi Erli[:noemi_erli]]

Backed out 3 changesets (Bug 1686654) for causing reftest and mochitest failures

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=414066984e1e821d626f78da998d91a3bdebcadc&selectedTaskRun=SSTa0Ec9S02V6ZwdIPx43Q.0

Log: https://treeherder.mozilla.org/logviewer?job_id=374692352&repo=autoland&lineNumber=13991
https://treeherder.mozilla.org/logviewer?job_id=374691891&repo=autoland&lineNumber=18601
https://treeherder.mozilla.org/logviewer?job_id=374688706&repo=autoland&lineNumber=26629

Backout: https://hg.mozilla.org/integration/autoland/rev/07ca6569a753cb8d24841aec155c1ae91b9b354a

Comment 13

[Nicolas Silva [:nical]]

Attachment: Bug 1686654 - Track CompositorHitTestInfo accross blob groups. r=jrmuizel

Before this patch the hit test info is accumulated in the painting loop of a group, there are two issues with that:

• We can early out before getting to the painting loop if the group does not contain any visible item (hit test info items don't count as visible) so a nsDisplayCompositorHitTestInfo can be ignored if it is between two active items.
• Group boundaries should not affect the behavior of hit testing.

With this patch, hit test info is accumulated in the ConstructItem loops, is not reset in EndGroup and is carried over from a group to the next.
This means the hit test flags are extended to the scope of the svg container, I'm not entirely sure that it's correct but I believe it is at least less incorrect than the current behavior.

Depends on D143130

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b20169526ef
Push hit test items for active items that are split off a group. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/3da39fb722c1
Adjust reftest expectations. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/2225497ca573
Reenable active SVG images. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/950227a48351
Track CompositorHitTestInfo accross blob groups. r=jrmuizel

Comment 15

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7b20169526ef
https://hg.mozilla.org/mozilla-central/rev/3da39fb722c1
https://hg.mozilla.org/mozilla-central/rev/2225497ca573
https://hg.mozilla.org/mozilla-central/rev/950227a48351

Comment 16

[Andrej (:aglavic)]

== Change summary for alert #34116 (as of Fri, 13 May 2022 16:46:05 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
43%	tsvgr_opacity	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	145.55 -> 83.47
43%	tsvgr_opacity	linux1804-64-shippable-qr	e10s fission stylo webrender	148.17 -> 85.15
39%	tsvgr_opacity	windows10-64-shippable-qr	e10s fission stylo webrender-sw	111.25 -> 67.63
39%	tsvgr_opacity	windows10-64-shippable-qr	e10s fission stylo webrender-sw	110.89 -> 67.41
37%	tsvgr_opacity	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	88.04 -> 55.35
...	...	...	...	...
8%	tsvg_static	linux1804-64-shippable-qr	e10s fission stylo webrender	80.14 -> 73.56

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34116

Comment 17

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #14)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b20169526ef
Push hit test items for active items that are split off a group. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/3da39fb722c1
Adjust reftest expectations. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/2225497ca573
Reenable active SVG images. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/950227a48351
Track CompositorHitTestInfo accross blob groups. r=jrmuizel

== Change summary for alert #34162 (as of Wed, 18 May 2022 05:13:14 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	google-slides loadtime	linux1804-64-shippable-qr	cold webrender	4,631.73 -> 4,456.12
4%	google-slides loadtime	linux1804-64-shippable-qr	cold fission webrender	4,603.23 -> 4,429.08
3%	google-slides loadtime	linux1804-64-shippable-qr	cold fission webrender	4,655.21 -> 4,502.96
2%	google-slides PerceptualSpeedIndex	linux1804-64-shippable-qr	cold fission webrender	868.25 -> 846.67
2%	google-slides ContentfulSpeedIndex	linux1804-64-shippable-qr	cold fission webrender	1,179.83 -> 1,154.83

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34162