Closed Bug 1459890 Opened 7 years ago Closed 7 years ago

SVG clip-path produces wrong result on filtered element

Categories

(Core :: SVG, defect)

58 Branch
defect
Not set
normal

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)

Attached image clip-issue.png (deleted) —
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.
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.
Keywords: regression
Version: unspecified → 58 Branch
A temporary workaround is to use a mask instead of a clip path.
ni?ing the patch author.
Flags: needinfo?(botond)
Confirmed, will investigate.
Assignee: nobody → botond
Flags: needinfo?(botond)
Flags: in-testsuite?
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
Attachment #8975664 - Flags: review?(longsonr)
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+
Attachment #8975664 - Flags: review?(longsonr) → review+
Unfortunately, it looks like the patch breaks layout/reftests/bugs/600974-2.html on Windows: https://treeherder.mozilla.org/#/jobs?repo=try&revision=547d2dcdfffe07ae16b358766eaa2f75545c5fb1
(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
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
Backout by aciure@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3df5df21892e Backed out changeset 51cd668c0e87 dev request a=backout CLOSED TREE
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
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
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?
(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.
Flags: in-testsuite? → in-testsuite+
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 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+
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.

Attachment

General

Creator:
Created:
Updated:
Size: