1435291 - background-image: url(svg) uses blob fallback

Status: RESOLVED FIXED

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

Description

[Jeff Muizelaar [:jrmuizel]]

<img src="svg"> doesn't use blob image, neither should background-image: url(svg).

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1435291-Avoid-falling-back-to-blob-images-for-ba.patch, v1

It looks straightforward enough, but on closer inspection, I'm basically reverting bug 1394711, which seems to have landed due to try failures which may or may not still be present. Let's test and see:

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

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1435291-Allow-VectorImage-IsImageContainerAvaila.patch, v2

Not sure where the previous patch came from, my brain failed. The real problem is that VectorImage::IsImageContainerAvailable fails while VectorImage::IsImageContainerAvailableAtSize works. Really all of the WebRender paths should be using the latter, but it appears nsCSSRendering::CanBuildWebRenderDisplayItemsForStyleImageLayer uses it because it doesn't have the image size until later on. We should probably fix that in this patch as well, but for now, this is useful for seeing if any tests fail.

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

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1435291-Make-background-SVGs-use-WebRender-inste.patch, v3

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

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8953572 [details] [diff] [review]
0001-Bug-1435291-Make-background-SVGs-use-WebRender-inste.patch, v3

Sadness, try is not green. Those tests pass locally for me, but only when screen scaling is in effect (which is my default config).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef8ec604139
Part 1. Enable shared surfaces by default with WebRender. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0409a405f2
Part 2. Make background SVGs use WebRender instead of fallback. r=jrmuizel

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Pulsebot from comment #5)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef8ec604139
> Part 1. Enable shared surfaces by default with WebRender. r=jrmuizel
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0409a405f2
> Part 2. Make background SVGs use WebRender instead of fallback. r=jrmuizel

Ugh. I verbally got an r+ for part 1, but completely forgot I never did leave part 2 up long enough for the review! Backout out part 2.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1435291-Make-background-SVGs-use-WebRender-inste.patch, v4

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

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4058fc040cfe
Backed out changeset 7f0409a405f2 .

Comment 9

[Jan Alexander Steffens [:heftig]]

Attachment: svg-bg.html

Is this the same issue that makes the attached test page slow?

Comment 10

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/4ef8ec604139

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8958592 [details] [diff] [review]
0001-Bug-1435291-Make-background-SVGs-use-WebRender-inste.patch, v4

Review of attachment 8958592 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/wr/WebRenderLayerManager.h
@@ +62,5 @@
>    virtual KnowsCompositor* AsKnowsCompositor() override;
>    WebRenderLayerManager* AsWebRenderLayerManager() override { return this; }
>    virtual CompositorBridgeChild* GetCompositorBridgeChild() override;
>  
> +  virtual int32_t GetMaxTextureSize() const override { return INT32_MAX; }

Add a comment saying that WebRender handles chopping up images larger than the max texture size already.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd37857e52a1
Part 2. Make background SVGs use WebRender instead of fallback. r=jrmuizel

Comment 13

[Andreea Pavel [:apavel]]

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

Comment 14

[Ionuț Goldan [:igoldan]]

Big perf wins! \o/

== Change summary for alert #12116 (as of Tue, 13 Mar 2018 17:59:25 GMT) ==

Improvements:

 57%  Heap Unclassified linux64-qr opt stylo     1,874,862,511.83 -> 810,652,099.54
 51%  Explicit Memory linux64-qr opt stylo       2,274,934,306.27 -> 1,105,364,108.11
 27%  Resident Memory linux64-qr opt stylo       3,804,494,658.91 -> 2,792,104,519.97

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