Closed Bug 1435291 Opened 2 years ago Closed 2 years ago

background-image: url(svg) uses blob fallback

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: jrmuizel, Assigned: aosmond)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

<img src="svg"> doesn't use blob image, neither should background-image: url(svg).
Assignee: nobody → aosmond
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
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
Attachment #8948680 - Attachment is obsolete: true
Blocks: 1439760
Summary: background-image: url(svg) uses blob image → background-image: url(svg) uses blob fallback
Priority: P2 → P1
Attachment #8953572 - Flags: review?(jmuizelaar)
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).
Attachment #8953572 - Flags: review?(jmuizelaar)
Pushed by aosmond@gmail.com:
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
(In reply to Pulsebot from comment #5)
> Pushed by aosmond@gmail.com:
> 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.
Attachment #8953572 - Attachment is obsolete: true
Attached file svg-bg.html
Is this the same issue that makes the attached test page slow?
https://hg.mozilla.org/mozilla-central/rev/4ef8ec604139
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
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.
Attachment #8958592 - Flags: review?(jmuizelaar) → review+
Pushed by aosmond@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd37857e52a1
Part 2. Make background SVGs use WebRender instead of fallback. r=jrmuizel
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
Duplicate of this bug: 1424960
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.