Closed Bug 1458968 Opened 2 years ago Closed 2 years ago

Make every nsSVGOuterSVGAnonChildFrame a reference frame

Categories

(Core :: SVG, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: mstange, Assigned: mstange)

References

(Depends on 2 open bugs)

Details

Attachments

(3 files)

The frame tree for an outer <svg> element looks like this:
                                   
> SVGOuterSVG(svg) ... [sc=11e210408]<
>   SVGOuterSVGAnonChild(svg) ... [sc=12d880c08:-moz-svg-outer-svg-anon-child]<
>     [contents]
>   >
> >

In other words, every nsSVGOuterSVGFrame has a single child frame which is an nsSVGOuterSVGAnonChildFrame which wraps the contents of the SVG.

If there is a viewbox transform, the nsSVGOuterSVGAnonChildFrame becomes a reference frame and display list building creates an nsDisplayTransform that wraps its contents.

I propose that we make this the case *even if there is no viewbox transform*: Make the nsSVGOuterSVGAnonChildFrame always a reference frame and always create a nsDisplayTransform for it.
This will have the effect that the offset between this <svg> element and the parent reference frame is completely absorbed by that nsDisplayTransform, and that this offset does not affect the SVG contents' transforms. Consequently, if the <svg> element moves, e.g. during scrolling, the transform matrices of the contents are unaffected.

This simplifies invalidation in two ways:
 - It fixes bug 1165185, which is caused by floating point inaccuracies when comparing transform matrices that move during scrolling. The shift is applied to the two to-be-compared matrices in different orders, and this gives slightly different results.
 - It means that WebRender's special path for SVG painting doesn't need to do any shifting during invalidation - it can just compare everything in the reference frame space and call it a day.
Component: Graphics: WebRender → SVG
Moving the nsDisplaySVGWrapper inside the viewbox transform caused some interesting reftest failures. Some of them seem to be snapping-related, one seems fuzzable, viewport-percent-graphic-user-01.svg looks entirely wrong and there are a few unexpected passes.
Comment on attachment 8973038 [details]
Bug 1458968 - Make the nsSVGOuterSVGAnonChildFrame a reference frame by always returning true from IsSVGTransformed.

https://reviewboard.mozilla.org/r/241568/#review247538
Attachment #8973038 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8973039 [details]
Bug 1458968 - Disable blend-difference-stacking.html because it fails now.

https://reviewboard.mozilla.org/r/241570/#review247540
Attachment #8973039 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8973040 [details]
Bug 1458968 - Create the nsDisplaySVGWrapper item in nsSVGOuterSVGAnonChildFrame, not in nsSVGOuterSVGFrame.

https://reviewboard.mozilla.org/r/241572/#review247542
Attachment #8973040 - Flags: review?(matt.woodrow) → review+
Depends on: 1459339
The test crash is caused by WebRender not handling very large blob images. We basically end up OOMing as we try to create 42 million primitives. This should be helped by https://github.com/servo/webrender/pull/2704
Priority: -- → P3
Depends on: 1461849
layout/reftests/invalidation/fractional-transform-2.html is failing on Windows with this patch
Any ideas mstange why this test would fail and only on Window 10?
Flags: needinfo?(mstange)
(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> Any ideas mstange why this test would fail and only on Window 10?

No, no idea. If you can make a Windows build with --enable-dump-painting (or manually remove the #ifdef MOZ_DUMP_PAINTING lines around the nsLayoutUtils::InvalidationDebuggingIsEnabled() checks), we should be able to find out what causes the invalidations.

Alternatively, since these failures are in new tests, we could land the tests disabled on Windows. But it would still be nice to know why we invalidate.
Flags: needinfo?(mstange)
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30d54bb4cc27
Make the nsSVGOuterSVGAnonChildFrame a reference frame by always returning true from IsSVGTransformed. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/6629dc2614ed
Disable blend-difference-stacking.html because it fails now. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f55d0ffe494
Create the nsDisplaySVGWrapper item in nsSVGOuterSVGAnonChildFrame, not in nsSVGOuterSVGFrame. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/133a13c44abe
Adjust fuzz for webrender tests r=mstange
Depends on: 1467169
Depends on: 1489815
Regressions: 1557245
You need to log in before you can comment on or make changes to this bug.