Closed
Bug 1459890
Opened 7 years ago
Closed 7 years ago
SVG clip-path produces wrong result on filtered element
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | wontfix |
firefox60 | --- | wontfix |
firefox61 | --- | fixed |
firefox62 | --- | fixed |
People
(Reporter: jan.boesenberg, Assigned: botond)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
text/x-review-board-request
|
mstange
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36
Steps to reproduce:
Open the attached SVG or look at this codepen: https://codepen.io/anon/pen/XqVMNR?editors=1000
The SVG consists of a red square on which a blur filter is applied. The result is then clipped to a square that is larger than the bounds of the blurred square.
Actual results:
The blurred red square is not clipped to the specified clip path, but to the narrower bounds of the unfiltered square.
Expected results:
The blurred red square should be completely visible, it should not be clipped.
Reporter | ||
Comment 1•7 years ago
|
||
Without knowing the code, I think what is happening here is that the rendering tries to take a shortcut and reduces the clip path to the element's shape. This makes sens because you would expect that the rendered area cannot be larger than the bounds of the original element.
But what it misses is that due to the blur filter the area that has to be rendered is larger than the original element. Therefore the shortcut produces a wrong result.
Updated•7 years ago
|
status-firefox60:
--- → affected
status-firefox61:
--- → affected
status-firefox62:
--- → affected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → affected
Keywords: regression
Version: unspecified → 58 Branch
Comment 2•7 years ago
|
||
Reporter | ||
Comment 3•7 years ago
|
||
A temporary workaround is to use a mask instead of a clip path.
Assignee | ||
Comment 5•7 years ago
|
||
Confirmed, will investigate.
Assignee: nobody → botond
Flags: needinfo?(botond)
Updated•7 years ago
|
Flags: in-testsuite?
Assignee | ||
Comment 6•7 years ago
|
||
We are computing the bounds of the clip path correctly, but then intersecting it with a rectangle that's already too small (the "bbox" of the clipped content rather than its visual overflow rect).
I discussed this with Markus, and we have a rough idea for a fix, for which I have a WIP draft here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8620e5f31b0776d57e024d6ff1ea5268c2ae0c97
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8975664 -
Flags: review?(longsonr)
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8975664 [details]
Bug 1459890 - Do not clip mask for clip path to bbox of clipped content.
https://reviewboard.mozilla.org/r/243904/#review250036
I think this approach is fine but it would be good to get a review from an SVG peer as well.
Robert: We want to compute the bounds of the clip-path itself, not the bounds for the clipped element. nsSVGClipPathFrame doesn't currently have a method to get just the bounds of the clip path.
However, nsSVGClipPathFrame does have the GetBBoxForClipPathFrame method. But this method intersects the computed bounds with the BBox of the contents. However, while this is the expected behavior for GetBBox, it's not adequate for our use case, because it can result in a rectangle that is too small. That's because the rendering of the contents can extend beyond their BBox, for example due to filters.
Attachment #8975664 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8975664 -
Flags: review?(longsonr) → review+
Assignee | ||
Comment 9•7 years ago
|
||
Unfortunately, it looks like the patch breaks layout/reftests/bugs/600974-2.html on Windows:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=547d2dcdfffe07ae16b358766eaa2f75545c5fb1
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #9)
> Unfortunately, it looks like the patch breaks
> layout/reftests/bugs/600974-2.html on Windows:
Never mind, that looks like it's just an intermittent. I did need to increase the fuzzing a bit for Windows. New Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e8cfb0e2441c08e83c635a2ec0291a5637dc0cf
Comment 11•7 years ago
|
||
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/51cd668c0e87
Do not clip mask for clip path to bbox of clipped content. r=mstange
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
Backout by aciure@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3df5df21892e
Backed out changeset 51cd668c0e87 dev request a=backout CLOSED TREE
Comment 15•7 years ago
|
||
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ed9f0ddabe14
Do not clip mask for clip path to bbox of clipped content. r=mstange
Comment 16•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Reporter | ||
Comment 17•7 years ago
|
||
I can confirm that this issue is solved in 62.0a1. Also, the problem in the original scenario, from which the test case was derived, does not occur anymore. Very nice.
Is there any chance that the fix will make it into Firefox 61?
Assignee | ||
Comment 18•7 years ago
|
||
(In reply to Jan Bösenberg from comment #17)
> Is there any chance that the fix will make it into Firefox 61?
Yes, I will request approval to uplift the fix to 61. As we're early in the cycle, I think its chances are pretty good.
Updated•7 years ago
|
Flags: in-testsuite? → in-testsuite+
Assignee | ||
Comment 19•7 years ago
|
||
Comment on attachment 8975664 [details]
Bug 1459890 - Do not clip mask for clip path to bbox of clipped content.
Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1382534.
[User impact if declined]:
Some SVGs with certain structures, such as a blur filter
inside a clip path, are rendered incorrectly.
[Is this code covered by automated tests?]:
Yes, reftest added in this bug.
[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]:
None.
[Is the change risky?]:
Low risk.
[Why is the change risky/not risky?]:
It's a small, targeted change.
[String changes made/needed]:
None.
Attachment #8975664 -
Flags: approval-mozilla-beta?
Comment 20•7 years ago
|
||
Comment on attachment 8975664 [details]
Bug 1459890 - Do not clip mask for clip path to bbox of clipped content.
Fix for an SVG rendering issue we've been shipping since Fx58. Thanks for including an automated test. Approved for 61.0b7.
Attachment #8975664 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 21•7 years ago
|
||
bugherder uplift |
Comment 22•7 years ago
|
||
This doesn't feel like a must have for ESR60, so wontfixing for that branch. Feel free to reset and request uplift if you disagree.
You need to log in
before you can comment on or make changes to this bug.
Description
•