Merge branch 'platform-work' into scene-refactor-base

core/include/tangram/platform.h

@@ -61,6 +61,7 @@ class Platform {
     Platform();
     virtual ~Platform();
 
+    virtual void shutdown() = 0;
     // Request that a new frame be rendered by the windowing system
     virtual void requestRender() const = 0;
 

platforms/android/tangram/src/main/cpp/androidPlatform.cpp

@@ -133,10 +133,16 @@ class JniThreadBinding {
 public:
     JniThreadBinding(JavaVM* _jvm) : jvm(_jvm) {
         status = jvm->GetEnv((void**)&jniEnv, TANGRAM_JNI_VERSION);
-        if (status == JNI_EDETACHED) { jvm->AttachCurrentThread(&jniEnv, NULL);}
+        if (status == JNI_EDETACHED) {
+            LOG("---------------->>> ATTACH");
+            jvm->AttachCurrentThread(&jniEnv, NULL);
+        }
     }
     ~JniThreadBinding() {
-        if (status == JNI_EDETACHED) { jvm->DetachCurrentThread(); }
+        if (status == JNI_EDETACHED) {
+            LOG("---------------->>> DETACH");
+            jvm->DetachCurrentThread();
+        }
     }
 
     JNIEnv* operator->() const {
@@ -173,8 +179,10 @@ std::string AndroidPlatform::fontPath(const std::string& _family, const std::str
     return resultStr;
 }
 
-AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance) {
-    m_tangramInstance = _jniEnv->NewGlobalRef(_tangramInstance);
+AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance)
+    : m_jniWorker(jvm) {
+
+    m_tangramInstance = _jniEnv->NewWeakGlobalRef(_tangramInstance);
 
     m_assetManager = AAssetManager_fromJava(_jniEnv, _assetManager);
 
@@ -188,35 +196,32 @@ AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject
 #endif
 }
 
-void AndroidPlatform::dispose(JNIEnv* _jniEnv) {
-    _jniEnv->DeleteGlobalRef(m_tangramInstance);
-}
-
 void AndroidPlatform::requestRender() const {
-
-    JniThreadBinding jniEnv(jvm);
-
-    jniEnv->CallVoidMethod(m_tangramInstance, requestRenderMethodID);
+    m_jniWorker.enqueue([&](JNIEnv *jniEnv) {
+        jniEnv->CallVoidMethod(m_tangramInstance, requestRenderMethodID);
+    });
 }
 
-std::string AndroidPlatform::fontFallbackPath(int _importance, int _weightHint) const {
-
+std::vector<FontSourceHandle> AndroidPlatform::systemFontFallbacksHandle() const {
     JniThreadBinding jniEnv(jvm);
 
-    jstring returnStr = (jstring) jniEnv->CallObjectMethod(m_tangramInstance, getFontFallbackFilePath, _importance, _weightHint);
-
-    auto resultStr = stringFromJString(jniEnv, returnStr);
-    jniEnv->DeleteLocalRef(returnStr);
-
-    return resultStr;
-}
-
-std::vector<FontSourceHandle> AndroidPlatform::systemFontFallbacksHandle() const {
     std::vector<FontSourceHandle> handles;
 
     int importance = 0;
     int weightHint = 400;
 
+    auto fontFallbackPath = [&](int _importance, int _weightHint) {
+
+        jstring returnStr = (jstring) jniEnv->CallObjectMethod(m_tangramInstance,
+                                                               getFontFallbackFilePath, _importance,
+                                                               _weightHint);
+
+        auto resultStr = stringFromJString(jniEnv, returnStr);
+        jniEnv->DeleteLocalRef(returnStr);
+
+        return resultStr;
+    };
+
     std::string fallbackPath = fontFallbackPath(importance, weightHint);
 
     while (!fallbackPath.empty()) {
@@ -291,18 +296,17 @@ std::vector<char> AndroidPlatform::bytesFromFile(const Url& url) const {
 
 UrlRequestHandle AndroidPlatform::startUrlRequest(Url _url, UrlCallback _callback) {
 
-    JniThreadBinding jniEnv(jvm);
-
     // Get the current value of the request counter and add one, atomically.
     UrlRequestHandle requestHandle = m_urlRequestCount++;
+    if (!_callback) { return requestHandle; }
 
     // If the requested URL does not use HTTP or HTTPS, retrieve it synchronously.
     if (!_url.hasHttpScheme()) {
-        UrlResponse response;
-        response.content = bytesFromFile(_url);
-        if (_callback) {
-            _callback(std::move(response));
-        }
+        m_fileWorker.enqueue([=](){
+             UrlResponse response;
+             response.content = bytesFromFile(_url);
+             _callback(std::move(response));
+        });
         return requestHandle;
     }
 
@@ -312,26 +316,28 @@ UrlRequestHandle AndroidPlatform::startUrlRequest(Url _url, UrlCallback _callbac
         m_callbacks[requestHandle] = _callback;
     }
 
-    jlong jRequestHandle = static_cast<jlong>(requestHandle);
-
-    // Check that it's safe to convert the UrlRequestHandle to a jlong and back.
-    assert(requestHandle == static_cast<UrlRequestHandle>(jRequestHandle));
+    m_jniWorker.enqueue([=](JNIEnv *jniEnv) {
+        jlong jRequestHandle = static_cast<jlong>(requestHandle);
 
-    jstring jUrl = jstringFromString(jniEnv, _url.string());
+        // Check that it's safe to convert the UrlRequestHandle to a jlong and back... cmon :P
+        assert(requestHandle == static_cast<UrlRequestHandle>(jRequestHandle));
 
-    // Call the MapController method to start the URL request.
-    jniEnv->CallVoidMethod(m_tangramInstance, startUrlRequestMID, jUrl, jRequestHandle);
+        jstring jUrl = jstringFromString(jniEnv, _url.string());
 
+        // Call the MapController method to start the URL request.
+        jniEnv->CallVoidMethod(m_tangramInstance, startUrlRequestMID, jUrl, jRequestHandle);
+    });
     return requestHandle;
 }
 
 void AndroidPlatform::cancelUrlRequest(UrlRequestHandle request) {
 
-    JniThreadBinding jniEnv(jvm);
+    m_jniWorker.enqueue([=](JNIEnv *jniEnv) {
 
-    jlong jRequestHandle = static_cast<jlong>(request);
+        jlong jRequestHandle = static_cast<jlong>(request);
 
-    jniEnv->CallVoidMethod(m_tangramInstance, cancelUrlRequestMID, jRequestHandle);
+        jniEnv->CallVoidMethod(m_tangramInstance, cancelUrlRequestMID, jRequestHandle);
+    });
 
     // We currently don't try to cancel requests for local files.
 }
@@ -355,20 +361,21 @@ void AndroidPlatform::onUrlComplete(JNIEnv* _jniEnv, jlong _jRequestHandle, jbyt
         response.error = error.c_str();
     }
 
-    // Find the callback associated with the request.
-    UrlCallback callback;
-    {
-        std::lock_guard<std::mutex> lock(m_callbackMutex);
-        UrlRequestHandle requestHandle = static_cast<UrlRequestHandle>(_jRequestHandle);
-        auto it = m_callbacks.find(requestHandle);
-        if (it != m_callbacks.end()) {
-            callback = std::move(it->second);
-            m_callbacks.erase(it);
+
+    m_fileWorker.enqueue([this, _jRequestHandle, r = std::move(response)]() mutable {
+        // Find the callback associated with the request.
+        UrlCallback callback;
+        {
+            std::lock_guard<std::mutex> lock(m_callbackMutex);
+            UrlRequestHandle requestHandle = static_cast<UrlRequestHandle>(_jRequestHandle);
+            auto it = m_callbacks.find(requestHandle);
+            if (it != m_callbacks.end()) {
+                callback = std::move(it->second);
+                m_callbacks.erase(it);
+            }
         }
-    }
-    if (callback) {
-        callback(std::move(response));
-    }
+        if (callback) { callback(std::move(r)); }
+    });
 }
 
 void setCurrentThreadPriority(int priority) {

platforms/android/tangram/src/main/cpp/androidPlatform.h

@@ -1,10 +1,12 @@
 #pragma once
 
 #include "platform.h"
+#include "jniWorker.h"
+#include "util/asyncWorker.h"
+
 
 #include <jni.h>
 #include <android/asset_manager.h>
-
 #include <atomic>
 #include <mutex>
 #include <string>
@@ -29,7 +31,7 @@ class AndroidPlatform : public Platform {
 public:
 
     AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance);
-    void dispose(JNIEnv* _jniEnv);
+    void shutdown() override {}
     void requestRender() const override;
     void setContinuousRendering(bool _isContinuous) override;
     FontSourceHandle systemFont(const std::string& _name, const std::string& _weight, const std::string& _face) const override;
@@ -64,6 +66,9 @@ class AndroidPlatform : public Platform {
     std::mutex m_callbackMutex;
     std::unordered_map<UrlRequestHandle, UrlCallback> m_callbacks;
 
+    mutable JniWorker m_jniWorker;  // FIX requestRender const.. Lets use Rust if we want this for real
+    AsyncWorker m_fileWorker;
+
 };
 
 } // namespace Tangram

platforms/android/tangram/src/main/cpp/jniExports.cpp

@@ -144,16 +144,9 @@ extern "C" {
         return reinterpret_cast<jlong>(map);
     }
 
-    JNIEXPORT void JNICALL Java_com_mapzen_tangram_MapController_nativeDispose(JNIEnv* jniEnv, jobject obj, jlong mapPtr) {
+    JNIEXPORT void JNICALL Java_com_mapzen_tangram_MapController_nativeDispose(JNIEnv* jniEnv, jobject tangramInstance, jlong mapPtr) {
         assert(mapPtr > 0);
-        auto map = reinterpret_cast<Tangram::Map*>(mapPtr);
-        // Don't dispose MapController ref before map is teared down,
-        // delete map or worker threads might call back to it (e.g. requestRender)
-        auto platform = map->getPlatform();
-
-        delete map;
-
-        static_cast<Tangram::AndroidPlatform&>(*platform).dispose(jniEnv);
+        delete reinterpret_cast<Tangram::Map*>(mapPtr);
     }
 
     JNIEXPORT jint JNICALL Java_com_mapzen_tangram_MapController_nativeLoadScene(JNIEnv* jniEnv, jobject obj, jlong mapPtr, jstring path, jobjectArray updateStrings) {

platforms/android/tangram/src/main/cpp/jniWorker.h

@@ -0,0 +1,69 @@
+#include <condition_variable>
+#include <deque>
+#include <mutex>
+#include <thread>
+#include <jni.h>
+
+namespace Tangram {
+
+class JniWorker {
+public:
+
+    explicit JniWorker(JavaVM* _jvm) : jvm(_jvm){
+
+        thread = std::thread(&JniWorker::run, this);
+
+        pthread_setname_np(thread.native_handle(), "TangramJNI Worker");
+    }
+
+    ~JniWorker() {
+        {
+            std::unique_lock<std::mutex> lock(m_mutex);
+            m_running = false;
+        }
+        m_condition.notify_all();
+        thread.join();
+    }
+
+    void enqueue(std::function<void(JNIEnv *jniEnv)> _task) {
+        {
+            std::unique_lock<std::mutex> lock(m_mutex);
+            if (!m_running) { return; }
+
+            m_queue.push_back(std::move(_task));
+        }
+        m_condition.notify_one();
+    }
+
+private:
+
+    void run() {
+        jvm->AttachCurrentThread(&jniEnv, NULL);
+
+        while (true) {
+            std::function<void(JNIEnv *jniEnv)> task;
+            {
+                std::unique_lock<std::mutex> lock(m_mutex);
+                m_condition.wait(lock, [&]{ return !m_running || !m_queue.empty(); });
+                if (!m_running) { break; }
+
+                task = std::move(m_queue.front());
+                m_queue.pop_front();
+            }
+            task(jniEnv);
+        }
+        jvm->DetachCurrentThread();
+    }
+
+    std::thread thread;
+    bool m_running = true;
+    std::condition_variable m_condition;
+    std::mutex m_mutex;
+    std::deque<std::function<void(JNIEnv *)>> m_queue;
+
+    JavaVM* jvm;
+    JNIEnv *jniEnv;
+
+};
+
+}

platforms/android/tangram/src/main/java/com/mapzen/tangram/MapController.java

@@ -185,7 +185,7 @@ protected MapController(@NonNull Context context) {
         fontFileParser = new FontFileParser();
         fontFileParser.parse();
 
-        mapPointer = nativeInit(this, assetManager);
+        mapPointer = nativeInit(assetManager);
         if (mapPointer <= 0) {
             throw new RuntimeException("Unable to create a native Map object! There may be insufficient memory available.");
         }
@@ -1257,7 +1257,7 @@ boolean setMarkerDrawOrder(final long markerId, final int drawOrder) {
     // ==============
 
     private synchronized native void nativeOnLowMemory(long mapPtr);
-    private synchronized native long nativeInit(MapController instance, AssetManager assetManager);
+    private synchronized native long nativeInit(AssetManager assetManager);
     private synchronized native void nativeDispose(long mapPtr);
     private synchronized native int nativeLoadScene(long mapPtr, String path, String[] updateStrings);
     private synchronized native int nativeLoadSceneAsync(long mapPtr, String path, String[] updateStrings);

platforms/ios/framework/src/iosPlatform.h

@@ -11,6 +11,7 @@ class iOSPlatform : public Platform {
 public:
 
     iOSPlatform(__weak TGMapView* _mapView);
+    void shutdown() override {}
     void requestRender() const override;
     void setContinuousRendering(bool _isContinuous) override;
     std::vector<FontSourceHandle> systemFontFallbacksHandle() const override;