Merge pull request #435 from Shopify/bugfix/398-fix-crash-in-releasemode

Fix crash in release mode

package/android/CMakeLists.txt

@@ -31,7 +31,8 @@ add_library(
         "${PROJECT_SOURCE_DIR}/cpp/jni/JniSkiaManager.cpp"
         "${PROJECT_SOURCE_DIR}/cpp/jni/JniSkiaDrawView.cpp"
         "${PROJECT_SOURCE_DIR}/cpp/jni/JniPlatformContext.cpp"
-        "${PROJECT_SOURCE_DIR}/cpp/jni/SkiaOpenGLRenderer.cpp"
+        "${PROJECT_SOURCE_DIR}/cpp/rnskia-android/RNSkDrawViewImpl.cpp"
+        "${PROJECT_SOURCE_DIR}/cpp/rnskia-android/SkiaOpenGLRenderer.cpp"
 
         "${PROJECT_SOURCE_DIR}/cpp/jsi/JsiHostObject.cpp"
 
@@ -64,6 +65,7 @@ target_include_directories(
         cpp/api
         cpp/jsi
         cpp/jni/include
+        cpp/rnskia-android
         cpp/rnskia
         cpp/rnskia/values
         cpp/utils  

package/android/cpp/jni/JniSkiaDrawView.cpp

@@ -31,16 +31,13 @@ namespace RNSkia
     /**** DTOR ***/
     JniSkiaDrawView::~JniSkiaDrawView()
     {
-#if LOG_ALL_DRAWING
-        RNSkLogger::logToConsole("JniSkiaDrawView::~JniSkiaDrawView %i", getNativeId());
-#endif
     }
 
     /**** JNI ****/
 
     TSelf JniSkiaDrawView::initHybrid(
-        alias_ref<HybridClass::jhybridobject> jThis,
-        JavaSkiaManager skiaManager)
+            alias_ref<HybridClass::jhybridobject> jThis,
+            JavaSkiaManager skiaManager)
     {
         return makeCxxInstance(jThis, skiaManager);
     }
@@ -60,25 +57,25 @@ namespace RNSkia
     {
         if (mode.compare("continuous") == 0)
         {
-            setDrawingMode(RNSkDrawingMode::Continuous);
+            _drawView->setDrawingMode(RNSkDrawingMode::Continuous);
         }
         else
         {
-            setDrawingMode(RNSkDrawingMode::Default);
+            _drawView->setDrawingMode(RNSkDrawingMode::Default);
         }
     }
 
     void JniSkiaDrawView::setDebugMode(bool show)
     {
-        setShowDebugOverlays(show);
+        _drawView->setShowDebugOverlays(show);
     }
 
     void JniSkiaDrawView::updateTouchPoints(jni::JArrayDouble touches)
     {
         // Create touch points
         std::vector<RNSkia::RNSkTouchPoint> points;
         auto pin = touches.pin();
-        auto scale = getPlatformContext()->getPixelDensity();
+        auto scale = _drawView->getPixelDensity();
         points.reserve(pin.size() / 4);
         for (size_t i = 0; i < pin.size(); i += 4)
         {
@@ -89,81 +86,27 @@ namespace RNSkia
             point.type = (RNSkia::RNSkTouchType)pin[i + 3];
             points.push_back(point);
         }
-        updateTouchState(std::move(points));
+        _drawView->updateTouchState(std::move(points));
     }
 
     void JniSkiaDrawView::surfaceAvailable(jobject surface, int width, int height)
     {
-#if LOG_ALL_DRAWING
-        RNSkLogger::logToConsole("JniSkiaDrawView::surfaceAvailable %i", getNativeId());
-#endif
-
-        _width = width;
-        _height = height;
-
-        if (_renderer == nullptr)
-        {
-            // Create renderer!
-            _renderer = new SkiaOpenGLRenderer(
-                ANativeWindow_fromSurface(Environment::current(), surface), getNativeId());
-
-            // Set the draw function
-            setNativeDrawFunc(std::bind(&JniSkiaDrawView::drawFrame, this, std::placeholders::_1));
-
-            // Redraw
-            requestRedraw();
-        }
+        _drawView->surfaceAvailable(ANativeWindow_fromSurface(Environment::current(), surface), width, height);
     }
 
     void JniSkiaDrawView::surfaceSizeChanged(int width, int height)
     {
-#if LOG_ALL_DRAWING
-        RNSkLogger::logToConsole("JniSkiaDrawView::surfaceSizeChanged %i", getNativeId());
-#endif
-
-        _width = width;
-        _height = height;
-
-        // Redraw after size change
-        requestRedraw();
+        _drawView->surfaceSizeChanged(width, height);
     }
 
     void JniSkiaDrawView::surfaceDestroyed()
     {
-#if LOG_ALL_DRAWING
-        RNSkLogger::logToConsole("JniSkiaDrawView::surfaceDestroyed %i", getNativeId());
-#endif
-        if (_renderer != nullptr)
-        {
-            // Turn off drawing
-            setNativeDrawFunc(nullptr);
-
-            // Start teardown
-            _renderer->teardown();
-
-            // Ask for a redraw to tear down the render pipeline. This
-            // needs to be done on the render thread since OpenGL demands
-            // same thread access for OpenGL contexts.
-            getPlatformContext()->runOnRenderThread([this]()
-                                                    {
-                if(_renderer != nullptr) {
-                    _renderer->run(nullptr, 0, 0);
-                } });
-
-            // Wait until the above render has finished.
-            _renderer->waitForTeardown();
-
-            // Delete renderer. All resources should be released during teardown.
-            delete _renderer;
-            _renderer = nullptr;
-        }
+        _drawView->surfaceDestroyed();
     }
 
-    /**** Render method ****/
-
-    void JniSkiaDrawView::drawFrame(const sk_sp<SkPicture> picture)
-    {
-        // No need to check if the renderer is nullptr since we only get here if it is not.
-        _renderer->run(picture, _width, _height);
+    void JniSkiaDrawView::releaseSurface() {
+        jni::ThreadScope ts;
+        static auto method = javaPart_->getClass()->getMethod<void(void)>("releaseSurface");
+        method(javaPart_.get());
     }
 } // namespace RNSkia

package/android/cpp/jni/JniSkiaManager.cpp

@@ -47,7 +47,7 @@ void JniSkiaManager::initializeRuntime() {
 }
 
 void JniSkiaManager::registerSkiaView(int viewTag, JniSkiaDrawView *skiaView) {
-    _skManager->registerSkiaDrawView(viewTag, skiaView);
+    _skManager->registerSkiaDrawView(viewTag, skiaView->getDrawViewImpl());
 }
 
 void JniSkiaManager::unregisterSkiaView(int viewTag) {

package/android/cpp/jni/include/JniSkiaDrawView.h

@@ -5,17 +5,16 @@
 #include <thread>
 #include <string>
 
-#include <EGL/egl.h>
-#include <GLES2/gl2.h>
 #include <fbjni/fbjni.h>
 #include <jni.h>
 #include <jsi/jsi.h>
 #include <thread>
 
 #include <RNSkDrawView.h>
-#include "JniSkiaManager.h"
-#include "JniSkiaDrawView.h"
-#include "SkiaOpenGLRenderer.h"
+#include <JniSkiaManager.h>
+#include <JniSkiaDrawView.h>
+
+#include <RNSkDrawViewImpl.h>
 
 #include <SkSurface.h>
 #include <SkRefCnt.h>
@@ -31,16 +30,15 @@ namespace RNSkia
 
     using JavaSkiaManager = jni::alias_ref<JniSkiaManager::javaobject>;
 
-    class JniSkiaDrawView : public jni::HybridClass<JniSkiaDrawView>,
-                            public RNSkDrawView
+    class JniSkiaDrawView : public jni::HybridClass<JniSkiaDrawView>
     {
     public:
         static auto constexpr kJavaDescriptor = "Lcom/shopify/reactnative/skia/SkiaDrawView;";
         static auto constexpr TAG = "ReactNativeSkia";
 
         static jni::local_ref<jhybriddata> initHybrid(
-            jni::alias_ref<jhybridobject>,
-            JavaSkiaManager);
+                jni::alias_ref<jhybridobject>,
+                JavaSkiaManager);
 
         static void registerNatives();
 
@@ -52,30 +50,28 @@ namespace RNSkia
 
         ~JniSkiaDrawView();
 
-    protected:
-        int getWidth() override { return _width; }
-        int getHeight() override { return _height; }
+        std::shared_ptr<RNSkDrawView> getDrawViewImpl() { return _drawView; }
+
+        void releaseSurface();
 
+    protected:
         void setMode(std::string mode);
         void setDebugMode(bool show);
 
     private:
         friend HybridBase;
 
-        void drawFrame(const sk_sp<SkPicture> picture);
-
-        int _width = 0;
-        int _height = 0;
-
-        SkiaOpenGLRenderer* _renderer = nullptr;
+        std::shared_ptr<RNSkDrawViewImpl> _drawView;
 
         jni::global_ref<JniSkiaDrawView::javaobject> javaPart_;
 
         explicit JniSkiaDrawView(
-            jni::alias_ref<JniSkiaDrawView::jhybridobject> jThis,
-            JavaSkiaManager skiaManager)
-            : javaPart_(jni::make_global(jThis)),
-              RNSkDrawView(skiaManager->cthis()->getPlatformContext()) {
+                jni::alias_ref<JniSkiaDrawView::jhybridobject> jThis,
+                JavaSkiaManager skiaManager)
+                : javaPart_(jni::make_global(jThis)),
+                  _drawView(std::make_shared<RNSkDrawViewImpl>(skiaManager->cthis()->getPlatformContext(), [this]() {
+                      releaseSurface();
+                  })) {
         }
     };
 

package/android/cpp/jni/include/JniSkiaManager.h

@@ -7,7 +7,7 @@
 #include <memory>
 
 #include <JniPlatformContext.h>
-#include <JniPlatformContextWrapper.h>
+#include <RNSkPlatformContextImpl.h>
 
 #include <RNSkLog.h>
 
@@ -48,14 +48,14 @@ class JniSkiaManager : public jni::HybridClass<JniSkiaManager> {
         : _javaPart(jni::make_global(jThis)),
           _jsRuntime(runtime),
           _jsCallInvoker(jsCallInvoker),
-          _context(std::make_shared<JniPlatformContextWrapper>(platformContext, runtime, jsCallInvoker)) {
+          _context(std::make_shared<RNSkPlatformContextImpl>(platformContext, runtime, jsCallInvoker)) {
 
     }
 
     void registerSkiaView(int viewTag, JniSkiaDrawView *skiaView);
     void unregisterSkiaView(int viewTag);
 
-    std::shared_ptr<JniPlatformContextWrapper> getPlatformContext() { return _context; }
+    std::shared_ptr<RNSkPlatformContextImpl> getPlatformContext() { return _context; }
 
     void invalidate() {
         _context->stopDrawLoop();
@@ -73,7 +73,7 @@ class JniSkiaManager : public jni::HybridClass<JniSkiaManager> {
 
     jsi::Runtime *_jsRuntime;
     std::shared_ptr<facebook::react::CallInvoker> _jsCallInvoker;
-    std::shared_ptr<JniPlatformContextWrapper> _context;
+    std::shared_ptr<RNSkPlatformContextImpl> _context;
 
     void initializeRuntime();
 };

package/android/cpp/rnskia-android/RNSkDrawViewImpl.cpp

@@ -0,0 +1,68 @@
+#include <RNSkDrawViewImpl.h>
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdocumentation"
+
+#include <SkSurface.h>
+#include <SkCanvas.h>
+
+#pragma clang diagnostic pop
+
+#include <RNSkLog.h>
+
+namespace RNSkia {
+    RNSkDrawViewImpl::RNSkDrawViewImpl(std::shared_ptr <RNSkia::RNSkPlatformContext> context, std::function<void()> releaseSurfaceCallback) :
+        RNSkia::RNSkDrawView(context),
+        _releaseSurfaceCallback(std::move(releaseSurfaceCallback)) {}
+
+    void RNSkDrawViewImpl::surfaceAvailable(ANativeWindow* surface, int width, int height) {
+        _width = width;
+        _height = height;
+
+        if (_renderer == nullptr)
+        {
+            // Create renderer!
+            _renderer = std::make_unique<SkiaOpenGLRenderer>(surface, getNativeId());
+
+            // Redraw
+            requestRedraw();
+        }
+    }
+
+    void RNSkDrawViewImpl::surfaceDestroyed() {
+        if (_renderer != nullptr)
+        {
+            // Start teardown
+            _renderer->teardown();
+
+            // Teardown renderer on the render thread since OpenGL demands
+            // same thread access for OpenGL contexts.
+            getPlatformContext()->runOnRenderThread([weakSelf = weak_from_this()]() {
+                auto self = weakSelf.lock();
+                if(self) {
+                    auto drawViewImpl = std::dynamic_pointer_cast<RNSkDrawViewImpl>(self);
+                    if(drawViewImpl->_renderer != nullptr) {
+                        drawViewImpl->_renderer->run(nullptr, 0, 0);
+                    }
+                    // Remove renderer
+                    drawViewImpl->_renderer = nullptr;
+                    drawViewImpl->_releaseSurfaceCallback();
+                }
+            });
+        }
+    }
+
+    void RNSkDrawViewImpl::surfaceSizeChanged(int width, int height) {
+        _width = width;
+        _height = height;
+
+        // Redraw after size change
+        requestRedraw();
+    }
+
+    void RNSkDrawViewImpl::drawPicture(const sk_sp <SkPicture> picture) {
+        if(_renderer != nullptr) {
+            _renderer->run(picture, _width, _height);
+        }
+    }
+}