#1403 Added diposing of native resources (#1467)

--------- Co-authored-by: William Candillon <[email protected]>
Shopify · Apr 3, 2023 · 6ac9cb6 · 6ac9cb6
1 parent 9b6356f
commit 6ac9cb6
Show file tree

Hide file tree

Showing 14 changed files with 78 additions and 15 deletions.
diff --git a/package/cpp/api/JsiSkData.h b/package/cpp/api/JsiSkData.h
@@ -30,6 +30,13 @@ class JsiSkData : public JsiSkWrappingSkPtrHostObject<SkData> {
 
   JSI_EXPORT_PROPERTY_GETTERS(JSI_EXPORT_PROP_GET(JsiSkData, __typename__))
 
+  JSI_HOST_FUNCTION(dispose) {
+    setObject(nullptr);
+    return jsi::Value::undefined();
+  }
+
+  JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkData, dispose))
+
   /**
     Returns the underlying object from a host object of this type
    */

diff --git a/package/cpp/api/JsiSkImage.h b/package/cpp/api/JsiSkImage.h
@@ -104,12 +104,18 @@ class JsiSkImage : public JsiSkWrappingSkPtrHostObject<SkImage> {
     return jsi::String::createFromAscii(runtime, buffer);
   }
 
+  JSI_HOST_FUNCTION(dispose) {
+    setObject(nullptr);
+    return jsi::Value::undefined();
+  }
+
   JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkImage, width),
                        JSI_EXPORT_FUNC(JsiSkImage, height),
                        JSI_EXPORT_FUNC(JsiSkImage, makeShaderOptions),
                        JSI_EXPORT_FUNC(JsiSkImage, makeShaderCubic),
                        JSI_EXPORT_FUNC(JsiSkImage, encodeToBytes),
-                       JSI_EXPORT_FUNC(JsiSkImage, encodeToBase64))
+                       JSI_EXPORT_FUNC(JsiSkImage, encodeToBase64),
+                       JSI_EXPORT_FUNC(JsiSkImage, dispose))
 
   JsiSkImage(std::shared_ptr<RNSkPlatformContext> context,
              const sk_sp<SkImage> image)

diff --git a/package/cpp/api/JsiSkSVG.h b/package/cpp/api/JsiSkSVG.h
@@ -30,6 +30,13 @@ class JsiSkSVG : public JsiSkWrappingSkPtrHostObject<SkSVGDOM> {
 
   JSI_EXPORT_PROPERTY_GETTERS(JSI_EXPORT_PROP_GET(JsiSkSVG, __typename__))
 
+  JSI_HOST_FUNCTION(dispose) {
+    setObject(nullptr);
+    return jsi::Value::undefined();
+  }
+
+  JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkSVG, dispose))
+
   /**
     Returns the underlying object from a host object of this type
    */

diff --git a/package/cpp/api/JsiSkTypeface.h b/package/cpp/api/JsiSkTypeface.h
@@ -29,6 +29,13 @@ class JsiSkTypeface : public JsiSkWrappingSkPtrHostObject<SkTypeface> {
 
   JSI_EXPORT_PROPERTY_GETTERS(JSI_EXPORT_PROP_GET(JsiSkTypeface, __typename__))
 
+  JSI_HOST_FUNCTION(dispose) {
+    setObject(nullptr);
+    return jsi::Value::undefined();
+  }
+
+  JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkTypeface, dispose))
+
   JsiSkTypeface(std::shared_ptr<RNSkPlatformContext> context,
                 sk_sp<SkTypeface> typeface)
       : JsiSkWrappingSkPtrHostObject(std::move(context), std::move(typeface)) {}

