react-native-svg
react-native-svg copied to clipboard
fix: Fix combining of ClipPath child elements on Android
Summary
Fixes combining of ClipPath child elements to match the spec and browsers, Inkscape, etc. The problem affects both Android and iOS, but this PR currently only fixes it on Android I started looking at the iOS code, but got a bit lost
Relevant issues
- #983
- #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 clipRule
s 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 clipRule
s, 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:
- Path Translation:
SVGKit
translates the SVG paths specified inclipPath
elements intoCGPath
objects. This includes all the path commands and sub-paths.- Handling clip-rule: The
clip-rule
property in SVG, which can benonzero
orevenodd
, affects how the interior of a path is determined. In Core Graphics, when creating aCGPath
, you can specify the fill rule (kCGPathFillRuleEvenOdd
orkCGPathFillRuleNonZero
). This allowsSVGKit
to respect theclip-rule
specified in the SVG file when creating the mask.- Creating Masks: Once the paths are translated,
SVGKit
uses these paths to createCAShapeLayer
objects or similar constructs, which can act as masks. By setting the path of aCAShapeLayer
and using it as a mask, it can effectively clip the content.- 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.- Applying Masks: The mask is then applied to the relevant layers, effectively clipping them as per the
clipPath
definition.
Related commits
766926f75874dedd7b13dadf65315f7030c4234a a1097b85942c559ebcef6b93fa2ce601434d9c50
Note
This is a breaking change and 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
- [X] 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
@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?
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?
I'm not sure whether it's better to be consistent between Android and iOS or at least fix clipPath on Android
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
.
Here's one that's relevant to this PR:
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 theclip-path
property.This test uses the following elements :
<clipPath>
and the following properties :clip-path
.