Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClipPath appears to be broken #983

Closed
mstiggle opened this issue Mar 29, 2019 · 24 comments
Closed

ClipPath appears to be broken #983

mstiggle opened this issue Mar 29, 2019 · 24 comments

Comments

@mstiggle
Copy link

Hello! I'm trying to use ClipPath to cut a circle in a rect in order to highlight what is underneath that circle. Issue #936 covers this and there is a handy answer that says this code should do it:

import React from 'react';
import { View } from 'react-native';
import Svg, { Defs, ClipPath, Circle, Rect } from 'react-native-svg';

export default class App extends React.Component {
  render() {
    return (
      <View
        style={{
          flex: 1,
          alignContent: 'center',
          justifyContent: 'center',
          backgroundColor: '#ecf0f1',
        }}
      >
        <Svg width="100%" height="100%" viewBox="0 0 100 100">
          <Defs>
            <ClipPath id="clip_out" clipRule="evenodd">
              <Rect width={100} height={100}/>
              <Circle cx={50} cy={50} r={25}/>
            </ClipPath>
          </Defs>
          <Rect width={100} height={100} fill="green" clipPath="url(#clip_out)" />
        </Svg>
      </View>
    );
  }
}

Unfortunately the circle does not get cut out. It seems that ClipRule may be failing, but I'm not sure. Here is a snack that shows the behavior.

@msand
Copy link
Collaborator

msand commented Mar 31, 2019

It seems this has been broken on iOS since this commit which first came in v7.0.0: a1097b8
Handling of clipRule doesn't seem to work correctly at all.

Android seems to work since 766926f

I've released patch version of the v6, v7 and v8 branches, such that they work with the latest react-native version. If you're in need of clipRule evenodd I recommend you use v6.5.3 or the v6 branch, or make a fix and open a pull request. Those two commits touch all the most essential parts relating to this issue, and how it was fixed on android.

@msand
Copy link
Collaborator

msand commented Mar 31, 2019

Another workaround is to use a single Path in the ClipPath:
Correction, this is the spec conformant way to get nonzero clip-rule. But, setting it to evenodd on the Path element inside the ClipPath doesn't affect anything in react-native-svg right now, it needs to be set on the ClipPath element, which isn't part of the svg spec, as it only allows it to affect anything on elements which are direct descendants of a clip-path element, if I understand it correctly.

import React from 'react';
import { View } from 'react-native';
import Svg, { Defs, ClipPath, Path, Rect } from 'react-native-svg';

export default class App extends React.Component {
  render() {
    return (
      <View
        style={{
          flex: 1,
          alignContent: 'center',
          justifyContent: 'center',
          backgroundColor: '#ecf0f1',
        }}
      >
        <Svg width="100%" height="100%" viewBox="0 0 200 200">
          <Defs>
            <ClipPath id="clip">
              <Path
                d="M0 0 H200 V200 H0 z
                 M 100, 100
                 m -75, 0
                 a 75,75 0 1,0 150,0
                 a 75,75 0 1,0 -150,0"
              />
            </ClipPath>
          </Defs>
          <Rect width={200} height={200} clipPath="url(#clip)" />
        </Svg>
      </View>
    );
  }
}

msand added a commit that referenced this issue Mar 31, 2019
@msand
Copy link
Collaborator

msand commented Mar 31, 2019

I think I've fixed this in the develop branch: 5389209
Could you test it?
I'm not completely sure if this breaks something else. Would need more test cases in end-to-end tests.

@msand
Copy link
Collaborator

msand commented Mar 31, 2019

Seems at least this example might not conform to the spec now: #752
This probably requires making a few more examples and comparing to browsers, etc.
I guess ios will require similar logic as android, enabling to take the union of the paths/geometries.

msand added a commit that referenced this issue Apr 1, 2019
@msand
Copy link
Collaborator

msand commented Apr 1, 2019

Think I fixed it in a way which doesn't break the other case now. But I realized it's not fully spec conformant either. I've also added support for using the Use element inside ClipPath now. But the implementation of clipRule should probably be revised to only affect elements inside ClipPath. The behavior actually breaks the spec, and doesn't work in browsers. You would need to use a single path with clip-rule / fill-rule to do it in a web-compatible and spec conformant way:

import React from 'react';
import { View } from 'react-native';
import Svg, {
  Defs,
  ClipPath,
  Path,
  Rect,
  G,
  Text,
  Polygon,
  Use,
} from 'react-native-svg';

