react-native-svg icon indicating copy to clipboard operation
react-native-svg copied to clipboard

Update RNSVGRenderable.m

Open swittk opened this issue 5 years ago • 1 comments

Fixed memory leak when calling CGPathCreateCopyByTransformingPath (it's never released)

Summary

Explain the motivation for making this change: here are some points to help you:

  • What issues does the pull request solve? Please tag them so that they will get automatically closed once the PR is merged https://github.com/react-native-community/react-native-svg/issues/1436

  • What is the feature? (if applicable) Fixed memory leak

  • How did you implement the solution? Added CFAutoRelease to the temporary path object

  • What areas of the library does it impact? Rendering vectorEffect="nonScalingStroke"

Test Plan

Render this

<Path d={someData} strokeWidth={2} stroke="black" strokeLinecap="round" vectorEffect="nonScalingStroke"  />

What's required for testing (prerequisites)?

React native

Show memory debugger when running the code as above

Compatibility

This only affects iOS

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

swittk avatar Aug 15 '20 06:08 swittk

@swittk , @WoLewicki, can we merge this? This is significantly impacting performance. I have a patch-package in my repo with a slightly modified version of that code:

     if (_vectorEffect == kRNSVGVectorEffectNonScalingStroke) {
-        path = CGPathCreateCopyByTransformingPath(path, &svgToClientTransform);
+        CGPathRef newPath = CGPathCreateCopyByTransformingPath(path, &svgToClientTransform);
+        if (newPath) {
+            path = CFAutorelease(newPath);
+        }
         CGContextConcatCTM(context, CGAffineTransformInvert(svgToClientTransform));
     }

Needed to add the conditional if (newPath) to make sure that the path is null checked before calling CFAutorelease.

guiccbr avatar Aug 16 '22 15:08 guiccbr