Closed Bug 1323962 Opened 7 years ago Closed 5 years ago

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

Categories

(Core :: SVG, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: u459114, Assigned: violet.bugreport)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(5 files)

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.
Attached file mask-transform.html
Priority: -- → P3
Blocks: svg-enhance
I think the correct solution might be to build up display list and paint that list(using nsLayoutUtils::PaintFrame) inside nsSVGMaskFrame::GetMaskForMaskedFrame
Assignee: cku → nobody

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?

(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.

(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/...

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

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

The same as mask.

Assignee: nobody → violet.bugreport
Status: NEW → ASSIGNED

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.

Keywords: leave-open
No longer blocks: 1539698
Pushed by violet.bugreport@gmail.com:
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

<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>

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

Pushed by violet.bugreport@gmail.com:
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
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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

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?

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: