fix(🤖): Remove internal draw loop (#2763)

On Android it removes the need to use JNI back and forth for each frame.
Shopify · Nov 23, 2024 · ba1db84 · ba1db84
1 parent 1352eca
commit ba1db84
Show file tree

Hide file tree

Showing 41 changed files with 256 additions and 766 deletions.
diff --git a/apps/paper/ios/paper.xcodeproj/project.pbxproj b/apps/paper/ios/paper.xcodeproj/project.pbxproj
diff --git a/packages/skia/android/CMakeLists.txt b/packages/skia/android/CMakeLists.txt
@@ -87,7 +87,6 @@ add_library(
 
         "${PROJECT_SOURCE_DIR}/../cpp/rnskia/RNSkManager.cpp"
         "${PROJECT_SOURCE_DIR}/../cpp/rnskia/RNSkDomView.cpp"
-        "${PROJECT_SOURCE_DIR}/../cpp/rnskia/RNSkDispatchQueue.cpp"
 
         "${PROJECT_SOURCE_DIR}/../cpp/rnskia/dom/base/DrawingContext.cpp"
         "${PROJECT_SOURCE_DIR}/../cpp/rnskia/dom/base/ConcatablePaint.cpp"

diff --git a/packages/skia/android/cpp/jni/JniPlatformContext.cpp b/packages/skia/android/cpp/jni/JniPlatformContext.cpp
@@ -63,10 +63,6 @@ using TSelf = jni::local_ref<JniPlatformContext::jhybriddata>;
 void JniPlatformContext::registerNatives() {
   registerHybrid({
       makeNativeMethod("initHybrid", JniPlatformContext::initHybrid),
-      makeNativeMethod("notifyDrawLoop",
-                       JniPlatformContext::notifyDrawLoopExternal),
-      makeNativeMethod("notifyTaskReady",
-                       JniPlatformContext::notifyTaskReadyExternal),
   });
 }
 
@@ -134,51 +130,6 @@ sk_sp<SkImage> JniPlatformContext::takeScreenshotFromViewTag(size_t tag) {
   return skImage;
 }
 
-void JniPlatformContext::startDrawLoop() {
-  jni::ThreadScope ts;
-  // Start drawing loop
-  static auto method =
-      javaPart_->getClass()->getMethod<void(void)>("beginDrawLoop");
-  method(javaPart_.get());
-}
-
-void JniPlatformContext::stopDrawLoop() {
-  jni::ThreadScope ts;
-  // Stop drawing loop
-  static auto method =
-      javaPart_->getClass()->getMethod<void(void)>("endDrawLoop");
-  method(javaPart_.get());
-}
-
-void JniPlatformContext::notifyDrawLoopExternal() {
-  jni::ThreadScope ts;
-  _onNotifyDrawLoop();
-}
-
-void JniPlatformContext::runTaskOnMainThread(std::function<void()> task) {
-  _taskMutex->lock();
-  _taskCallbacks.push(task);
-  _taskMutex->unlock();
-
-  // Notify Java that task is ready
-  static auto method = javaPart_->getClass()->getMethod<void(void)>(
-      "notifyTaskReadyOnMainThread");
-  method(javaPart_.get());
-}
-
-void JniPlatformContext::notifyTaskReadyExternal() {
-  jni::ThreadScope ts;
-  _taskMutex->lock();
-  auto task = _taskCallbacks.front();
-  if (task != nullptr) {
-    _taskCallbacks.pop();
-    _taskMutex->unlock();
-    task();
-  } else {
-    _taskMutex->unlock();
-  }
-}
-
 void JniPlatformContext::performStreamOperation(
     const std::string &sourceUri,
     const std::function<void(std::unique_ptr<SkStreamAsset>)> &op) {

diff --git a/packages/skia/android/cpp/jni/include/JniPlatformContext.h b/packages/skia/android/cpp/jni/include/JniPlatformContext.h
@@ -33,11 +33,6 @@ class JniPlatformContext : public jni::HybridClass<JniPlatformContext> {
 
   void raiseError(const std::exception &err);
 
-  void startDrawLoop();
-  void stopDrawLoop();
-
-  void notifyDrawLoopExternal();
-
   void notifyTaskReadyExternal();
 
   void runTaskOnMainThread(std::function<void()> task);
@@ -46,10 +41,6 @@ class JniPlatformContext : public jni::HybridClass<JniPlatformContext> {
 
   sk_sp<SkImage> takeScreenshotFromViewTag(size_t tag);
 
-  void setOnNotifyDrawLoop(const std::function<void(void)> &callback) {
-    _onNotifyDrawLoop = callback;
-  }
-
   jni::global_ref<jobject> createVideo(const std::string &url);
 
 private:
@@ -58,16 +49,9 @@ class JniPlatformContext : public jni::HybridClass<JniPlatformContext> {
 
   float _pixelDensity;
 
-  std::function<void(void)> _onNotifyDrawLoop;
-
-  std::queue<std::function<void()>> _taskCallbacks;
-
-  std::shared_ptr<std::mutex> _taskMutex;
-
   explicit JniPlatformContext(
       jni::alias_ref<JniPlatformContext::jhybridobject> jThis,
       const float pixelDensity)
-      : _taskMutex(std::make_shared<std::mutex>()),
-        javaPart_(jni::make_global(jThis)), _pixelDensity(pixelDensity) {}
+      : javaPart_(jni::make_global(jThis)), _pixelDensity(pixelDensity) {}
 };
 } // namespace RNSkia
diff --git a/packages/skia/android/cpp/jni/include/JniSkiaBaseView.h b/packages/skia/android/cpp/jni/include/JniSkiaBaseView.h
@@ -39,8 +39,6 @@ class JniSkiaBaseView {
 
   virtual void surfaceDestroyed() { _skiaAndroidView->surfaceDestroyed(); }
 
-  virtual void setMode(std::string mode) { _skiaAndroidView->setMode(mode); }
-
   virtual void setDebugMode(bool show) {
     _skiaAndroidView->setShowDebugInfo(show);
   }

diff --git a/packages/skia/android/cpp/jni/include/JniSkiaDomView.h b/packages/skia/android/cpp/jni/include/JniSkiaDomView.h
@@ -40,7 +40,6 @@ class JniSkiaDomView : public jni::HybridClass<JniSkiaDomView>,
          makeNativeMethod("surfaceDestroyed", JniSkiaDomView::surfaceDestroyed),
          makeNativeMethod("surfaceSizeChanged",
                           JniSkiaDomView::surfaceSizeChanged),
-         makeNativeMethod("setMode", JniSkiaDomView::setMode),
          makeNativeMethod("setDebugMode", JniSkiaDomView::setDebugMode),
          makeNativeMethod("registerView", JniSkiaDomView::registerView),
          makeNativeMethod("unregisterView", JniSkiaDomView::unregisterView)});
@@ -57,8 +56,6 @@ class JniSkiaDomView : public jni::HybridClass<JniSkiaDomView>,
 
   void surfaceDestroyed() override { JniSkiaBaseView::surfaceDestroyed(); }
 
-  void setMode(std::string mode) override { JniSkiaBaseView::setMode(mode); }
-
   void setDebugMode(bool show) override { JniSkiaBaseView::setDebugMode(show); }
 
   void registerView(int nativeId) override {

diff --git a/packages/skia/android/cpp/jni/include/JniSkiaManager.h b/packages/skia/android/cpp/jni/include/JniSkiaManager.h
@@ -53,8 +53,6 @@ class JniSkiaManager : public jni::HybridClass<JniSkiaManager> {
   std::shared_ptr<RNSkManager> getSkiaManager() { return _skManager; }
 
   void invalidate() {
-    _context->stopDrawLoop();
-    _context->notifyDrawLoop(true);
     _skManager = nullptr;
     _context = nullptr;
   }

diff --git a/packages/skia/android/cpp/jni/include/JniSkiaPictureView.h b/packages/skia/android/cpp/jni/include/JniSkiaPictureView.h
@@ -41,7 +41,6 @@ class JniSkiaPictureView : public jni::HybridClass<JniSkiaPictureView>,
                           JniSkiaPictureView::surfaceDestroyed),
          makeNativeMethod("surfaceSizeChanged",
                           JniSkiaPictureView::surfaceSizeChanged),
-         makeNativeMethod("setMode", JniSkiaPictureView::setMode),
          makeNativeMethod("setDebugMode", JniSkiaPictureView::setDebugMode),
          makeNativeMethod("registerView", JniSkiaPictureView::registerView),
          makeNativeMethod("unregisterView",
@@ -59,8 +58,6 @@ class JniSkiaPictureView : public jni::HybridClass<JniSkiaPictureView>,
 
   void surfaceDestroyed() override { JniSkiaBaseView::surfaceDestroyed(); }
 
-  void setMode(std::string mode) override { JniSkiaBaseView::setMode(mode); }
-
   void setDebugMode(bool show) override { JniSkiaBaseView::setDebugMode(show); }
 
   void registerView(int nativeId) override {

diff --git a/packages/skia/android/cpp/rnskia-android/MainThreadDispatcher.h b/packages/skia/android/cpp/rnskia-android/MainThreadDispatcher.h
@@ -0,0 +1,69 @@
+#pragma once
+
+#include <android/looper.h>
+#include <unistd.h>
+
+class MainThreadDispatcher {
+private:
+  ALooper *mainLooper;
+  int messagePipe[2];
+  std::queue<std::function<void()>> taskQueue;
+  std::mutex queueMutex;
+
+  static constexpr int LOOPER_ID_MAIN = 1;
+
+  void processMessages() {
+    std::lock_guard<std::mutex> lock(queueMutex);
+    while (!taskQueue.empty()) {
+      auto task = taskQueue.front();
+      taskQueue.pop();
+      task();
+    }
+  }
+
+public:
+  static MainThreadDispatcher &getInstance() {
+    static MainThreadDispatcher instance;
+    return instance;
+  }
+
+  void post(std::function<void()> task) {
+    // TODO: this is disabled for now but we can clean this up
+    // if (ALooper_forThread() == mainLooper) {
+    //     task();
+    // } else {
+    {
+      std::lock_guard<std::mutex> lock(queueMutex);
+      taskQueue.push(std::move(task));
+    }
+    char wake = 1;
+    write(messagePipe[1], &wake, 1);
+    // }
+  }
+
+  ~MainThreadDispatcher() {
+    close(messagePipe[0]);
+    close(messagePipe[1]);
+  }
+
+private:
+  MainThreadDispatcher() {
+    mainLooper = ALooper_forThread();
+    if (!mainLooper) {
+      mainLooper = ALooper_prepare(ALOOPER_PREPARE_ALLOW_NON_CALLBACKS);
+    }
+
+    pipe(messagePipe);
+
+    ALooper_addFd(
+        mainLooper, messagePipe[0], LOOPER_ID_MAIN, ALOOPER_EVENT_INPUT,
+        [](int fd, int events, void *data) -> int {
+          char buf[1];
+          read(fd, buf, 1);
+          auto dispatcher = static_cast<MainThreadDispatcher *>(data);
+          dispatcher->processMessages();
+          return 1;
+        },
+        this);
+  }
+};
diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h b/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h
@@ -17,6 +17,7 @@
 
 #include "AHardwareBufferUtils.h"
 #include "JniPlatformContext.h"
+#include "MainThreadDispatcher.h"
 #include "RNSkAndroidVideo.h"
 #include "RNSkPlatformContext.h"
 
@@ -37,13 +38,9 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
       std::shared_ptr<facebook::react::CallInvoker> jsCallInvoker)
       : RNSkPlatformContext(runtime, jsCallInvoker,
                             jniPlatformContext->getPixelDensity()),
-        _jniPlatformContext(jniPlatformContext) {
-    // Hook onto the notify draw loop callback in the platform context
-    jniPlatformContext->setOnNotifyDrawLoop(
-        [this]() { notifyDrawLoop(false); });
-  }
+        _jniPlatformContext(jniPlatformContext) {}
 
-  ~RNSkAndroidPlatformContext() { stopDrawLoop(); }
+  ~RNSkAndroidPlatformContext() {}
 
   void performStreamOperation(
       const std::string &sourceUri,
@@ -163,17 +160,13 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
   }
 
   void runOnMainThread(std::function<void()> task) override {
-    _jniPlatformContext->runTaskOnMainThread(task);
+    MainThreadDispatcher::getInstance().post(std::move(task));
   }
 
   sk_sp<SkImage> takeScreenshotFromViewTag(size_t tag) override {
     return _jniPlatformContext->takeScreenshotFromViewTag(tag);
   }
 
-  void startDrawLoop() override { _jniPlatformContext->startDrawLoop(); }
-
-  void stopDrawLoop() override { _jniPlatformContext->stopDrawLoop(); }
-
 private:
   JniPlatformContext *_jniPlatformContext;
 };

diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidView.h b/packages/skia/android/cpp/rnskia-android/RNSkAndroidView.h
@@ -19,8 +19,6 @@ class RNSkBaseAndroidView {
 
   virtual float getPixelDensity() = 0;
 
-  virtual void setMode(std::string mode) = 0;
-
   virtual void setShowDebugInfo(bool show) = 0;
 
   virtual void viewDidUnmount() = 0;
@@ -42,7 +40,7 @@ class RNSkAndroidView : public T, public RNSkBaseAndroidView {
 
     // Try to render directly when the surface has been set so that
     // we don't have to wait until the draw loop returns.
-    RNSkView::renderImmediate();
+    RNSkView::redraw();
   }
 
   void surfaceDestroyed() override {
@@ -55,24 +53,16 @@ class RNSkAndroidView : public T, public RNSkBaseAndroidView {
         ->surfaceSizeChanged(surface, width, height);
     // This is only need for the first time to frame, this renderImmediate call
     // will invoke updateTexImage for the previous frame
-    RNSkView::renderImmediate();
+    RNSkView::redraw();
   }
 
   float getPixelDensity() override {
     return T::getPlatformContext()->getPixelDensity();
   }
 
-  void setMode(std::string mode) override {
-    if (mode.compare("continuous") == 0) {
-      T::setDrawingMode(RNSkDrawingMode::Continuous);
-    } else {
-      T::setDrawingMode(RNSkDrawingMode::Default);
-    }
-  }
-
   void setShowDebugInfo(bool show) override { T::setShowDebugOverlays(show); }
 
-  void viewDidUnmount() override { T::endDrawingLoop(); }
+  void viewDidUnmount() override {}
 
   std::shared_ptr<RNSkView> getSkiaView() override {
     return T::shared_from_this();

diff --git a/packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp b/packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp
@@ -64,7 +64,6 @@ bool RNSkOpenGLCanvasProvider::renderToCanvas(
 
       // Swap buffers and show on screen
       _surfaceHolder->present();
-
       return true;
     } else {
       // the render context did not provide a surface