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

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
21 days ago

People

(Reporter: u459114, Assigned: violet.bugreport)

Tracking

(Blocks 1 bug, {dev-doc-needed})

unspecified
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(5 attachments)

Reporter

Description

3 years ago
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.
Reporter

Comment 1

3 years ago
Posted file mask-transform.html
Priority: -- → P3
Reporter

Updated

3 years ago
Blocks: svg-enhance
Reporter

Comment 2

3 years ago
I think the correct solution might be to build up display list and paint that list(using nsLayoutUtils::PaintFrame) inside nsSVGMaskFrame::GetMaskForMaskedFrame
Duplicate of this bug: 1178297
Reporter

Updated

a year ago
Assignee: cku → nobody
Duplicate of this bug: 1459828

Comment 7

5 months ago

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

Duplicate of this bug: 1523415
Duplicate of this bug: 1533483
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
Assignee

Comment 12

a month ago

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

Assignee

Comment 13

a month ago

The same as mask.

Assignee

Updated

a month ago
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.

Assignee

Updated

a month ago
Keywords: leave-open
Assignee

Updated

a month ago
No longer blocks: 1539698
Duplicate of this bug: 1539698

Comment 16

a month ago
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
Assignee

Comment 18

a month ago

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

Assignee

Comment 19

a month ago

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

Assignee

Updated

a month ago
Duplicate of this bug: 1461289

Comment 21

a month ago
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
Assignee

Updated

a month ago
Keywords: leave-open

Comment 22

a month ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Comment 23

a month ago

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

Assignee

Updated

22 days ago
Duplicate of this bug: 1429841
Assignee

Updated

21 days ago
Duplicate of this bug: 1118710
You need to log in before you can comment on or make changes to this bug.