Closed Bug 1531890 Opened 9 months ago Closed 9 months ago

SVG: clip path on a clip path doesn't seem to work sometimes

Categories

(Core :: SVG, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jrmuizel, Assigned: heycam)

Details

Attachments

(2 files, 1 obsolete file)

Attached image svg-clipping-perf.svg (obsolete) —

There should be a bunch of green shapes. Firefox draws nothing.

Attached image reduce.svg

Here's a further reduced version.

Attachment #9047819 - Attachment is obsolete: true
Summary: svg-clipping-perf.svg doesn't paint the same as in Chrome/Safari → SVG: clip path on a clip path doesn't seem to work
Summary: SVG: clip path on a clip path doesn't seem to work → SVG: clip path on a clip path doesn't seem to work sometimes

Well it's all about which element you use for userSpaceOnUse units.

The first clip does not have clipPathUnits and so it defaults to userSpaceOnUse units. Firefox uses the userspace of the original object, the rect. It seems that Chrome uses the space of the other clipPath. That's in objectBoundingBox units so it uses those for the referenced clipPath too.

The SVG spec says this:

If clipPathUnits="userSpaceOnUse", the contents of the ‘clipPath’ represent values in the current user coordinate system in place at the time when the ‘clipPath’ element is referenced (i.e., the user coordinate system for the element referencing the ‘clipPath’ element via the ‘clip-path’ property).

So either interpretation is arguable as far as I can see.

Seems like w3c should clarify things here.

Flags: needinfo?(amelia.bellamy.royds)

The downside of Chrome's interpretation is that you can't get back to the original rect units, you're stuck in objectBoundingBox units all the way further down the chain once you encounter one.

The example in reduce.svg isn't a clip-path on a <clipPath>, it's a clip-path on the rect which happens to be inside another clipping path. This rect is the "the element referencing the clipPath" in this context. The user-space for the clipped rect is the coordinate system where it has width and height of 1. So Chrome's rendering is correct, IMO: this is definitely a Firefox bug (although it still wouldn't hurt to add clarification to the spec).

That said:

If it was a case of <clipPath id="clip" clipPathUnits="objectBoundingBox" clip-path="url(#clip2)">...</clipPath>, that's when things would get trickier. The spec doesn't clearly define what is the coordinate system for the clipPath itself. In this case, however, both Firefox and Chrome pass through the original user-space coordinates to the final clip. But Edge doesn't, instead treating it the same as a clip on the scaled contents of the clipping path.

Test case: https://codepen.io/AmeliaBR/pen/fd314d690be3c48a72b238d695db395e

So that one definitely needs a spec clarification, and there is an open bug for it:
https://github.com/w3c/fxtf-drafts/issues/247

Flags: needinfo?(amelia.bellamy.royds)

For attachment 9047962 [details], here is where we set up the clip that applies to the <rect> inside #clip:

https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/layout/svg/nsSVGClipPathFrame.cpp#204

Note that we're passing in aClippedFrame here, which is the actual displayed <rect>. Inside ApplyClipPath, we set up some transforms based on the <circle> and the <clipPath> it's in, but we never do anything with <clipPath id="#clip">'s transform (to take its objectBoundingBox value into account). It's in mMatrixForChildren but we don't use that until we actually paint #clip's children.

I think we should be using mMatrixForChildren instead of aMatrix in that ApplyClipPath call. (And similarly in the clip mask layer case.)

Assignee: nobody → cam
Status: NEW → ASSIGNED
Priority: -- → P3
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88a8be92d7ae
Take objectBoundingBox into account when a clipPath's child has its own clip-path. r=longsonr
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16006 for changes under testing/web-platform/tests
Upstream PR was closed without merging
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.