fix: Fix combining of ClipPath child elements

[wodin]

Summary

Fixes combining of ClipPath child elements to match the spec and browsers, Inkscape, etc.
The problem affected both Android and iOS - both are now fixed.

Relevant issues

• ClipPath appears to be broken #983
• Intersection of SVG to clip path results in addition in Android and subtraction (*BUG) in iOS #1520

The code was conflating clipRules with how child elements need to be combined. These are unrelated. Children always need to be combined using UNION

From https://www.w3.org/TR/SVG11/masking.html#EstablishingANewClippingPath

When the ‘clipPath’ element contains multiple child elements, the silhouettes of the child elements are logically OR'd together to create a single silhouette which is then used to restrict the region onto which paint can be applied.

iOS

After spending some more time trying to understand the iOS code it looks to me like it does not keep track of the individual clipRules on the ClipPath's child elements when adding them together. I believe this is part of the problem.
As mentioned above, all of the child elements' silhouettes need to be OR'd (or UNION'd) together. The clipRule affects what the silhouette of each child element looks like. So one needs to keep track of the individual clipRules, or else somehow calculate the silhouette while adding the child elements together.

Given that the SVG spec says the following, maybe this can be implemented in terms of masks.

A clipping path can be thought of as a mask wherein those pixels outside the clipping path are black with an alpha value of zero and those pixels inside the clipping path are white with an alpha value of one (with the possible exception of anti-aliasing along the edge of the silhouette).

And according to ChatGPT, that is what SVGKit does:

1. Path Translation: SVGKit translates the SVG paths specified in clipPath elements into CGPath objects. This includes all the path commands and sub-paths.
2. Handling clip-rule: The clip-rule property in SVG, which can be nonzero or evenodd, affects how the interior of a path is determined. In Core Graphics, when creating a CGPath, you can specify the fill rule (kCGPathFillRuleEvenOdd or kCGPathFillRuleNonZero). This allows SVGKit to respect the clip-rule specified in the SVG file when creating the mask.
3. Creating Masks: Once the paths are translated, SVGKit uses these paths to create CAShapeLayer objects or similar constructs, which can act as masks. By setting the path of a CAShapeLayer and using it as a mask, it can effectively clip the content.
4. Composite Masks for Multiple Elements: When a clipPath contains multiple elements, SVGKit combines them into a single mask. This is done by rendering each element into an offscreen bitmap and then using this composite image as a mask. The composite operation inherently respects the individual path's fill rules as they were rendered.
5. Applying Masks: The mask is then applied to the relevant layers, effectively clipping them as per the clipPath definition.

Related commits

766926f
a1097b8

Breaking Changes

1. Default clip-rule changed from evenodd to nonzero - Per SVG spec, nonzero is the default. If your clipPaths relied on the previous evenodd default for self-intersecting paths, add clip-rule="evenodd" explicitly.

2. ClipPath children now correctly UNION - Previously, overlapping children could create holes. Now they combine as the spec requires.

The ClipPath example image will need to be updated in USAGE.

Test Plan

yarn test gave the following warning on the main branch (i.e. before my change. My fix is completely unrelated to this file):

/tmp/react-native-svg/src/css/LocalSvg.tsx
  26:34  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

After adding // eslint-disable-next-line @typescript-eslint/no-explicit-any on the previous line, everything is happy with my changes applied:

% yarn test
yarn run v1.22.19
$ npm run lint && npm run tsc

> [email protected] lint
> eslint --ext .ts,.tsx src


> [email protected] tsc
> tsc --noEmit

✨  Done in 5.09s.

yarn jest failed before my change. I ran yarn jest -u on the main branch to update the snapshots.
After that, yarn jest passes on the branch containing my change.

% git checkout wodin/fix-combining-of-clip-path-children
M	__tests__/__snapshots__/css.test.tsx.snap
M	src/css/LocalSvg.tsx
Switched to branch 'wodin/fix-combining-of-clip-path-children'
% yarn jest
yarn run v1.22.19
$ jest
 PASS  __tests__/css.test.tsx
  ✓ inlines styles (6 ms)
  ✓ supports CSS in style element (8 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   2 passed, 2 total
Time:        0.737 s, estimated 1 s
Ran all test suites.
✨  Done in 2.72s.

What's required for testing (prerequisites)?

N/A

What are the steps to reproduce (after prerequisites)?

Code like this reproduces the problem and demonstrates that the fix works:

    <Svg viewBox="0 0 250 234">
      <ClipPath id="clip">
        <Polygon points="31,0 31,234 170,234 250,0" />
        <Rect x="0" y="0" width="41" height="234" rx="8" ry="8" />
        <Rect x="0" y="112" width="250" height="10" />
      </ClipPath>
      <Rect x="0" y="0" width="250" height="234" fill="red" clipPath="url(#clip)" />
    </Svg>

Before the fix the result looks like this:

After the fix it looks like this:

Compatibility

OS	Implemented
iOS	✅
Android	✅

Checklist

• I have tested this on a device and a simulator
• I added documentation in README.md
• I updated the typed files (typescript)
• I added a test for the API in the __tests__ folder

[wodin]

@WoLewicki I would love your thoughts on this if you have any?

Unfortunately I have no experience with the iOS side of this, so struggled to get anywhere with it, and have run out of energy for the moment.

While looking into this I came across the SVG 1.1 Second Edition Test Suite

I wonder whether it would be useful to take the test cases from the test suite and display the RNSVG version and a WebView version next to each other for visual comparison?

[WoLewicki]

I am no expert in this code either unfortunately. As for displaying the web versions of the components next to the original ones, I am up for it 🚀 We also have a web version inside Example app so it can be also seen there. Do you want me to merge this one for start or to wait until the solution on iOS is also found to keep the behavior consistent?

[wodin]

I'm not sure whether it's better to be consistent between Android and iOS or at least fix clipPath on Android

[wodin]

We also have a web version inside Example app so it can be also seen there.

Do you mean react-native-web? OK. Although I think it might be easier to compare them if they're on the same screen

I thought of maybe running svgr during the build process to convert the .svg files from the test suite to components.
I suppose we'd probably have to delete all the ones that contain animation? And other stuff like <set> and <script> etc.

EDIT: It's not quite as straightforward as that, but it sort of works with a bit of tweaking:

RNSVG is on top and the WebView is below it. This one is testing currentColor.

[wodin]

Here's one that's relevant to this PR:

masking-path-01-b.svg

Pass Criteria

The test at the top shows an orange rectangle (with black stroke) being clipped by another rectangle. So only the middle portion of the orange rectangle should be visible. Also the black stroke should only be visible along the top and bottom edge of the rectangle.

The example at the bottom has a group containing a text string and two rectangles. The group has a clipping path defined using two overlapping rectangles. Of concern is the overlapping area shared by the two rectangles. There should not be holes in this overlapping area, the clip region is the union of the two rectangles. For clarity, guide rectangles in grey show the position of the clipping rectangles.

The rendered picture should match the reference image exactly, except for possible variations in the labelling text (per CSS2 rules).

Test Description

Test to see if the basic clipping works using the clipPath element and the clip-path property.

This test uses the following elements : <clipPath> and the following properties : clip-path.

[wodin]

OK @jakex7, I've made more updates for this and added relevant test cases from the W3C SVG test suite.
I have not yet been able to test properly on iOS/macOS. Having some space issues on my machine :-/
I also only tested with paper-example. Let me know if I should also test with fabric-example or other examples?

[wodin]

Hi @jakex7

I managed to run the paper-example tests on iOS (simulator). They look correct visually, but there are minor discrepancies like on Android. Mostly because of font differences. I tried removing all text, except if it was actually part of the tested functionality and discovered that there were still enough minor discrepancies for them to all fail. The main issue is the rectangle around each test not being pixel-identical, so the diff has some thin lines. e.g.:

Those discrepancies are obviously not part of what this PR is trying to address, so will keep them in failedCases.json, unless you have a better suggestion.

Another issue I ran into is that on iOS, the test that includes <image> tags renders fine in the simulator, but the ViewShot capture is missing the images. After a lot of debugging I think what's happening is that the images load asynchronously and ViewShot captures it too soon, so the image size is 0 and they don't render. I tried a 100ms delay before calling ViewShot and that worked sometimes. Do you have any ideas about how this can be handled? Or should I just add it to failedCases.json like the others?

@kacperzolkiewski I see you make a fix to the e2e tests recently. Do you have any suggestions for the above issues?

I am unsure of the license for the W3C tests. The tests themselves contain a reference to their "legal" page, and if you follow the links there you end up here:
https://www.w3.org/copyright/test-suites-licenses/#lic1

According to that, as long as RN SVG does not claim compliance with the spec as a result of passing these modified tests, it seems they should be safe to include in the project and modify them however we like.

However, it also says:

How does this affect test suites published prior to this policy?

It does not affect existing test suites until they are modified to include the new license.

and the text included in the files says "All Rights Reserved."
So I might have to come up with similar test cases instead of using these ones. Or else we'd need to have something that separately downloads the test suite and generates the reference images without actually including them in the project. But even that is probably not allowed if "all rights [are] reserved."

I'll probably see if I can come up with some alternative images to avoid the licensing uncertainty. Maybe it would be best to leave the tests out of this PR.

[wodin]

Hah! Found a bug in the way generateE2eReferences renders this SVG:

SVG

This renders correctly with my fixes and matches what I see in Firefox, Chrome and Inkscape

Reference image

This is the reference image generated by yarn generateE2eReferences

---

Tried a few things and came to the following conclusions:

• Puppeteer: Chrome 127.0.6533.88 (has clip-rule bug)
• Playwright: Chromium 143.0.7499.4 (renders correctly)

[wodin]

I've replaced the W3C tests with new ones and tidied up the branch, so no more test case license uncertainty.

The reference generation is still broken for clip-rule.svg, so I generated those manually using Inkscape, but I verified it looks the same in Chrome (v143) and Firefox as well. It's just the old Chrom v127 used by Puppeteer that's broken.

I also added a test case based on #1520. This one still fails on iOS after the changes because of antialiasing differences.

Other failed cases are unrelated to these changes:

clip-path-shapes (iOS + Android):

• clipPathUnits="objectBoundingBox" not supported - normalized 0-1 coordinates aren't being scaled to element's bounding box

mask-edge-cases (iOS + Android):

• visibility:hidden inside <mask> doesn't hide the element - mask content still renders

I tested on both PaperExample and FabricExample. I discovered that the macOS examples don't support the E2E tests.