Alternative value subscription implementation (#310)

Author: wcandillon

package/cpp/rnskia/RNSkJsiViewApi.h

@@ -197,9 +197,11 @@ class RNSkJsiViewApi : public JsiHostObject {
       : JsiHostObject(), _platformContext(platformContext) {}
 
   /**
-   * Destructor
+   * Invalidates the api object
    */
-  ~RNSkJsiViewApi() { unregisterAll(); }
+  void invalidate() {
+    unregisterAll();
+  }
 
   /**
    Call to remove all draw view infos

package/cpp/rnskia/RNSkManager.cpp

@@ -33,8 +33,8 @@ void RNSkManager::invalidate() {
   }
   _isInvalidated = true;
 
-  // We need to unregister all views when we get here
-  _viewApi->unregisterAll();
+  // Invalidate members
+  _viewApi->invalidate();
   _platformContext->invalidate();
 }
 

package/cpp/rnskia/values/RNSkDerivedValue.h

@@ -9,11 +9,11 @@
 #include <algorithm>
 #include <functional>
 #include <chrono>
-#include <mutex>
 
 namespace RNSkia
 {
 using namespace facebook;
+
 /**
   Creates a readonly value that depends on one or more other values. The derived value has a callback
   function that is used to calculate the new value when any of the dependencies change.

package/cpp/rnskia/values/RNSkReadonlyValue.h

@@ -26,9 +26,12 @@ class RNSkReadonlyValue : public JsiHostObject
   RNSkReadonlyValue(std::shared_ptr<RNSkPlatformContext> platformContext)
       : JsiHostObject(),
     _platformContext(platformContext),
-    _propNameId(jsi::PropNameID::forUtf8(*platformContext->getJsRuntime(), "value"))
-  {}
-
+    _propNameId(jsi::PropNameID::forUtf8(*platformContext->getJsRuntime(), "value")) {}
+  
+  ~RNSkReadonlyValue() {
+    _invalidated = true;
+  }
+  
   JSI_PROPERTY_GET(__typename__) {
     return jsi::String::createFromUtf8(runtime, "RNSkValue");
   }
@@ -73,7 +76,9 @@ class RNSkReadonlyValue : public JsiHostObject
     auto listenerId = _listenerId++;
     _listeners.emplace(listenerId, cb);
     return [this, listenerId]() {
-      removeListener(listenerId);
+      if(!_invalidated) {
+        removeListener(listenerId);
+      }
     };
   }
 
@@ -128,6 +133,8 @@ class RNSkReadonlyValue : public JsiHostObject
   const std::shared_ptr<RNSkPlatformContext> getPlatformContext() {
     return _platformContext;
   }
+  
+  std::atomic<bool> _invalidated = { false };
 
 private:
   jsi::PropNameID _propNameId;

package/ios/RNSkia-iOS/SkiaDrawView.mm

@@ -1,3 +1,5 @@
+#import "RCTBridge.h"
+
 #import <SkiaDrawView.h>
 #import <RNSkDrawViewImpl.h>
 #import <RNSkManager.h>
@@ -20,6 +22,15 @@ - (instancetype) initWithManager: (RNSkia::RNSkManager*)manager;
     _nativeId = 0;
     _debugMode = false;
     _drawingMode = RNSkia::RNSkDrawingMode::Default;
+    // Listen to notifications about module invalidation
+    auto nc = [NSNotificationCenter defaultCenter];
+    [nc addObserverForName:RCTBridgeWillInvalidateModulesNotification
+                    object:nil
+                     queue:nil
+                usingBlock:^(NSNotification *notification){
+      // Remove local variables
+      self->_manager = nullptr;                
+    }];
   }
   return self;
 }
@@ -38,7 +49,7 @@ - (void) willMoveToWindow:(UIWindow *)newWindow {
   if (newWindow == NULL) {
     // Remove implementation view when the parent view is not set
     if(_impl != nullptr) {
-      if(_nativeId != 0) {
+      if(_nativeId != 0 && _manager != nullptr) {
         _manager->setSkiaDrawView(_nativeId, nullptr);
       }
       _impl = nullptr;

package/src/renderer/Canvas.tsx

@@ -27,8 +27,8 @@ import type { FontMgr } from "../skia/FontMgr/FontMgr";
 import { debug as hostDebug, skHostConfig } from "./HostConfig";
 // import { debugTree } from "./nodes";
 import { vec } from "./processors";
-import { createDependencyManager } from "./DependencyManager";
 import { Container } from "./Host";
+import { DependencyManager } from "./DependencyManager";
 
 // useContextBridge() is taken from https://github.com/pmndrs/drei#usecontextbridge
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -72,14 +72,11 @@ skiaReconciler.injectIntoDevTools({
   rendererPackageName: "react-native-skia",
 });
 
-const render = (
-  element: ReactNode,
-  container: OpaqueRoot,
-  update: () => void
-) => {
-  skiaReconciler.updateContainer(element, container, null, () => {
+const render = (element: ReactNode, root: OpaqueRoot, container: Container) => {
+  skiaReconciler.updateContainer(element, root, null, () => {
     hostDebug("updateContainer");
-    update();
+
+    container.depMgr.subscribe();
   });
 };
 
@@ -99,34 +96,35 @@ export const Canvas = forwardRef<SkiaView, CanvasProps>(
     const [tick, setTick] = useState(0);
     const redraw = useCallback(() => setTick((t) => t + 1), []);
 
-    const tree = useMemo(() => new Container(redraw), [redraw]);
+    const container = useMemo(
+      () => new Container(new DependencyManager(ref), redraw),
+      [redraw, ref]
+    );
 
     const canvasCtx = useRef({ width: 0, height: 0 });
-    const container = useMemo(
-      () => skiaReconciler.createContainer(tree, 0, false, null),
-      [tree]
+    const root = useMemo(
+      () => skiaReconciler.createContainer(container, 0, false, null),
+      [container]
     );
     // Render effect
     useEffect(() => {
       render(
         <CanvasContext.Provider value={canvasCtx.current}>
           {children}
         </CanvasContext.Provider>,
-        container,
-        redraw
+        root,
+        container
       );
-    }, [children, container, redraw]);
-
-    const depsManager = useMemo(() => createDependencyManager(ref), [ref]);
+    }, [children, root, redraw, container]);
 
     // Draw callback
     const onDraw = useDrawCallback(
       (canvas, info) => {
         // TODO: if tree is empty (count === 1) maybe we should not render?
         const { width, height, timestamp } = info;
-        canvasCtx.current.width = width;
-        canvasCtx.current.height = height;
-        onTouch && onTouch(info.touches);
+        if (onTouch) {
+          onTouch(info.touches);
+        }
         const paint = Skia.Paint();
         paint.setAntiAlias(true);
         const ctx = {
@@ -140,21 +138,17 @@ export const Canvas = forwardRef<SkiaView, CanvasProps>(
           center: vec(width / 2, height / 2),
           fontMgr: fontMgr ?? Skia.FontMgr.RefDefault(),
         };
-        tree.draw(ctx);
+        canvasCtx.current = ctx;
+        container.draw(ctx);
       },
       [tick, onTouch]
     );
 
-    // Handle value dependency registration of values to the underlying
-    // SkiaView. Every time the tree changes (children), we will visit all
-    // our children and register their dependencies.
     useEffect(() => {
-      // Register all values in the current tree
-      depsManager.visitChildren(tree);
-      // Subscribe / return unsubscribe function
-      depsManager.subscribe();
-      return depsManager.unsubscribe;
-    }, [depsManager, tree, children]);
+      return () => {
+        container.depMgr.unsubscribe();
+      };
+    }, [container]);
 
     return (
       <SkiaView

package/src/renderer/DependencyManager.tsx

@@ -3,46 +3,57 @@ import type { RefObject } from "react";
 import type { SkiaView } from "../views";
 import type { SkiaReadonlyValue } from "../values";
 
+import { isValue } from "./processors";
 import type { Node } from "./Host";
-import { isValue, processProps } from "./processors";
-
-export const createDependencyManager = (ref: RefObject<SkiaView>) => {
-  const values: SkiaReadonlyValue<unknown>[] = [];
-  const unsubscribe: Array<() => void> = [];
-
-  return {
-    visitChildren: function (node: Node<unknown>) {
-      processProps(node.props, (value) => {
-        if (isValue(value)) {
-          this.registerValue(value);
-        }
-      });
-      node.children.forEach((c) => this.visitChildren(c));
-    },
-    registerValue: function <T>(value: SkiaReadonlyValue<T>) {
-      if (!ref.current) {
-        throw new Error("Canvas ref is not set");
-      }
-      if (values.indexOf(value) === -1) {
-        values.push(value);
-      }
-    },
-    subscribe: function () {
-      if (!ref.current) {
-        throw new Error("Canvas ref is not set");
-      }
-      if (values.length === 0) {
-        return;
+
+type Unsubscribe = () => void;
+type Props = { [key: string]: unknown };
+
+export class DependencyManager {
+  ref: RefObject<SkiaView>;
+  subscriptions: Map<
+    Node,
+    { values: SkiaReadonlyValue<unknown>[]; unsubscribe: null | Unsubscribe }
+  > = new Map();
+
+  constructor(ref: RefObject<SkiaView>) {
+    this.ref = ref;
+  }
+
+  unSubscribeNode(node: Node) {
+    const subscription = this.subscriptions.get(node);
+    if (subscription && subscription.unsubscribe) {
+      subscription.unsubscribe();
+    }
+    this.subscriptions.delete(node);
+  }
+
+  subscribeNode(node: Node, props: Props) {
+    const values = Object.values(props).filter(isValue);
+    if (values.length > 0) {
+      this.subscriptions.set(node, { values, unsubscribe: null });
+    }
+  }
+
+  subscribe() {
+    if (this.ref.current === null) {
+      throw new Error("Canvas ref is not set");
+    }
+    this.subscriptions.forEach((subscription) => {
+      if (subscription.unsubscribe === null) {
+        subscription.unsubscribe = this.ref.current!.registerValues(
+          subscription.values
+        );
       }
-      unsubscribe.push(ref.current.registerValues(values));
-      values.splice(0, values.length);
-    },
-    unsubscribe: function () {
-      if (unsubscribe.length === 0) {
-        return;
+    });
+  }
+
+  unsubscribe() {
+    this.subscriptions.forEach(({ unsubscribe }) => {
+      if (unsubscribe) {
+        unsubscribe();
       }
-      unsubscribe.forEach((unsub) => unsub());
-      unsubscribe.splice(0, unsubscribe.length);
-    },
-  };
-};
+    });
+    this.subscriptions.clear();
+  }
+}

package/src/renderer/Host.ts

@@ -4,6 +4,7 @@ import type { DrawingContext } from "./DrawingContext";
 import type { DeclarationResult, DeclarationProps } from "./nodes/Declaration";
 import type { DrawingProps } from "./nodes";
 import type { AnimatedProps } from "./processors/Animations/Animations";
+import type { DependencyManager } from "./DependencyManager";
 
 export enum NodeType {
   Declaration = "skDeclaration",
@@ -16,14 +17,20 @@ export abstract class Node<P = unknown> {
   memoizable = false;
   memoized = false;
   parent?: Node;
+  depMgr: DependencyManager;
 
-  constructor(props: AnimatedProps<P>) {
+  constructor(depMgr: DependencyManager, props: AnimatedProps<P>) {
     this._props = props;
+    this.depMgr = depMgr;
+    this.depMgr.unSubscribeNode(this);
+    this.depMgr.subscribeNode(this, props);
   }
 
   abstract draw(ctx: DrawingContext): void | DeclarationResult;
 
   set props(props: AnimatedProps<P>) {
+    this.depMgr.unSubscribeNode(this);
+    this.depMgr.subscribeNode(this, props);
     this._props = props;
   }
 
@@ -55,8 +62,8 @@ export abstract class Node<P = unknown> {
 export class Container extends Node {
   redraw: () => void;
 
-  constructor(redraw: () => void) {
-    super({});
+  constructor(depMgr: DependencyManager, redraw: () => void) {
+    super(depMgr, {});
     this.redraw = redraw;
   }
 

package/src/renderer/HostConfig.ts

@@ -108,6 +108,11 @@ const removeNode = (parent: Node, child: Node) => {
   bustBranchMemoization(parent);
   const index = parent.children.indexOf(child);
   parent.children.splice(index, 1);
+  child.depMgr.unSubscribeNode(child);
+  // unsubscribe to all children as well
+  for (const c of child.children) {
+    removeNode(child, c);
+  }
 };
 
 const insertBefore = (parent: Node, child: Node, before: Node) => {
@@ -120,14 +125,14 @@ const insertBefore = (parent: Node, child: Node, before: Node) => {
   parent.children.splice(beforeIndex, 0, child);
 };
 
-const createNode = (type: NodeType, props: Props) => {
+const createNode = (container: Container, type: NodeType, props: Props) => {
   switch (type) {
     case NodeType.Drawing:
       const { onDraw, skipProcessing, ...p1 } = props;
-      return new DrawingNode(onDraw, skipProcessing, p1);
+      return new DrawingNode(container.depMgr, onDraw, skipProcessing, p1);
     case NodeType.Declaration:
       const { onDeclare, ...p2 } = props;
-      return new DeclarationNode(onDeclare, p2);
+      return new DeclarationNode(container.depMgr, onDeclare, p2);
     default:
       // TODO: here we need to throw a nice error message
       // This is the error that will show up when the user uses nodes not supported by Skia (View, Audio, etc)
@@ -186,9 +191,15 @@ export const skHostConfig: SkiaHostConfig = {
     throw new Error("Text nodes are not supported yet");
   },
 
-  createInstance(type, props, _root, _hostContext, _internalInstanceHandle) {
+  createInstance(
+    type,
+    props,
+    container,
+    _hostContext,
+    _internalInstanceHandle
+  ) {
     debug("createInstance", type);
-    return createNode(type, props) as Node;
+    return createNode(container, type, props) as Node;
   },
 
   appendInitialChild(parentInstance, child) {