const SvgComponent = props => (
  <Svg width="100%" height="30%" viewBox="0 0 140 110" {...props}>
    <Defs>
      <Path
        id="prefix__a"
        d="M25 10a20 20 0 1 0 0 40 20 20 1 1 0 0-40m20 0a20 20 0 1 0 0 40 20 20 1 1 0 0-40"
      />
      <ClipPath id="prefix__b" clipRule="nonzero">
        <Use xlinkHref="#prefix__a" clipRule="nonzero" />
      </ClipPath>
      <ClipPath id="prefix__c" clipRule="evenodd">
        <Path
          id="prefix__a"
          y={50}
          d="M25 10a20 20 0 1 0 0 40 20 20 1 1 0 0-40m20 0a20 20 0 1 0 0 40 20 20 1 1 0 0-40"
        />
      </ClipPath>
      <ClipPath id="prefix__d" clipRule="evenodd">
        <Use xlinkHref="#prefix__a" clipRule="evenodd" y={50} />
      </ClipPath>
    </Defs>
    <Path clipPath="url(#prefix__b)" fill="#6495ed" d="M0 5h70v50H0z" />
    <Text x={15} y={54} fontSize={5} fill="#fff">
      {'non-zero clip-rule'}
    </Text>
    <Use xlinkHref="#prefix__a" x={70} stroke="#6495ed" />
    <Text x={86} y={54} fontSize={5} fill="#fff">
      {'non-zero fill-rule'}
    </Text>
    <G>
      <Path clipPath="url(#prefix__d)" fill="#daa520" d="M0 55h70v50H0z" />
      <Text x={14} y={105} fontSize={5} fill="#fff">
        {'even-odd clip-rule'}
      </Text>
    </G>
    <G>
      <Use
        fillRule="evenodd"
        xlinkHref="#prefix__a"
        x={70}
        y={50}
        stroke="#daa520"
      />
      <Text x={85} y={105} fontSize={5} fill="#fff">
        {'even-odd fill-rule'}
      </Text>
    </G>
  </Svg>
);

const SvgComponent2 = props => (
  <Svg height="90" width="100" viewBox="0 0 100 90">
    <Defs>
      <Path d="M50,0 21,90 98,35 2,35 79,90z" id="star" fill="white" />
      <ClipPath id="emptyStar" clipRule="evenodd">
        <Use href="#star" clipRule="evenodd" />
      </ClipPath>
      <ClipPath id="filledStar" clipRule="nonzero">
        <Use href="#star" clipRule="nonzero" />
      </ClipPath>
    </Defs>
    <Rect clipPath="url(#emptyStar)" width="50" height="90" fill="blue" />
    <Rect
      clipPath="url(#filledStar)"
      width="50"
      height="90"
      x="50"
      fill="red"
    />
  </Svg>
);

const SvgComponent3 = ({ clipRule = 'nonzero' }) => (
  <Svg width="50%" height="500">
    <Defs>
      <ClipPath id="windowClip" clipRule={clipRule}>
        <G>
          <Rect x={30} y={50} height={100} width={100} />
          <Text x={30} y={70} fontSize={40}>
            Hello world
          </Text>
          <Rect x={30} y={200} height={100} width={100} />
          <Rect x={100} y={250} height={100} width={100} />
          <Rect x={30} y={370} height={100} width={100} />
          <Polygon
            transform="translate(70,400)"
            points="0,0 0,100 100,120 120,0"
          />
        </G>
      </ClipPath>
    </Defs>
    <Rect height="100%" width="100%" fill="#0af" clipPath="url(#windowClip)" />
  </Svg>
);

export default class App extends React.Component {
  render() {
    return (
      <View
        style={{
          flex: 1,
          alignContent: 'center',
          justifyContent: 'center',
          backgroundColor: '#ecf0f1',
        }}
      >
        <SvgComponent />
        <SvgComponent2 />
        <View
          style={{
            flex: 1,
            flexDirection: 'row',
          }}
        >
          <SvgComponent3 />
          <SvgComponent3 clipRule="evenodd" />
        </View>
      </View>
    );
  }
}

@msand
Copy link
Collaborator

msand commented Apr 1, 2019

Screenshot_1554082987
Simulator Screen Shot - iPhone XR - 2019-04-01 at 04 43 15

@msand
Copy link
Collaborator

msand commented Apr 1, 2019

Can you please test the latest commit in the develop branch: https://github.com/react-native-community/react-native-svg/tree/develop
e.g.

yarn add react-native-svg@react-native-community/react-native-svg#76b8b82

@msand
Copy link
Collaborator

msand commented Apr 1, 2019

@msand
Copy link
Collaborator

msand commented Apr 1, 2019

I'm thinking of suggesting to add support for clip-rule on the clip-path element for svg 2.0 or later. This seems like a very useful thing, and much more efficient than using a mask.

@msand
Copy link
Collaborator

msand commented Apr 1, 2019

Noticed a caching bug in android, fixed in latest commit of develop branch:
Screenshot_1554138231

@mstiggle
Copy link
Author

mstiggle commented Apr 3, 2019

