1555356 - Google docs images use blobs

Status: RESOLVED FIXED

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

Description

[Nicolas Silva [:nical]]

Images in google docs are wrapped in svg elements which causes us to use a blob image and do unecessary work.

It would be good if we could detect that an SVG element only contains elements we support and avoid the blob fallback (in the case of google docs it is a single image).

Comment 1

[Aria Beingessner [:Gankra]]

looked into this and am currently confused because at least highlight-painted-layers seems to think we're not making a blob for google docs images (even though they're still inside an svg)

currently manually confirming the machinery isn't broken and that there indeed is no blob

Comment 2

[Aria Beingessner [:Gankra]]

Ok so it looks like a naive active-layers approach to this one won't work. The svg in question shows up as a transform containing an opaque(?) nsDisplaySVGGeometry, and not an nsDisplayImage as I'd hoped. I'll need to actually dig into the SVG itself to try to do this.

Comment 3

[Aria Beingessner [:Gankra]]

Attachment: Bug 1555356 - Make images inside of SVGs active.

Comment 4

[Aria Beingessner [:Gankra]]

:aosmond -- don't bother reviewing that yet, mostly just wanted to test that i had pushing patches up for review properly configured :p

Comment 5

[Aria Beingessner [:Gankra]]

Ok this is ready to land, but I'm holding off until after we cut the beta, so that we have room to test this in nightly.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a8a36e50541
Make images inside of SVGs active. r=aosmond

Comment 7

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 3a8a36e50541 (Bug 1555356) for reftest failures at pattern-big-image.html.
https://hg.mozilla.org/integration/autoland/rev/d9ce692efa0dedcc6c91a403f6ea6393377321a1

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=3a8a36e5054164cceb1b518bdae364c3f87c2838&selectedJob=288802735

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=288802735&repo=autoland&lineNumber=6911

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6550b62f1fe7
Make images inside of SVGs active. r=aosmond

Comment 9

[Aria Beingessner [:Gankra]]

just slightly fuzzier on mac (+2 pixels)

Comment 10

[Alexandru Michis [:malexandru]]

Backed out changeset 6550b62f1fe7 (Bug 1555356) for causing reftest failures in SVG files.

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

Pushes with failures:

• https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cdebug%2Creftests%2Ctest-windows10-64-qr%2Fdebug-reftest-e10s-1%2Cr%28r1%29&tochange=65204cac7dc47b303fe7e6234105b9609c12cef0&fromchange=b7672602baaacbed97eef8eb0c8254d7b1cd3ad6&selectedJob=289004045
• https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cdebug%2Creftests%2Ctest-windows10-64-qr%2Fdebug-reftest-e10s-4%2Cr%28r4%29&fromchange=8baaed81dd2949e0797546fa64ca3a09e14f7121&tochange=65204cac7dc47b303fe7e6234105b9609c12cef0&selectedJob=289000621

Failure logs:

• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=289000621&repo=autoland&lineNumber=36567
• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=289000656&repo=autoland&lineNumber=54563

Comment 11

[Florin Strugariu [:Bebe]]

== Change summary for alert #24964 (as of Mon, 17 Feb 2020 12:14:53 GMT) ==

Improvements:

11% tsvgx windows10-64-shippable-qr opt e10s stylo 183.66 -> 163.28
10% tsvgx windows10-64-shippable-qr opt e10s stylo 184.07 -> 165.22

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

And the backout:

== Change summary for alert #24960 (as of Mon, 17 Feb 2020 10:47:17 GMT) ==

Regressions:

9% tsvgx windows10-64-shippable-qr opt e10s stylo 170.04 -> 185.89

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

Comment 12

[Aria Beingessner [:Gankra]]

Issues from backout:

• minor fuzzy bump on a specific platform
• exposing pre-existing Bug 1616326

For the latter, i'm inclined to mark this test as failing under WR until the root bug is fixed, and pressing along with this patch

Comment 13

[Marco Castelluccio [:marco]]

This is not a regression, the regression in tsvgx was caused by the backout of this patch.

Alexis, did you make a decision about comment 12?

Comment 14

[Aria Beingessner [:Gankra]]

I am currently working on fixing the root issue, it's just a bit of a mess in there.

Comment 15

[Aria Beingessner [:Gankra]]

I believe the patch here is ultimately correct, but Bug 1616326 needs to be fixed before we reland this.

Comment 16

[Bert Peers [:bpeers]]

I've rebased this and did not see any regressions locally (Windows at 100% DPI), with reftest reftest-sanity, reftest svg (both enable-webrender) nor wrench reftest (-p1). Here's a try run.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a86dcd6715c9
Make images inside of SVGs active. r=aosmond

Comment 18

[Cristian Brindusan [:cbrindusan]]

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

Comment 19

[Florin Strugariu [:Bebe]]

== Change summary for alert #25975 (as of Sat, 16 May 2020 10:56:27 GMT) ==

Improvements:

12% raptor-tp6-slides-firefox-cold fcp windows10-64-shippable-qr opt 1,424.58 -> 1,248.33
10% raptor-tp6-slides-firefox-cold fcp linux64-shippable-qr opt 1,381.75 -> 1,247.00
3% raptor-tp6-slides-firefox-cold windows10-64-shippable-qr opt 1,246.98 -> 1,207.00
3% raptor-tp6-slides-firefox-cold linux64-shippable-qr opt 1,290.08 -> 1,257.69

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

Comment 20

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

== Change summary for alert #25984 (as of Mon, 18 May 2020 05:27:32 GMT) ==

Improvements:

14% tsvgx linux64-shippable-qr opt e10s stylo 247.31 -> 211.67
13% tsvgx linux64-shippable-qr opt e10s stylo 247.31 -> 214.56
6% tsvg_static linux64-shippable-qr opt e10s stylo 42.71 -> 39.95

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

Comment 22

[Jeff Muizelaar [:jrmuizel]]

I wonder if we should consider backing this out given the high number of regressions.

Comment 23

[Robert Longson [:longsonr]]

Attachment: Backout Bug 1555356 for multiple regressions

Comment 24

[Robert Longson [:longsonr]]

backout looks like this...

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

Comment 25

[Robert Longson [:longsonr]]

bug 1684625 puts this code path under a pref that disables it. That fixes the regressions. I've checked in some reftests as part of that bug so if this code path is to be re-enabled they'd need to be fixed.