1323962 - CSS transforms are not supported in indirectly rendered things such as masks, markers, patterns or clip-paths

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Description

[u459114]

Bug 1118710 comment 13, Robert's comment:
CSS transforms are not supported in indirectly rendered things such as masks, patterns or clip-paths because the rendering is done outside of a display list.

Comment 1

[u459114]

Attachment: mask-transform.html

Comment 2

[u459114]

I think the correct solution might be to build up display list and paint that list(using nsLayoutUtils::PaintFrame) inside nsSVGMaskFrame::GetMaskForMaskedFrame

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

How does this bug differ from bug 1118710?

Comment 6

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Possible test case:

https://codepen.io/thebabydino/pen/vjWdPx

Comment 7

[Arnaud Brousseau]

This bug came up in an email discussion and seems to be a good first step towards solving bug#878346. Quoting Robert Longson:

In any case if I was going to do something about putting transforms into CSS the first thing to solve would be bug
1323962 otherwise when you map attribute transforms to CSS, mask, clipPath etc transforms will stop working altogether.

Can anyone pair up with me and point me in the right direction? I'm new to Firefox's codebase but I'm interested in learning and taking on this bug before moving on to bug#878346.

A few questions (feel free to point me to an MDN page if it's already answered somewhere):

• is the general approach in comment#2 still valid? Anything missing?
• There is a test case in comment#6: should I work on converting this to an automated test?
• what's the best workflow to iterate on a branch like this? Do you typically use debuggers? Lean on (automated) tests? Manual tests?

Comment 8

[Robert Longson [:longsonr]]

(In reply to Arnaud Brousseau from comment #7)

• is the general approach in comment#2 still valid? Anything missing?

comment 2 is valid

• There is a test case in comment#6: should I work on converting this to an automated test?

no, the bug's current attachment would make a good reftest though.

• what's the best workflow to iterate on a branch like this? Do you typically use debuggers? Lean on (automated) tests? Manual tests?

up to you really.

Comment 9

[Brian Birtles (:birtles)]

(In reply to Arnaud Brousseau from comment #7)

• what's the best workflow to iterate on a branch like this? Do you typically use debuggers? Lean on (automated) tests? Manual tests?

You can certainly use debuggers (folks on Linux love rr) but since you are going to want to submit automated tests with your patches anyway, when possible I often like to write a few automated tests first and check they fail.

Where possible we try to make tests that can be synchronized with web-platform-tests (see testing/web-platform/tests/...). You might find there are already some tests there that are failing because of this.

You can run such tests using "./mach test <path>".

Tests that are expected to fail are annotated with a corresponding file in testing/web-platform/meta/...

Comment 12

[violet.bugreport]

Attachment: Bug 1323962 - Should support CSS transform for mask

SVGElement::PrependLocalTransformsTo doesn't take CSS transform into
account, we should use nsIFrame::GetTransformMatrix instead.

Comment 13

[violet.bugreport]

Attachment: Bug 1323962 - Should support CSS transform for pattern

The same as mask.

Comment 14

[Robert Longson [:longsonr]]

This is all great stuff Violet. I thought we'd need to convert to display lists but this is much simpler than I imagined it would be.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be0c4eed11e4
Should support CSS transform for mask r=longsonr
https://hg.mozilla.org/integration/autoland/rev/141cbcdbcef8
Should support CSS transform for pattern r=longsonr

Comment 17

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/be0c4eed11e4
https://hg.mozilla.org/mozilla-central/rev/141cbcdbcef8

Comment 18

[violet.bugreport]

Attachment: Bug 1323962 - Should support CSS transform for clip-path

<clipPath> can itself have SVG transform, thus we need to override
nsIFrame::IsSVGTransformed() method so that layout code will be aware
of the SVG transform.

The remaining is similar to <mask>

Comment 19

[violet.bugreport]

Attachment: Bug 1323962 - PaintSVG for container should consider CSS transform for its children

Container manages SVG/CSS transform of its children, thus PaintSVG of container
should take children's CSS transform into account.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29ad5178399d
Should support CSS transform for clip-path r=longsonr
https://hg.mozilla.org/integration/autoland/rev/03ea838144b5
PaintSVG for container should consider CSS transform for its children r=longsonr

Comment 22

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/29ad5178399d
https://hg.mozilla.org/mozilla-central/rev/03ea838144b5

Comment 23

[hughesm02]

The repro case from 1539698 confirmed working in mozilla68! Thanks for fixing this!

Comment 26

[Rachel Andrew]

This is flagged dev-doc-needed and is on my list to look at, I'm not sure however what needs changing in the MDN docs due to this, other than a note in the release notes. Is there anything else I should be adding?