The latest commit on the dev branch (fde018b) is working beautifully! At least on iOS. I'll test on android in a bit but I imagine it will be good too. Thank you!!

@mstiggle
Copy link
Author

mstiggle commented Apr 3, 2019

I initially thought everything was going fine on android, but actually our app's performance has plummeted on android only. We've upgraded a lot of things recently, but after intensive debugging we were able to narrow it down to this library. Something is causing the app to freeze/die with no error messages. As we disabled more and more Svg components the app stayed alive longer, but after enough clicking around it would freeze/die again. I doubt it's a new issue with the development branch, but I will test it on master soon.

I'm sure you'll want a reproduction so after we confirm it's happening on master we'll see if we can figure out how to get a repro.

@msand
Copy link
Collaborator

msand commented Apr 3, 2019

Oh, sorry to hear. Yeah a reproduction would certainly make a huge difference in potential to fix it. I suspect it's related to handling of layout / bounds calculations and communicating this to android and react-native such that the gesture responder system would work correctly. There's probably some calls happening which aren't needed. If you make a good reproduction, it's probably possible to git bisect to find the commits causing the regression, and it'll likely be related to the gesture and bounds work. What version(s) did you use before?

@msand
Copy link
Collaborator

msand commented Apr 3, 2019

Also, measuring where the time is spent using android studio might reveal what is causing it.

@mstiggle
Copy link
Author

mstiggle commented Apr 4, 2019

We spent a lot of time in the profiler and really haven't been able to track much down. The heap seemed to just be overwhelmed easily which would sometimes lead to crashes and sometimes lead to the app still technically being alive, but being completely unresponsive. The app would run much better with js debugging enabled because of chrome's better resources.

We did a big upgrade with this library as well as many others (including RN). The heap seemed to be getting overwhelmed by graphics, so our prime suspects are our RN's Animated or this lib.

As a temporary fix we've just upped our heap size for android. We'll still be investigating and trying to make performance enhancements, but the app at least runs now.

Thank you for the quick fixes for clipRule!

@mstiggle
Copy link
Author

We've figured out a bunch of things on our end for android and our world is no longer crumbling. We upgraded to your latest (9.4.0) in the process and clipPath is up and running again. Thank you for the quick fixes on this! I'm going to close :)

@mstiggle
Copy link
Author

Ahh, actually it looks like it's again broken for android on 9.4.0. Same behavior as before, the circle isn't getting cut out. Works great on iOS though.

@mstiggle mstiggle reopened this Apr 19, 2019
@mstiggle
Copy link
Author

As you can see in the snack from the beginning of this thread (https://snack.expo.io/SywHux3uE), clipPath is working for android on v9.3.6 which is what expo uses, but it is broken for iOS on that version. The reverse is true for v9.4.0

@mstiggle
Copy link
Author

Any ideas what might be going on?

@msand
Copy link
Collaborator

msand commented Apr 25, 2019

At least it seems to work correctly in plain react-native with react-native-svg 9.4.0, are you sure you used the latest version / cleaned and rebuilt to native code and reset js cache?

@msand msand closed this as completed Sep 28, 2019
@thiskevinwang
Copy link

ClipPath (with Rect) appears to be broken again, on iOS.

  • working fine on Android
"expo": "~41.0.1",
"react-native-svg": "12.1.0",
"react-native": "https://github.com/expo/react-native/archive/sdk-41.0.0.tar.gz",

@Loovery
Copy link

Loovery commented Apr 28, 2022

ClipPath doesn't work on Android, but it works great on iOS.


// after change width 100 to 200, nothing happen, but it works on iOS

<Defs>
  <ClipPath id="testClipPath">
    <Rect width={width} height={height} />
  </ClipPath>
</Defs>

<Path d={myPath} clipPath="url(#testClipPath)" fill="#000" />


or

<Defs>
  <ClipPath id="testClipPath">
    <Path d={myPath} />
  </ClipPath>
</Defs>

<Rect clipPath="url(#testClipPath)" width={width} height={height} fill="#000" />

UPDATE:

Solution - clipRule={Platform.isAndroid ? 'nonzero' : 'evenodd'}

<Defs>
  <ClipPath id="testClipPath">
    <Path d={myPath} />
  </ClipPath>
</Defs>

<Rect clipRule={Platform.isAndroid ? 'nonzero' : 'evenodd'} clipPath="url(#testClipPath)" width={width} height={height} fill="#000" />

JackWillie added a commit to JackWillie/react-native-svg that referenced this issue Nov 27, 2022
JackWillie added a commit to JackWillie/react-native-svg that referenced this issue Nov 27, 2022
@amitchaudhary140
Copy link

Thanks @Loovery , your fix worked.

@mhfortuna
Copy link

it is still broken but @Loovery fix still works!
version 15.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants