Bug 41428

Summary: clip-path does not work inside mask element
Product: WebKit Reporter: rspierer
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: krit, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
test case
none
Patch
none
Patch v2
none
Patch v3 krit: review+

Description rspierer 2010-06-30 14:06:12 PDT
Created attachment 60148 [details]
test case

The figure is gray circle instead of black astroid.
Comment 1 Dirk Schulze 2010-07-01 05:18:28 PDT
Looks like a bug in clipPath. The example works as expected, if clipPath just has one circle. We create a ImageMask if clipPath has more than one shape for clipping. This masking operation seems to be the problem. I could just test it on Chromium/Linux. Did you test it on Safari?
Comment 2 rspierer 2010-07-01 09:26:55 PDT
I tested it on:
 Safari 5.0 (7533.16) on WinXP - gray filled circle
 Opera 10.54 (3423) on WinXP - no problems
 Google Chrome 5.0.375.86 on WinXP - no problems
 Firefox 3.6.6 on Win2003 - no problems

Opera chokes on the following modified test case:

<svg xmlns="http://www.w3.org/2000/svg"
     xmlns:xlink="http://www.w3.org/1999/xlink">

<clipPath id="clip" clipPathUnits="objectBoundingBox">
 <circle cx="0" cy="0" r="0.5" />
 <circle cx="0" cy="1" r="0.5" />
 <circle cx="1" cy="0" r="0.5" />
 <circle cx="1" cy="1" r="0.5" />
</clipPath>

<defs>
 <linearGradient id="grad">
  <stop offset="0" stop-color="black" />
  <stop offset="1" stop-color="white" />
 </linearGradient>
 
 <mask id="gmask" x="0" y="0" width="1" height="1" maskContentUnits="objectBoundingBox"> 
  <rect x="0" y="0" width="1" height="1" fill="url(#grad)" />
 </mask>

 <mask id="mask" x="0" y="0" width="1" height="1" maskContentUnits="objectBoundingBox">
  <rect x="0" y="0" width="1" height="1" fill="white" />
  <rect x="0" y="0" width="1" height="1" fill="black" clip-path="url(#clip)" />
 </mask>
</defs>

<g mask="url(#gmask)">
 <circle cx="50%" cy="50%" r="50" fill="black" mask="url(#mask)" />
</g>
</svg>
Comment 3 Dirk Schulze 2010-07-02 01:50:59 PDT
I checked the wokring path of svg mask and clippath. It turns out, that both, clipPath as well as Mask don't have mistakes. It works correctly. The real problem is the following: We create intermediate ImageBuffers for the mask, as well as clipPath. The size of this buffers depend on the userSpace of the references object. If the object is very small (like for objects with relative params), the ImageBuffer is very small too. The result gets scaled at the end. This can cause strange behavior like on the example of above.

We have to make sure, that the used ImageBuffer are as big as the end result in device pixels.
Comment 4 Nikolas Zimmermann 2010-08-22 03:10:05 PDT
Created attachment 65053 [details]
Patch

Integrated the testcase within the patch + 7 more complex tests.
Comment 5 Nikolas Zimmermann 2010-08-22 03:38:56 PDT
Created attachment 65056 [details]
Patch v2

Fixed Dirks comments from IRC.
Comment 6 Nikolas Zimmermann 2010-08-22 03:41:48 PDT
(In reply to comment #2)
> I tested it on:
>  Safari 5.0 (7533.16) on WinXP - gray filled circle
>  Opera 10.54 (3423) on WinXP - no problems
>  Google Chrome 5.0.375.86 on WinXP - no problems
>  Firefox 3.6.6 on Win2003 - no problems
> 
> Opera chokes on the following modified test case:

> 
> <g mask="url(#gmask)">
>  <circle cx="50%" cy="50%" r="50" fill="black" mask="url(#mask)" />
> </g>
> </svg>

I noticed the same, Opera has problems when using <g mask="..> <rect mask="...
Comment 7 Nikolas Zimmermann 2010-08-22 04:26:17 PDT
Comment on attachment 65056 [details]
Patch v2

Clearing review flag, the tests have issues when panning.
The clamping of the absolute target rects interfere with the "currentContentTransformation"... need to think about this a little more, and create additional tests covering panning&zooming on deep nested masks/clippers.
Comment 8 Nikolas Zimmermann 2010-08-24 01:11:20 PDT
Created attachment 65227 [details]
Patch v3

Resolved all issues with zoom & pan. No regressions, but beautiful progressions :-)
Comment 9 Dirk Schulze 2010-08-24 01:39:46 PDT
Comment on attachment 65227 [details]
Patch v3

WebCore/ChangeLog:24
 +          (WebCore::RenderSVGResourceClipper::applyClippingToContext): Moved some code from createClipData, to avoid having to pass 5 arguments to createClipDAta.
createClipData

Great patch. r=me.
Comment 10 Nikolas Zimmermann 2010-08-24 04:22:12 PDT
Landed in r65880.