SVG filters seem to mess with the transformation matrix origin of their target element

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bugmail, Assigned: u459114)

Tracking

({regression, testcase})

Trunk
mozilla55
regression, testcase
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54+ fixed, firefox55 fixed)

Details

Attachments

(4 attachments)

(Reporter)

Description

2 years ago
Created attachment 8842598 [details]
simple testcase

Applying a filter to an SVG element seems to alter its transformation origin (center coordinates of the transformation). This is a regression from between 2017-01-05 and 2017-02-25. I'll try to narrow down the regression window as soon as I find some time. But if someone beats me to it, be my guest. :-)

The two circles in the testcase should both be drawn in the center of the image. The green one on top of the red one, so red should actually be hidden (except for its "corona").
tracking-firefox54: --- → ?
Flags: needinfo?(cku)
Removing the "regressionwindow-wanted" keyword since the regression window was provided in comment 1.
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
Keywords: regressionwindow-wanted
Tracking 54+ for this SVG regression.
tracking-firefox54: ? → +
(Assignee)

Comment 4

2 years ago
I will try to fix it in this week
Assignee: nobody → cku
Flags: needinfo?(cku)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1341672
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8843872 - Flags: review?(mstange)
Attachment #8843873 - Flags: review?(mstange)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8843873 [details]
Bug 1343664 - Part 2. Reftest.

https://reviewboard.mozilla.org/r/117498/#review119486

::: layout/reftests/svg/filters/filter-transform-01.svg:15
(Diff revision 2)
> + <circle r="15" fill="red" filter="url(#blur)" transform="skewX(0) skewY(0)"/>
> + <circle cx="-10" cy="-10" r="15" fill="red" filter="url(#blur)" transform="translate(10, 10)"/>
> + <circle cx="-10000" cy="-10000" r="15" fill="red" filter="url(#blur)" transform="translate(10000, 10000)"/>
> + <circle r="10" fill="red" filter="url(#blur)" transform="scale(1.5, 1.5)"/>
> +
> + <!-- Cicles above should be completely covered by the next one. -->

You want is "The circles" rather than "Cicles"
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 years ago
mozreview-review-reply
Comment on attachment 8843873 [details]
Bug 1343664 - Part 2. Reftest.

https://reviewboard.mozilla.org/r/117498/#review119486

> You want is "The circles" rather than "Cicles"

Fixed. Thanks.

Comment 13

2 years ago
mozreview-review
Comment on attachment 8843872 [details]
Bug 1343664 - Part 1. Correct transform matrix.

https://reviewboard.mozilla.org/r/117496/#review123100

Sorry, it took me quite a while to explain to myself that this order of matrix operation was correct. I am convinced now.

Here's how I explained it to myself:
If you have a vector in filter space, and want to transform it into DrawTarget device space, then the calculation that is going to be performed is:
vectorDev = vectorFilter * translationInFilterSpace * scaleFromFilterSpaceToUseSpace * userSpaceToDeviceSpaceDTTransform,
so our adjustments need to be multiplied from the left to the DT's old matrix.
Attachment #8843872 - Flags: review?(mstange) → review+

Comment 14

2 years ago
mozreview-review
Comment on attachment 8843873 [details]
Bug 1343664 - Part 2. Reftest.

https://reviewboard.mozilla.org/r/117498/#review123114
Attachment #8843873 - Flags: review?(mstange) → review+

Updated

2 years ago
status-firefox55: --- → affected
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 17

2 years ago
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fe02ae88f611
Part 1. Correct transform matrix. r=mstange
https://hg.mozilla.org/integration/autoland/rev/44e9eace508a
Part 2. Reftest. r=mstange

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fe02ae88f611
https://hg.mozilla.org/mozilla-central/rev/44e9eace508a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 19

2 years ago
Comment on attachment 8843872 [details]
Bug 1343664 - Part 1. Correct transform matrix.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1224207
[User impact if declined]: A graphic object with filter applied may be painted on the wrong place if that object also has transform attribute.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no.
[Why is the change risky/not risky?]: change is simple, all reftest are pass.
[String changes made/needed]: N/A

Please considerate uplift both Part 1 and Part 2 patch.
Attachment #8843872 - Flags: approval-mozilla-aurora?
Comment on attachment 8843872 [details]
Bug 1343664 - Part 1. Correct transform matrix.

Fix an SVG regression. Aurora54+.
Attachment #8843872 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
cjku: can you take a look at the reftests like Tomcats-MacBook-Pro-820:mozilla-central Tomcat$ vim layout/reftests/svg/filters/reftest.list
Tomcats-MacBook-Pro-820:mozilla-central Tomcat$ hg revert -a
forgetting layout/reftests/svg/filters/filter-transform-01.svg
reverting layout/reftests/svg/filters/reftest.list
Tomcats-MacBook-Pro-820:mozilla-central Tomcat$ hg resolve --mark
(no more unresolved files)
continue: hg graft --continue
Flags: needinfo?(cku)
(Assignee)

Comment 22

2 years ago
Created attachment 8849488 [details] [diff] [review]
Part 2 on Aurora
Flags: needinfo?(cku)
(Assignee)

Comment 23

2 years ago
Hi Tomcat,
Please use attachment 8849488 [details] [diff] [review] instead. Sorry for inconvenience.
(Assignee)

Comment 24

2 years ago
ni in case.
Flags: needinfo?(cbook)

Comment 26

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/1a53aec4fb0d
status-firefox54: affected → fixed
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.