diff --git a/package/cpp/rnskia/dom/base/JsiDomRenderNode.h b/package/cpp/rnskia/dom/base/JsiDomRenderNode.h
@@ -132,6 +132,14 @@ class JsiDomRenderNode : public JsiDomNode {
    */
   void resetPendingChanges() override { JsiDomNode::resetPendingChanges(); }
 
+  /**
+   Overridden dispose to release resources
+   */
+  void dispose(bool immediate) override {
+    JsiDomNode::dispose(immediate);
+    _paintCache.clear();
+  }
+
 protected:
   /**
    Invalidates and marks then context as changed.
@@ -232,6 +240,10 @@ class JsiDomRenderNode : public JsiDomNode {
   }
 
   struct PaintCache {
+    void clear() {
+      parent = nullptr;
+      child = nullptr;
+    }
     std::shared_ptr<SkPaint> parent;
     std::shared_ptr<SkPaint> child;
   };

diff --git a/package/src/skia/core/Data.ts b/package/src/skia/core/Data.ts
@@ -3,7 +3,12 @@ import { Image } from "react-native";
 
 import { Skia } from "../Skia";
 import { isRNModule } from "../types";
-import type { SkData, DataModule, DataSourceParam } from "../types";
+import type {
+  SkData,
+  DataModule,
+  DataSourceParam,
+  JsiDisposable,
+} from "../types";
 
 const resolveAsset = (source: DataModule) => {
   return isRNModule(source)
@@ -27,7 +32,7 @@ const factoryWrapper = <T>(
 
 const loadData = <T>(
   source: DataSourceParam,
-  factory: (data: SkData) => T,
+  factory: (data: SkData) => T | null,
   onError?: (err: Error) => void
 ): Promise<T | null> => {
   if (source === null || source === undefined) {
@@ -43,32 +48,35 @@ const loadData = <T>(
     );
   }
 };
-const useLoading = <T>(
+const useLoading = <T extends JsiDisposable>(
   source: DataSourceParam,
   loader: () => Promise<T | null>
 ) => {
   const mounted = useRef(false);
   const [data, setData] = useState<T | null>(null);
+  const dataRef = useRef<T | null>(null);
   useEffect(() => {
     mounted.current = true;
     loader().then((value) => {
       if (mounted.current) {
         setData(value);
+        dataRef.current = value;
       }
     });
     return () => {
+      dataRef.current?.dispose();
       mounted.current = false;
     };
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [source]);
   return data;
 };
 
-export const useRawData = <T>(
+export const useRawData = <T extends JsiDisposable>(
   source: DataSourceParam,
-  factory: (data: SkData) => T,
+  factory: (data: SkData) => T | null,
   onError?: (err: Error) => void
-) => useLoading(source, () => loadData(source, factory, onError));
+) => useLoading(source, () => loadData<T>(source, factory, onError));
 
 const identity = (data: SkData) => data;
 

diff --git a/package/src/skia/types/Data/Data.ts b/package/src/skia/types/Data/Data.ts
@@ -1,6 +1,6 @@
-import type { SkJSIInstance } from "../JsiInstance";
+import type { JsiDisposable, SkJSIInstance } from "../JsiInstance";
 
-export type SkData = SkJSIInstance<"Data">;
+export type SkData = SkJSIInstance<"Data"> & JsiDisposable;
 
 type RNModule = number;
 type ESModule = {

diff --git a/package/src/skia/types/Image/Image.ts b/package/src/skia/types/Image/Image.ts
@@ -1,5 +1,5 @@
 import type { SkMatrix } from "../Matrix";
-import type { SkJSIInstance } from "../JsiInstance";
+import type { JsiDisposable, SkJSIInstance } from "../JsiInstance";
 import type { TileMode } from "../ImageFilter";
 import type { SkShader } from "../Shader";
 
@@ -20,7 +20,7 @@ export enum ImageFormat {
   WEBP = 6,
 }
 
-export interface SkImage extends SkJSIInstance<"Image"> {
+export interface SkImage extends SkJSIInstance<"Image">, JsiDisposable {
   /**
    * Returns the possibly scaled height of the image.
    */

diff --git a/package/src/skia/types/JsiInstance.ts b/package/src/skia/types/JsiInstance.ts
@@ -1,3 +1,7 @@
 export interface SkJSIInstance<T extends string> {
   __typename__: T;
 }
+
+export interface JsiDisposable {
+  dispose: () => void;
+}
diff --git a/package/src/skia/types/SVG/SVG.ts b/package/src/skia/types/SVG/SVG.ts
@@ -1,3 +1,3 @@
-import type { SkJSIInstance } from "../JsiInstance";
+import type { JsiDisposable, SkJSIInstance } from "../JsiInstance";
 
-export type SkSVG = SkJSIInstance<"SVG">;
+export type SkSVG = SkJSIInstance<"SVG"> & JsiDisposable;
diff --git a/package/src/skia/types/Typeface/Typeface.ts b/package/src/skia/types/Typeface/Typeface.ts
@@ -1,3 +1,3 @@
-import type { SkJSIInstance } from "../JsiInstance";
+import type { JsiDisposable, SkJSIInstance } from "../JsiInstance";
 
-export type SkTypeface = SkJSIInstance<"Typeface">;
+export type SkTypeface = SkJSIInstance<"Typeface"> & JsiDisposable;
diff --git a/package/src/skia/web/JsiSkData.ts b/package/src/skia/web/JsiSkData.ts
@@ -10,4 +10,8 @@ export class JsiSkData extends HostObject<Data, "Data"> implements SkData {
   constructor(CanvasKit: CanvasKit, ref: Data) {
     super(CanvasKit, ref, "Data");
   }
+
+  dispose() {
+    // Not implemented in data
+  }
 }
diff --git a/package/src/skia/web/JsiSkImage.ts b/package/src/skia/web/JsiSkImage.ts
@@ -109,4 +109,8 @@ export class JsiSkImage extends HostObject<Image, "Image"> implements SkImage {
     const bytes = this.encodeToBytes(fmt, quality);
     return toBase64String(bytes);
   }
+
+  dispose() {
+    this.ref.delete();
+  }
 }
diff --git a/package/src/skia/web/JsiSkTypeface.ts b/package/src/skia/web/JsiSkTypeface.ts
@@ -25,4 +25,8 @@ export class JsiSkTypeface
     );
     return false;
   }
+
+  dispose() {
+    this.ref.delete();
+  }
 }