From 9e984ebc13b9e67abf7e4fcf63ffe9fba8262dae Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 17:41:39 +0100 Subject: [PATCH 01/12] Create `References::adoptRef` and `References::adoptEngineRef` --- package/cpp/References.h | 53 ++++++++++++++++++++++++++++++ package/cpp/core/EngineWrapper.cpp | 19 ++++------- package/cpp/core/EngineWrapper.h | 4 +-- 3 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 package/cpp/References.h diff --git a/package/cpp/References.h b/package/cpp/References.h new file mode 100644 index 00000000..8ac0f989 --- /dev/null +++ b/package/cpp/References.h @@ -0,0 +1,53 @@ +// +// Created by Marc Rousavy on 23.02.24. +// + +#pragma once + +#include +#include +#include + +namespace margelo { + +template +struct References { +public: + using CleanupRefFunction = std::function; + using CleanupEngineRefFunction = std::function engine, T* ref)>; + + struct Deleter { + public: + explicit Deleter(const CleanupRefFunction& cleanup) : _cleanup(std::move(cleanup)) {} + void operator()(T* ref) { + _cleanup(ref); + } + private: + CleanupRefFunction _cleanup; + }; + + /** + * Adopt a raw reference to T and take over it's memory ownership. + * When the resulting `shared_ptr` is deleted, `cleanup` will be called with the reference to T, and the caller is expected to properly clean up T. + * @param value The raw reference to T that will be adopted. + * @param cleanup The function to run when the shared_ptr's ref count reaches zero. The caller is expected to delete the reference to T here. + * @return A shared_ptr safely managing the reference to T. + */ + static std::shared_ptr adoptRef(T *value, const CleanupRefFunction& cleanup) { + return std::shared_ptr(value, Deleter(std::move(cleanup))); + } + + /** + * Same as `adoptRef(T*, CleanupRefFunction)`, but with an additional argument: `Engine`. + * This is used to clean up children of `Engine`, e.g. via `Engine::destroy(Renderer)`. + */ + static std::shared_ptr adoptEngineRef(std::shared_ptr engine, T* value, CleanupEngineRefFunction cleanup) { + return adoptRef(value, [engine, cleanup = std::move(cleanup)] (T* ref) { + cleanup(engine, ref); + }); + } + +}; + +} // namespace margelo + diff --git a/package/cpp/core/EngineWrapper.cpp b/package/cpp/core/EngineWrapper.cpp index 860283c8..3d587d90 100644 --- a/package/cpp/core/EngineWrapper.cpp +++ b/package/cpp/core/EngineWrapper.cpp @@ -6,18 +6,17 @@ #include #include +#include "References.h" namespace margelo { EngineWrapper::EngineWrapper(filament::Engine::Backend backend) { - _engine = Engine::create(backend); + _engine = References::adoptRef(Engine::create(backend), [](Engine* engine) { + engine->destroy(&engine); + }); } EngineWrapper::~EngineWrapper() { - if (_swapChain) { - _engine->destroy(_swapChain); - } - _engine->destroy(&_engine); } void EngineWrapper::loadHybridMethods() { @@ -39,17 +38,11 @@ void EngineWrapper::setSurfaceProvider(std::shared_ptr surfaceP void EngineWrapper::setSurface(std::shared_ptr surface) { void* nativeWindow = surface->getSurface(); - if (_swapChain == nullptr || _swapChain->getNativeWindow() != nativeWindow) { - destroySurface(); - _swapChain = _engine->createSwapChain(nativeWindow); - } + // TODO: do something with surface } void EngineWrapper::destroySurface() { - if (_swapChain != nullptr) { - _engine->destroy(_swapChain); - _swapChain = nullptr; - } + // TODO: destroy surface } } // namespace margelo \ No newline at end of file diff --git a/package/cpp/core/EngineWrapper.h b/package/cpp/core/EngineWrapper.h index b251c866..277fe466 100644 --- a/package/cpp/core/EngineWrapper.h +++ b/package/cpp/core/EngineWrapper.h @@ -8,6 +8,7 @@ #include "SurfaceProvider.h" #include #include +#include "References.h" #include "jsi/HybridObject.h" @@ -29,8 +30,7 @@ class EngineWrapper : public HybridObject { void destroySurface(); private: - Engine* _engine; - SwapChain* _swapChain; + std::shared_ptr _engine; std::shared_ptr _surfaceProvider; std::unique_ptr _listener; }; From 837885e75c99bc16c859decb22043e5b1770ed2f Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 17:43:50 +0100 Subject: [PATCH 02/12] Format --- package/cpp/References.h | 70 +++++++++++++++--------------- package/cpp/core/EngineWrapper.cpp | 11 ++--- package/cpp/core/EngineWrapper.h | 2 +- 3 files changed, 39 insertions(+), 44 deletions(-) diff --git a/package/cpp/References.h b/package/cpp/References.h index 8ac0f989..d6d54c3f 100644 --- a/package/cpp/References.h +++ b/package/cpp/References.h @@ -4,50 +4,48 @@ #pragma once +#include #include #include -#include namespace margelo { -template -struct References { +template struct References { public: - using CleanupRefFunction = std::function; - using CleanupEngineRefFunction = std::function engine, T* ref)>; - - struct Deleter { - public: - explicit Deleter(const CleanupRefFunction& cleanup) : _cleanup(std::move(cleanup)) {} - void operator()(T* ref) { - _cleanup(ref); - } - private: - CleanupRefFunction _cleanup; - }; - - /** - * Adopt a raw reference to T and take over it's memory ownership. - * When the resulting `shared_ptr` is deleted, `cleanup` will be called with the reference to T, and the caller is expected to properly clean up T. - * @param value The raw reference to T that will be adopted. - * @param cleanup The function to run when the shared_ptr's ref count reaches zero. The caller is expected to delete the reference to T here. - * @return A shared_ptr safely managing the reference to T. - */ - static std::shared_ptr adoptRef(T *value, const CleanupRefFunction& cleanup) { - return std::shared_ptr(value, Deleter(std::move(cleanup))); - } - - /** - * Same as `adoptRef(T*, CleanupRefFunction)`, but with an additional argument: `Engine`. - * This is used to clean up children of `Engine`, e.g. via `Engine::destroy(Renderer)`. - */ - static std::shared_ptr adoptEngineRef(std::shared_ptr engine, T* value, CleanupEngineRefFunction cleanup) { - return adoptRef(value, [engine, cleanup = std::move(cleanup)] (T* ref) { - cleanup(engine, ref); - }); + using CleanupRefFunction = std::function; + using CleanupEngineRefFunction = std::function engine, T* ref)>; + + struct Deleter { + public: + explicit Deleter(const CleanupRefFunction& cleanup) : _cleanup(std::move(cleanup)) {} + void operator()(T* ref) { + _cleanup(ref); } + private: + CleanupRefFunction _cleanup; + }; + + /** + * Adopt a raw reference to T and take over it's memory ownership. + * When the resulting `shared_ptr` is deleted, `cleanup` will be called with the reference to T, and the caller is expected to properly + * clean up T. + * @param value The raw reference to T that will be adopted. + * @param cleanup The function to run when the shared_ptr's ref count reaches zero. The caller is expected to delete the reference to T + * here. + * @return A shared_ptr safely managing the reference to T. + */ + static std::shared_ptr adoptRef(T* value, const CleanupRefFunction& cleanup) { + return std::shared_ptr(value, Deleter(std::move(cleanup))); + } + + /** + * Same as `adoptRef(T*, CleanupRefFunction)`, but with an additional argument: `Engine`. + * This is used to clean up children of `Engine`, e.g. via `Engine::destroy(Renderer)`. + */ + static std::shared_ptr adoptEngineRef(std::shared_ptr engine, T* value, CleanupEngineRefFunction cleanup) { + return adoptRef(value, [engine, cleanup = std::move(cleanup)](T* ref) { cleanup(engine, ref); }); + } }; } // namespace margelo - diff --git a/package/cpp/core/EngineWrapper.cpp b/package/cpp/core/EngineWrapper.cpp index 3d587d90..ace4f59a 100644 --- a/package/cpp/core/EngineWrapper.cpp +++ b/package/cpp/core/EngineWrapper.cpp @@ -4,20 +4,17 @@ #include "EngineWrapper.h" +#include "References.h" #include #include -#include "References.h" namespace margelo { EngineWrapper::EngineWrapper(filament::Engine::Backend backend) { - _engine = References::adoptRef(Engine::create(backend), [](Engine* engine) { - engine->destroy(&engine); - }); + _engine = References::adoptRef(Engine::create(backend), [](Engine* engine) { engine->destroy(&engine); }); } -EngineWrapper::~EngineWrapper() { -} +EngineWrapper::~EngineWrapper() {} void EngineWrapper::loadHybridMethods() { registerHybridMethod("setSurfaceProvider", &EngineWrapper::setSurfaceProvider, this); @@ -42,7 +39,7 @@ void EngineWrapper::setSurface(std::shared_ptr surface) { } void EngineWrapper::destroySurface() { - // TODO: destroy surface + // TODO: destroy surface } } // namespace margelo \ No newline at end of file diff --git a/package/cpp/core/EngineWrapper.h b/package/cpp/core/EngineWrapper.h index 277fe466..96415c10 100644 --- a/package/cpp/core/EngineWrapper.h +++ b/package/cpp/core/EngineWrapper.h @@ -4,11 +4,11 @@ #pragma once +#include "References.h" #include "Surface.h" #include "SurfaceProvider.h" #include #include -#include "References.h" #include "jsi/HybridObject.h" From c7148a30bff2011ae3e713a4bb24a62eb92cd4a8 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 17:48:57 +0100 Subject: [PATCH 03/12] Move `Engine` sp --- package/cpp/References.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/cpp/References.h b/package/cpp/References.h index d6d54c3f..e395a284 100644 --- a/package/cpp/References.h +++ b/package/cpp/References.h @@ -43,8 +43,8 @@ template struct References { * Same as `adoptRef(T*, CleanupRefFunction)`, but with an additional argument: `Engine`. * This is used to clean up children of `Engine`, e.g. via `Engine::destroy(Renderer)`. */ - static std::shared_ptr adoptEngineRef(std::shared_ptr engine, T* value, CleanupEngineRefFunction cleanup) { - return adoptRef(value, [engine, cleanup = std::move(cleanup)](T* ref) { cleanup(engine, ref); }); + static std::shared_ptr adoptEngineRef(const std::shared_ptr& engine, T* value, CleanupEngineRefFunction cleanup) { + return adoptRef(value, [engine = std::move(engine), cleanup = std::move(cleanup)](T* ref) { cleanup(engine, ref); }); } }; From 6088b7b58af3c226985af3978cc759062cafdd00 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 17:49:26 +0100 Subject: [PATCH 04/12] Remove unneeded destructor --- package/cpp/core/EngineWrapper.cpp | 2 -- package/cpp/core/EngineWrapper.h | 1 - 2 files changed, 3 deletions(-) diff --git a/package/cpp/core/EngineWrapper.cpp b/package/cpp/core/EngineWrapper.cpp index ace4f59a..ee193c6b 100644 --- a/package/cpp/core/EngineWrapper.cpp +++ b/package/cpp/core/EngineWrapper.cpp @@ -14,8 +14,6 @@ EngineWrapper::EngineWrapper(filament::Engine::Backend backend) { _engine = References::adoptRef(Engine::create(backend), [](Engine* engine) { engine->destroy(&engine); }); } -EngineWrapper::~EngineWrapper() {} - void EngineWrapper::loadHybridMethods() { registerHybridMethod("setSurfaceProvider", &EngineWrapper::setSurfaceProvider, this); } diff --git a/package/cpp/core/EngineWrapper.h b/package/cpp/core/EngineWrapper.h index 96415c10..2e0130a7 100644 --- a/package/cpp/core/EngineWrapper.h +++ b/package/cpp/core/EngineWrapper.h @@ -19,7 +19,6 @@ using namespace filament; class EngineWrapper : public HybridObject { public: explicit EngineWrapper(filament::Engine::Backend backend); - ~EngineWrapper(); void setSurfaceProvider(std::shared_ptr surfaceProvider); From 83c4c08eb94a30fdabe64ffe1368039af644ecb6 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 17:49:44 +0100 Subject: [PATCH 05/12] Update EngineWrapper.h --- package/cpp/core/EngineWrapper.h | 1 - 1 file changed, 1 deletion(-) diff --git a/package/cpp/core/EngineWrapper.h b/package/cpp/core/EngineWrapper.h index 2e0130a7..d3ccecb5 100644 --- a/package/cpp/core/EngineWrapper.h +++ b/package/cpp/core/EngineWrapper.h @@ -4,7 +4,6 @@ #pragma once -#include "References.h" #include "Surface.h" #include "SurfaceProvider.h" #include From a69b7fede1a6d26e3d6228d9bf36dd564b67dccb Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 18:20:59 +0100 Subject: [PATCH 06/12] feat: Create `Choreographer` --- package/android/CMakeLists.txt | 2 + .../src/main/cpp/AndroidFilamentProxy.cpp | 4 ++ .../src/main/cpp/AndroidFilamentProxy.h | 1 + .../main/cpp/java-bindings/JChoreographer.cpp | 39 ++++++++++++++++ .../main/cpp/java-bindings/JChoreographer.h | 33 ++++++++++++++ .../main/cpp/java-bindings/JFilamentProxy.cpp | 9 ++++ .../main/cpp/java-bindings/JFilamentProxy.h | 2 + .../filament/FilamentChoreographer.java | 45 +++++++++++++++++++ .../com/margelo/filament/FilamentProxy.java | 7 +++ package/cpp/Choreographer.cpp | 26 +++++++++++ package/cpp/Choreographer.h | 31 +++++++++++++ package/cpp/FilamentProxy.h | 4 +- package/cpp/Listener.cpp | 4 ++ package/cpp/Listener.h | 5 ++- package/cpp/SurfaceProvider.cpp | 9 +--- 15 files changed, 211 insertions(+), 10 deletions(-) create mode 100644 package/android/src/main/cpp/java-bindings/JChoreographer.cpp create mode 100644 package/android/src/main/cpp/java-bindings/JChoreographer.h create mode 100644 package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java create mode 100644 package/cpp/Choreographer.cpp create mode 100644 package/cpp/Choreographer.h diff --git a/package/android/CMakeLists.txt b/package/android/CMakeLists.txt index fc483bfc..5e893331 100644 --- a/package/android/CMakeLists.txt +++ b/package/android/CMakeLists.txt @@ -22,6 +22,7 @@ add_library( ../cpp/FilamentProxy.cpp ../cpp/Surface.cpp ../cpp/SurfaceProvider.cpp + ../cpp/Choreographer.cpp ../cpp/Listener.cpp ../cpp/jsi/HybridObject.cpp ../cpp/jsi/Promise.cpp @@ -34,6 +35,7 @@ add_library( src/main/cpp/JNISharedPtr.cpp src/main/cpp/FilamentInstaller.cpp src/main/cpp/java-bindings/JFilamentProxy.cpp + src/main/cpp/java-bindings/JChoreographer.cpp src/main/cpp/java-bindings/JFilamentView.cpp src/main/cpp/java-bindings/JSurfaceProvider.cpp ) diff --git a/package/android/src/main/cpp/AndroidFilamentProxy.cpp b/package/android/src/main/cpp/AndroidFilamentProxy.cpp index d4d4e545..4aed7b98 100644 --- a/package/android/src/main/cpp/AndroidFilamentProxy.cpp +++ b/package/android/src/main/cpp/AndroidFilamentProxy.cpp @@ -25,4 +25,8 @@ std::shared_ptr AndroidFilamentProxy::findFilamentView(int id) { return _proxy->cthis()->findFilamentView(id); } +std::shared_ptr AndroidFilamentProxy::createChoreographer() { + return _proxy->cthis()->createChoreographer(); +} + } // namespace margelo diff --git a/package/android/src/main/cpp/AndroidFilamentProxy.h b/package/android/src/main/cpp/AndroidFilamentProxy.h index a56b2c89..2981ac0f 100644 --- a/package/android/src/main/cpp/AndroidFilamentProxy.h +++ b/package/android/src/main/cpp/AndroidFilamentProxy.h @@ -25,6 +25,7 @@ class AndroidFilamentProxy : public FilamentProxy { // TODO(hanno): implement int loadModel(std::string path) override; std::shared_ptr findFilamentView(int id) override; + std::shared_ptr createChoreographer() override; private: jni::global_ref _proxy; diff --git a/package/android/src/main/cpp/java-bindings/JChoreographer.cpp b/package/android/src/main/cpp/java-bindings/JChoreographer.cpp new file mode 100644 index 00000000..d9f86b00 --- /dev/null +++ b/package/android/src/main/cpp/java-bindings/JChoreographer.cpp @@ -0,0 +1,39 @@ +// +// Created by Marc Rousavy on 23.02.24. +// + +#include "JChoreographer.h" +#include + +namespace margelo { + +void JChoreographer::registerNatives() { + registerHybrid({ + makeNativeMethod("initHybrid", JChoreographer::initHybrid), + makeNativeMethod("start", JChoreographer::start), + makeNativeMethod("stop", JChoreographer::stop), + makeNativeMethod("onFrame", JChoreographer::onFrameLong), + }); +} + +JChoreographer::JChoreographer(const jni::alias_ref& javaThis) : _javaPart(jni::make_global(javaThis)) {} + +jni::local_ref JChoreographer::initHybrid(jni::alias_ref javaThis) { + return makeCxxInstance(javaThis); +} + +void JChoreographer::onFrameLong(jlong timestamp) { + onFrame(static_cast(timestamp)); +} + +void JChoreographer::start() { + static const auto method = javaClassLocal()->getMethod("start"); + method(_javaPart); +} + +void JChoreographer::stop() { + static const auto method = javaClassLocal()->getMethod("stop"); + method(_javaPart); +} + +} // namespace margelo \ No newline at end of file diff --git a/package/android/src/main/cpp/java-bindings/JChoreographer.h b/package/android/src/main/cpp/java-bindings/JChoreographer.h new file mode 100644 index 00000000..8d776651 --- /dev/null +++ b/package/android/src/main/cpp/java-bindings/JChoreographer.h @@ -0,0 +1,33 @@ +// +// Created by Marc Rousavy on 23.02.24. +// + +#pragma once + +#include "Choreographer.h" +#include + +namespace margelo { + +using namespace facebook; + +class JChoreographer : public jni::HybridClass, public Choreographer { +public: + static void registerNatives(); + + void start() override; + void stop() override; + void onFrameLong(jlong timestamp); + +private: + friend HybridBase; + jni::global_ref _javaPart; + static auto constexpr TAG = "JChoreographer"; + static auto constexpr kJavaDescriptor = "Lcom/margelo/filament/FilamentChoreographer;"; + +private: + explicit JChoreographer(const jni::alias_ref& javaThis); + static jni::local_ref initHybrid(jni::alias_ref javaThis); +}; + +} // namespace margelo diff --git a/package/android/src/main/cpp/java-bindings/JFilamentProxy.cpp b/package/android/src/main/cpp/java-bindings/JFilamentProxy.cpp index b2655f21..8093dcbd 100644 --- a/package/android/src/main/cpp/java-bindings/JFilamentProxy.cpp +++ b/package/android/src/main/cpp/java-bindings/JFilamentProxy.cpp @@ -3,6 +3,7 @@ // #include "JFilamentProxy.h" +#include "JChoreographer.h" #include "JFilamentView.h" #include "JNISharedPtr.h" #include @@ -32,6 +33,14 @@ std::shared_ptr JFilamentProxy::findFilamentView(int id) { return std::static_pointer_cast(sharedRef); } +std::shared_ptr JFilamentProxy::createChoreographer() { + static const auto method = javaClassLocal()->getMethod()>("createChoreographer"); + jni::local_ref choreographer = method(_javaPart); + jni::global_ref globalRef = jni::make_global(choreographer); + std::shared_ptr sharedRef = JNISharedPtr::make_shared_from_jni(globalRef); + return std::static_pointer_cast(sharedRef); +} + jsi::Runtime& JFilamentProxy::getRuntime() { if (_runtime == nullptr) { [[unlikely]]; diff --git a/package/android/src/main/cpp/java-bindings/JFilamentProxy.h b/package/android/src/main/cpp/java-bindings/JFilamentProxy.h index 290ee98d..e5781452 100644 --- a/package/android/src/main/cpp/java-bindings/JFilamentProxy.h +++ b/package/android/src/main/cpp/java-bindings/JFilamentProxy.h @@ -4,6 +4,7 @@ #pragma once +#include "Choreographer.h" #include "FilamentView.h" #include #include @@ -25,6 +26,7 @@ class JFilamentProxy : public jni::HybridClass { // TODO(hanno): implement int loadModel(const std::string& path); std::shared_ptr findFilamentView(int id); + std::shared_ptr createChoreographer(); jsi::Runtime& getRuntime(); diff --git a/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java b/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java new file mode 100644 index 00000000..c5cba420 --- /dev/null +++ b/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java @@ -0,0 +1,45 @@ +package com.margelo.filament; + +import android.view.Choreographer; + +import androidx.annotation.Keep; + +import com.facebook.jni.HybridData; +import com.facebook.proguard.annotations.DoNotStrip; + +/** @noinspection JavaJniMissingFunction*/ +public class FilamentChoreographer { + /** @noinspection unused, FieldCanBeLocal */ + @DoNotStrip + @Keep + private final HybridData mHybridData; + private final Choreographer choreographer; + private final Choreographer.FrameCallback frameCallback; + private boolean isRunning; + + public FilamentChoreographer() { + mHybridData = initHybrid(); + choreographer = Choreographer.getInstance(); + frameCallback = timestamp -> { + if (!isRunning) return; + onFrame(timestamp); + }; + } + + private synchronized void start() { + if (!isRunning) { + isRunning = true; + choreographer.postFrameCallback(frameCallback); + } + } + + private synchronized void stop() { + if (isRunning) { + isRunning = false; + choreographer.removeFrameCallback(frameCallback); + } + } + + private native HybridData initHybrid(); + private native void onFrame(long timestamp); +} diff --git a/package/android/src/main/java/com/margelo/filament/FilamentProxy.java b/package/android/src/main/java/com/margelo/filament/FilamentProxy.java index 01113138..80c70787 100644 --- a/package/android/src/main/java/com/margelo/filament/FilamentProxy.java +++ b/package/android/src/main/java/com/margelo/filament/FilamentProxy.java @@ -36,6 +36,13 @@ class FilamentProxy { reactContext = context; } + /** @noinspection unused*/ + @DoNotStrip + @Keep + FilamentChoreographer createChoreographer() { + return new FilamentChoreographer(); + } + /** @noinspection unused*/ @DoNotStrip @Keep diff --git a/package/cpp/Choreographer.cpp b/package/cpp/Choreographer.cpp new file mode 100644 index 00000000..0ece67c2 --- /dev/null +++ b/package/cpp/Choreographer.cpp @@ -0,0 +1,26 @@ +// +// Created by Marc Rousavy on 23.02.24. +// + +#include "Choreographer.h" + +namespace margelo { + +void Choreographer::loadHybridMethods() { + registerHybridMethod("setOnFrameCallback", &Choreographer::addOnFrameListener, this); +} + +std::shared_ptr Choreographer::addOnFrameListener(Choreographer::OnFrameCallback onFrameCallback) { + _callbacks.push_back(std::move(onFrameCallback)); + return std::make_shared([]() { + // TODO: Find a safe way to remove this listener from the vector. + }); +} + +void Choreographer::onFrame(double timestamp) { + for (const auto& callback : _callbacks) { + callback(timestamp); + } +} + +} // namespace margelo \ No newline at end of file diff --git a/package/cpp/Choreographer.h b/package/cpp/Choreographer.h new file mode 100644 index 00000000..7f69d16a --- /dev/null +++ b/package/cpp/Choreographer.h @@ -0,0 +1,31 @@ +// +// Created by Marc Rousavy on 23.02.24. +// + +#pragma once + +#include "Listener.h" +#include "jsi/HybridObject.h" +#include + +namespace margelo { + +class Choreographer : public HybridObject { +public: + using OnFrameCallback = std::function; + + std::shared_ptr addOnFrameListener(OnFrameCallback onFrameCallback); + + virtual void start() = 0; + virtual void stop() = 0; + +protected: + void onFrame(double timestamp); + + void loadHybridMethods() override; + +private: + std::vector _callbacks; +}; + +} // namespace margelo diff --git a/package/cpp/FilamentProxy.h b/package/cpp/FilamentProxy.h index e29d428d..e5239fa4 100644 --- a/package/cpp/FilamentProxy.h +++ b/package/cpp/FilamentProxy.h @@ -4,11 +4,10 @@ #pragma once -#include - #include #include +#include "Choreographer.h" #include "FilamentView.h" #include "jsi/HybridObject.h" #include "test/TestHybridObject.h" @@ -21,6 +20,7 @@ class FilamentProxy : public HybridObject { private: virtual int loadModel(std::string path) = 0; virtual std::shared_ptr findFilamentView(int id) = 0; + virtual std::shared_ptr createChoreographer() = 0; std::shared_ptr createTestObject(); diff --git a/package/cpp/Listener.cpp b/package/cpp/Listener.cpp index 33bbae83..0345e47c 100644 --- a/package/cpp/Listener.cpp +++ b/package/cpp/Listener.cpp @@ -20,4 +20,8 @@ void Listener::remove() { _isRemoved = true; } +void Listener::loadHybridMethods() { + registerHybridMethod("remove", &Listener::remove, this); +} + } // namespace margelo \ No newline at end of file diff --git a/package/cpp/Listener.h b/package/cpp/Listener.h index 6157689d..eeefb3b0 100644 --- a/package/cpp/Listener.h +++ b/package/cpp/Listener.h @@ -4,16 +4,19 @@ #pragma once +#include "jsi/HybridObject.h" #include namespace margelo { -class Listener { +class Listener : public HybridObject { public: explicit Listener(std::function remove); ~Listener(); void remove(); + void loadHybridMethods() override; + private: std::function _remove; bool _isRemoved; diff --git a/package/cpp/SurfaceProvider.cpp b/package/cpp/SurfaceProvider.cpp index 4a8df03a..755da2ee 100644 --- a/package/cpp/SurfaceProvider.cpp +++ b/package/cpp/SurfaceProvider.cpp @@ -14,13 +14,8 @@ Listener SurfaceProvider::addOnSurfaceChangedListener(margelo::SurfaceProvider:: std::unique_lock lock(_mutex); _callbacks.push_back(std::move(callback)); - size_t index = _callbacks.size(); - - return Listener([weakThis = this, index]() { - if (weakThis != nullptr) { - std::unique_lock lock(weakThis->_mutex); - weakThis->_callbacks.erase(weakThis->_callbacks.begin() + index); - } + return Listener([]() { + // TODO: Find a safe way to remove this listener from the vector. }); } From 10ea497b6d397bf91843649ef7c9c7a7d5ca001a Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 18:27:31 +0100 Subject: [PATCH 07/12] fix: Fix Listener move constructor --- package/cpp/Listener.h | 1 + package/cpp/core/EngineWrapper.cpp | 2 +- package/cpp/core/EngineWrapper.h | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package/cpp/Listener.h b/package/cpp/Listener.h index eeefb3b0..c8aa47bb 100644 --- a/package/cpp/Listener.h +++ b/package/cpp/Listener.h @@ -11,6 +11,7 @@ namespace margelo { class Listener : public HybridObject { public: + Listener(Listener&& listener): _remove(std::move(listener._remove)), _isRemoved(listener._isRemoved) {} explicit Listener(std::function remove); ~Listener(); void remove(); diff --git a/package/cpp/core/EngineWrapper.cpp b/package/cpp/core/EngineWrapper.cpp index ee193c6b..ddbf356e 100644 --- a/package/cpp/core/EngineWrapper.cpp +++ b/package/cpp/core/EngineWrapper.cpp @@ -28,7 +28,7 @@ void EngineWrapper::setSurfaceProvider(std::shared_ptr surfaceP Listener listener = surfaceProvider->addOnSurfaceChangedListener( SurfaceProvider::Callback{.onSurfaceCreated = [=](std::shared_ptr surface) { this->setSurface(surface); }, .onSurfaceDestroyed = [=](std::shared_ptr surface) { this->destroySurface(); }}); - _listener = std::make_unique(std::move(listener)); + _listener = std::make_shared(std::move(listener)); } void EngineWrapper::setSurface(std::shared_ptr surface) { diff --git a/package/cpp/core/EngineWrapper.h b/package/cpp/core/EngineWrapper.h index d3ccecb5..56528fff 100644 --- a/package/cpp/core/EngineWrapper.h +++ b/package/cpp/core/EngineWrapper.h @@ -30,7 +30,7 @@ class EngineWrapper : public HybridObject { private: std::shared_ptr _engine; std::shared_ptr _surfaceProvider; - std::unique_ptr _listener; + std::shared_ptr _listener; }; } // namespace margelo From 4a6e778f226ddcd643697fb986446fe19a01658d Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 18:41:42 +0100 Subject: [PATCH 08/12] fix: Fix Choreographer breaking --- package/android/src/main/cpp/Filament.cpp | 2 ++ .../main/cpp/java-bindings/JChoreographer.cpp | 2 -- .../filament/FilamentChoreographer.java | 21 +++++++++----- package/cpp/Choreographer.cpp | 4 ++- package/cpp/FilamentProxy.cpp | 1 + package/cpp/Listener.h | 2 +- package/src/index.tsx | 2 ++ package/src/native/FilamentProxy.ts | 15 +++++++++- package/src/test/TestChoreographer.ts | 29 +++++++++++++++++++ 9 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 package/src/test/TestChoreographer.ts diff --git a/package/android/src/main/cpp/Filament.cpp b/package/android/src/main/cpp/Filament.cpp index 23612648..cf708b8f 100644 --- a/package/android/src/main/cpp/Filament.cpp +++ b/package/android/src/main/cpp/Filament.cpp @@ -1,4 +1,5 @@ #include "FilamentInstaller.h" +#include "JChoreographer.h" #include "JFilamentProxy.h" #include "JFilamentView.h" #include "JSurfaceProvider.h" @@ -11,5 +12,6 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void*) { margelo::JFilamentProxy::registerNatives(); margelo::JSurfaceProvider::registerNatives(); margelo::JFilamentView::registerNatives(); + margelo::JChoreographer::registerNatives(); }); } diff --git a/package/android/src/main/cpp/java-bindings/JChoreographer.cpp b/package/android/src/main/cpp/java-bindings/JChoreographer.cpp index d9f86b00..8e750915 100644 --- a/package/android/src/main/cpp/java-bindings/JChoreographer.cpp +++ b/package/android/src/main/cpp/java-bindings/JChoreographer.cpp @@ -10,8 +10,6 @@ namespace margelo { void JChoreographer::registerNatives() { registerHybrid({ makeNativeMethod("initHybrid", JChoreographer::initHybrid), - makeNativeMethod("start", JChoreographer::start), - makeNativeMethod("stop", JChoreographer::stop), makeNativeMethod("onFrame", JChoreographer::onFrameLong), }); } diff --git a/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java b/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java index c5cba420..0abe8811 100644 --- a/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java +++ b/package/android/src/main/java/com/margelo/filament/FilamentChoreographer.java @@ -14,29 +14,36 @@ public class FilamentChoreographer { @Keep private final HybridData mHybridData; private final Choreographer choreographer; - private final Choreographer.FrameCallback frameCallback; private boolean isRunning; public FilamentChoreographer() { mHybridData = initHybrid(); choreographer = Choreographer.getInstance(); - frameCallback = timestamp -> { - if (!isRunning) return; - onFrame(timestamp); - }; } + private void onFrameCallback(long timestamp) { + if (!isRunning) return; + onFrame(timestamp); + choreographer.postFrameCallback(this::onFrameCallback); + } + + /** @noinspection unused */ + @DoNotStrip + @Keep private synchronized void start() { if (!isRunning) { isRunning = true; - choreographer.postFrameCallback(frameCallback); + choreographer.postFrameCallback(this::onFrameCallback); } } + /** @noinspection unused */ + @DoNotStrip + @Keep private synchronized void stop() { if (isRunning) { isRunning = false; - choreographer.removeFrameCallback(frameCallback); + choreographer.removeFrameCallback(this::onFrameCallback); } } diff --git a/package/cpp/Choreographer.cpp b/package/cpp/Choreographer.cpp index 0ece67c2..36373b99 100644 --- a/package/cpp/Choreographer.cpp +++ b/package/cpp/Choreographer.cpp @@ -7,7 +7,9 @@ namespace margelo { void Choreographer::loadHybridMethods() { - registerHybridMethod("setOnFrameCallback", &Choreographer::addOnFrameListener, this); + registerHybridMethod("addOnFrameListener", &Choreographer::addOnFrameListener, this); + registerHybridMethod("start", &Choreographer::start, this); + registerHybridMethod("stop", &Choreographer::stop, this); } std::shared_ptr Choreographer::addOnFrameListener(Choreographer::OnFrameCallback onFrameCallback) { diff --git a/package/cpp/FilamentProxy.cpp b/package/cpp/FilamentProxy.cpp index 0783c964..c903f2f0 100644 --- a/package/cpp/FilamentProxy.cpp +++ b/package/cpp/FilamentProxy.cpp @@ -22,6 +22,7 @@ void FilamentProxy::loadHybridMethods() { registerHybridMethod("loadModel", &FilamentProxy::loadModel, this); registerHybridMethod("findFilamentView", &FilamentProxy::findFilamentView, this); registerHybridMethod("createTestObject", &FilamentProxy::createTestObject, this); + registerHybridMethod("createChoreographer", &FilamentProxy::createChoreographer, this); } } // namespace margelo diff --git a/package/cpp/Listener.h b/package/cpp/Listener.h index c8aa47bb..a48f51b0 100644 --- a/package/cpp/Listener.h +++ b/package/cpp/Listener.h @@ -11,7 +11,7 @@ namespace margelo { class Listener : public HybridObject { public: - Listener(Listener&& listener): _remove(std::move(listener._remove)), _isRemoved(listener._isRemoved) {} + Listener(Listener&& listener) : _remove(std::move(listener._remove)), _isRemoved(listener._isRemoved) {} explicit Listener(std::function remove); ~Listener(); void remove(); diff --git a/package/src/index.tsx b/package/src/index.tsx index 9dd2f791..acd8cc09 100644 --- a/package/src/index.tsx +++ b/package/src/index.tsx @@ -1,3 +1,4 @@ +import { testChoreographer } from './test/TestChoreographer' import { testHybridObject } from './test/TestHybridObject' export * from './FilamentView' @@ -5,4 +6,5 @@ export * from './FilamentView' const TEST_HYBRID_OBJECT = true if (__DEV__ && TEST_HYBRID_OBJECT) { testHybridObject() + testChoreographer().catch(console.error) } diff --git a/package/src/native/FilamentProxy.ts b/package/src/native/FilamentProxy.ts index b78c30d9..c742c0aa 100644 --- a/package/src/native/FilamentProxy.ts +++ b/package/src/native/FilamentProxy.ts @@ -1,5 +1,15 @@ import { FilamentNativeModule } from './FilamentNativeModule' +interface Listener { + remove(): void +} + +interface Choreographer { + addOnFrameListener(onFrame: (timestamp: number) => void): Listener + start(): void + stop(): void +} + interface TestHybridObject { int: number string: string @@ -17,7 +27,10 @@ export interface TFilamentProxy { * @param path A web URL (http:// or https://), local file (file://) or resource ID. */ loadModel(path: string): number - + /** + * Create a new Choreographer instance running on the caller Thread. + */ + createChoreographer(): Choreographer /** * @private */ diff --git a/package/src/test/TestChoreographer.ts b/package/src/test/TestChoreographer.ts new file mode 100644 index 00000000..6b92ed1a --- /dev/null +++ b/package/src/test/TestChoreographer.ts @@ -0,0 +1,29 @@ +import { FilamentProxy } from '../native/FilamentProxy' + +function timeout(ms: number): Promise { + return new Promise((resolve) => { + setTimeout(() => resolve(), ms) + }) +} + +export async function testChoreographer() { + console.log('Creating Choreographer...') + const choreographer = FilamentProxy.createChoreographer() + console.log('Created Choreographer!', choreographer, 'Adding onFrame listener...') + + let calls = 0 + choreographer.addOnFrameListener((timestamp) => { + console.log(`Choreographer::onFrame(${timestamp})`) + calls++ + }) + choreographer.start() + await timeout(500) + console.log(`onFrame called ${calls} times in 500ms! Stopping...`) + const finalCalls = calls + choreographer.stop() + await timeout(500) + if (finalCalls > calls) { + throw new Error(`Choreographer::onFrame has been called ${finalCalls - calls} times after stoppingg!`) + } + console.log('Choreographer successfully stopped!') +} From 061df7ded33b55a6b97f4e3c2859e55881514cf7 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 18:59:00 +0100 Subject: [PATCH 09/12] feat: Implement Choreographer on iOS --- package/ios/src/AppleChoreographer.h | 36 ++++++++++++++++++ package/ios/src/AppleChoreographer.mm | 53 +++++++++++++++++++++++++++ package/ios/src/AppleFilamentProxy.h | 1 + package/ios/src/AppleFilamentProxy.mm | 6 +++ 4 files changed, 96 insertions(+) create mode 100644 package/ios/src/AppleChoreographer.h create mode 100644 package/ios/src/AppleChoreographer.mm diff --git a/package/ios/src/AppleChoreographer.h b/package/ios/src/AppleChoreographer.h new file mode 100644 index 00000000..8533e5d5 --- /dev/null +++ b/package/ios/src/AppleChoreographer.h @@ -0,0 +1,36 @@ +// +// AppleChoreographer.h +// Pods +// +// Created by Marc Rousavy on 23.02.24. +// + +#pragma once + +#include "Choreographer.h" +#import + +@interface DisplayLinkListener : NSObject +typedef void (^ OnFrameCallback)(double timestamp); + +- (instancetype) initWithCallback:(OnFrameCallback)callback; +- (void) start; +- (void) stop; +- (void) onFrame:(CADisplayLink*)displayLink; + +@end + +namespace margelo { + +class AppleChoreographer: public Choreographer { +public: + explicit AppleChoreographer(); + + void stop() override; + void start() override; + +private: + DisplayLinkListener* _displayLink; +}; + +} // namespace margelo diff --git a/package/ios/src/AppleChoreographer.mm b/package/ios/src/AppleChoreographer.mm new file mode 100644 index 00000000..d6e5203b --- /dev/null +++ b/package/ios/src/AppleChoreographer.mm @@ -0,0 +1,53 @@ +// +// AppleChoreographer.m +// DoubleConversion +// +// Created by Marc Rousavy on 23.02.24. +// + +#include "AppleChoreographer.h" +#import + +@implementation DisplayLinkListener { + CADisplayLink* _Nullable _displayLink; + OnFrameCallback _callback; +} +- (instancetype)initWithCallback:(OnFrameCallback)callback { + if (self = [super init]) { + _callback = callback; + } +} + +- (void)start { + _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(onFrame:)]; + [_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; +} + +- (void)stop { + [_displayLink invalidate]; + _displayLink = nil; +} + +- (void)onFrame:(CADisplayLink *)displayLink { + _callback(displayLink.timestamp); +} + +@end + +namespace margelo { + +AppleChoreographer::AppleChoreographer() { + _displayLink = [[DisplayLinkListener alloc] initWithCallback:^(double timestamp) { + onFrame(timestamp); + }]; +} + +void AppleChoreographer::stop() { + [_displayLink stop]; +} + +void AppleChoreographer::start() { + [_displayLink start]; +} + +} // namespace margelo diff --git a/package/ios/src/AppleFilamentProxy.h b/package/ios/src/AppleFilamentProxy.h index 81d694d1..e16cb5be 100644 --- a/package/ios/src/AppleFilamentProxy.h +++ b/package/ios/src/AppleFilamentProxy.h @@ -23,6 +23,7 @@ class AppleFilamentProxy : public FilamentProxy { public: int loadModel(std::string path) override; std::shared_ptr findFilamentView(int modelId) override; + std::shared_ptr createChoreographer() override; private: jsi::Runtime* _runtime; diff --git a/package/ios/src/AppleFilamentProxy.mm b/package/ios/src/AppleFilamentProxy.mm index 9498c128..b82eb436 100644 --- a/package/ios/src/AppleFilamentProxy.mm +++ b/package/ios/src/AppleFilamentProxy.mm @@ -7,6 +7,7 @@ #import "AppleFilamentProxy.h" #import "AppleFilamentView.h" +#import "AppleChoreographer.h" #import "FilamentMetalView.h" #import "FilamentView.h" #import @@ -42,4 +43,9 @@ return std::static_pointer_cast(result); } +std::shared_ptr AppleFilamentProxy::createChoreographer() { + std::shared_ptr choreographer = std::make_shared(); + return std::static_pointer_cast(choreographer); +} + } // namespace margelo From abfd52620804a1881c7c66c18949d5d66ddcf6c3 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 19:06:37 +0100 Subject: [PATCH 10/12] fix: Move DisplayLinkListener to separate .m file --- .../FilamentExample.xcodeproj/project.pbxproj | 10 ++++- package/ios/src/AppleChoreographer.h | 12 +----- package/ios/src/AppleChoreographer.mm | 27 -------------- package/ios/src/DisplayLinkListener.h | 21 +++++++++++ package/ios/src/DisplayLinkListener.m | 37 +++++++++++++++++++ 5 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 package/ios/src/DisplayLinkListener.h create mode 100644 package/ios/src/DisplayLinkListener.m diff --git a/package/example/ios/FilamentExample.xcodeproj/project.pbxproj b/package/example/ios/FilamentExample.xcodeproj/project.pbxproj index e611ca5d..75300f0e 100644 --- a/package/example/ios/FilamentExample.xcodeproj/project.pbxproj +++ b/package/example/ios/FilamentExample.xcodeproj/project.pbxproj @@ -582,7 +582,10 @@ "-DFOLLY_USE_LIBCPP=1", "-DFOLLY_CFG_NO_COROUTINES=1", ); - OTHER_LDFLAGS = "$(inherited) "; + OTHER_LDFLAGS = ( + "$(inherited)", + " ", + ); REACT_NATIVE_PATH = "${PODS_ROOT}/../../node_modules/react-native"; SDKROOT = iphoneos; USE_HERMES = true; @@ -650,7 +653,10 @@ "-DFOLLY_USE_LIBCPP=1", "-DFOLLY_CFG_NO_COROUTINES=1", ); - OTHER_LDFLAGS = "$(inherited) "; + OTHER_LDFLAGS = ( + "$(inherited)", + " ", + ); REACT_NATIVE_PATH = "${PODS_ROOT}/../../node_modules/react-native"; SDKROOT = iphoneos; USE_HERMES = true; diff --git a/package/ios/src/AppleChoreographer.h b/package/ios/src/AppleChoreographer.h index 8533e5d5..0cc62ea7 100644 --- a/package/ios/src/AppleChoreographer.h +++ b/package/ios/src/AppleChoreographer.h @@ -8,17 +8,7 @@ #pragma once #include "Choreographer.h" -#import - -@interface DisplayLinkListener : NSObject -typedef void (^ OnFrameCallback)(double timestamp); - -- (instancetype) initWithCallback:(OnFrameCallback)callback; -- (void) start; -- (void) stop; -- (void) onFrame:(CADisplayLink*)displayLink; - -@end +#include "DisplayLinkListener.h" namespace margelo { diff --git a/package/ios/src/AppleChoreographer.mm b/package/ios/src/AppleChoreographer.mm index d6e5203b..7b4861b9 100644 --- a/package/ios/src/AppleChoreographer.mm +++ b/package/ios/src/AppleChoreographer.mm @@ -6,33 +6,6 @@ // #include "AppleChoreographer.h" -#import - -@implementation DisplayLinkListener { - CADisplayLink* _Nullable _displayLink; - OnFrameCallback _callback; -} -- (instancetype)initWithCallback:(OnFrameCallback)callback { - if (self = [super init]) { - _callback = callback; - } -} - -- (void)start { - _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(onFrame:)]; - [_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; -} - -- (void)stop { - [_displayLink invalidate]; - _displayLink = nil; -} - -- (void)onFrame:(CADisplayLink *)displayLink { - _callback(displayLink.timestamp); -} - -@end namespace margelo { diff --git a/package/ios/src/DisplayLinkListener.h b/package/ios/src/DisplayLinkListener.h new file mode 100644 index 00000000..8a384443 --- /dev/null +++ b/package/ios/src/DisplayLinkListener.h @@ -0,0 +1,21 @@ +// +// DisplayLinkListener.h +// Pods +// +// Created by Marc Rousavy on 23.02.24. +// + +#pragma once + +#import +#import + +@interface DisplayLinkListener : NSObject +typedef void (^ OnFrameCallback)(double timestamp); + +- (instancetype) initWithCallback:(OnFrameCallback)callback; +- (void) start; +- (void) stop; +- (void) onFrame:(CADisplayLink*)displayLink; + +@end diff --git a/package/ios/src/DisplayLinkListener.m b/package/ios/src/DisplayLinkListener.m new file mode 100644 index 00000000..7b0f6dc2 --- /dev/null +++ b/package/ios/src/DisplayLinkListener.m @@ -0,0 +1,37 @@ +// +// DisplayLinkListener.m +// DoubleConversion +// +// Created by Marc Rousavy on 23.02.24. +// + +#import "DisplayLinkListener.h" +#import +#import + +@implementation DisplayLinkListener { + CADisplayLink* _Nullable _displayLink; + OnFrameCallback _callback; +} +- (instancetype)initWithCallback:(OnFrameCallback)callback { + if (self = [super init]) { + _callback = callback; + } + return self; +} + +- (void)start { + _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(onFrame:)]; + [_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode]; +} + +- (void)stop { + [_displayLink invalidate]; + _displayLink = nil; +} + +- (void)onFrame:(CADisplayLink *)displayLink { + _callback(displayLink.timestamp); +} + +@end From 33d4d2dac12c8361ea3c1113be721244b2d67142 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 19:22:09 +0100 Subject: [PATCH 11/12] fix: Run tests --- package/src/index.tsx | 9 ++------- package/src/test/RunTests.ts | 25 +++++++++++++++++++++++++ package/src/test/TestChoreographer.ts | 7 +++---- package/src/test/TestHybridObject.ts | 3 --- 4 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 package/src/test/RunTests.ts diff --git a/package/src/index.tsx b/package/src/index.tsx index acd8cc09..489d264a 100644 --- a/package/src/index.tsx +++ b/package/src/index.tsx @@ -1,10 +1,5 @@ -import { testChoreographer } from './test/TestChoreographer' -import { testHybridObject } from './test/TestHybridObject' +import { runTests } from './test/RunTests' export * from './FilamentView' -const TEST_HYBRID_OBJECT = true -if (__DEV__ && TEST_HYBRID_OBJECT) { - testHybridObject() - testChoreographer().catch(console.error) -} +runTests() diff --git a/package/src/test/RunTests.ts b/package/src/test/RunTests.ts new file mode 100644 index 00000000..81d2b182 --- /dev/null +++ b/package/src/test/RunTests.ts @@ -0,0 +1,25 @@ +import { testChoreographer } from './TestChoreographer' +import { testHybridObject } from './TestHybridObject' + +async function wrapTest(name: string, func: () => void | Promise): Promise { + console.log(`-------- BEGIN TEST: ${name}`) + try { + await func() + console.log(`-------- END TEST: ${name}`) + } catch (e) { + console.error(`-------- ERROR IN TEST ${name}:`, e) + } +} + +// TODO: Write proper tests for that that actually run on a device/simulator. +// I want to make sure Hybrid Objects and Choreographers all work fine and test those in the CI. +export function runTests() { + const TEST_HYBRID_OBJECTS = true + if (__DEV__ && TEST_HYBRID_OBJECTS) { + const run = async () => { + await wrapTest('HybridObject', testHybridObject) + await wrapTest('Choreographer', testChoreographer) + } + run() + } +} diff --git a/package/src/test/TestChoreographer.ts b/package/src/test/TestChoreographer.ts index 6b92ed1a..ac8027d6 100644 --- a/package/src/test/TestChoreographer.ts +++ b/package/src/test/TestChoreographer.ts @@ -12,13 +12,12 @@ export async function testChoreographer() { console.log('Created Choreographer!', choreographer, 'Adding onFrame listener...') let calls = 0 - choreographer.addOnFrameListener((timestamp) => { - console.log(`Choreographer::onFrame(${timestamp})`) + choreographer.addOnFrameListener((_timestamp) => { calls++ }) choreographer.start() - await timeout(500) - console.log(`onFrame called ${calls} times in 500ms! Stopping...`) + await timeout(1000) + console.log(`onFrame called ${calls} times in 1000ms, so it's running at ${calls} FPS! Stopping...`) const finalCalls = calls choreographer.stop() await timeout(500) diff --git a/package/src/test/TestHybridObject.ts b/package/src/test/TestHybridObject.ts index 02195d1e..5e637435 100644 --- a/package/src/test/TestHybridObject.ts +++ b/package/src/test/TestHybridObject.ts @@ -1,7 +1,6 @@ import { FilamentProxy } from '../native/FilamentProxy' export function testHybridObject() { - console.log('------ BEGIN HybridObject tests...') // 1. Creation console.log('Creating HybridObject...') const hybridObject = FilamentProxy.createTestObject() @@ -37,6 +36,4 @@ export function testHybridObject() { // 8. Create a new one const newObject = hybridObject.createNewHybridObject() console.log(`Created new hybrid object!`, newObject) - - console.log('------ END HybridObject tests!') } From 232d9b0871e3764a6ad1425598a1189ff3e0c749 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Fri, 23 Feb 2024 19:22:28 +0100 Subject: [PATCH 12/12] Format --- package/ios/src/AppleChoreographer.h | 6 +++--- package/ios/src/AppleFilamentProxy.mm | 2 +- package/ios/src/DisplayLinkListener.h | 10 +++++----- package/ios/src/DisplayLinkListener.m | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package/ios/src/AppleChoreographer.h b/package/ios/src/AppleChoreographer.h index 0cc62ea7..4b8b5021 100644 --- a/package/ios/src/AppleChoreographer.h +++ b/package/ios/src/AppleChoreographer.h @@ -12,13 +12,13 @@ namespace margelo { -class AppleChoreographer: public Choreographer { +class AppleChoreographer : public Choreographer { public: explicit AppleChoreographer(); - + void stop() override; void start() override; - + private: DisplayLinkListener* _displayLink; }; diff --git a/package/ios/src/AppleFilamentProxy.mm b/package/ios/src/AppleFilamentProxy.mm index b82eb436..fe2a826d 100644 --- a/package/ios/src/AppleFilamentProxy.mm +++ b/package/ios/src/AppleFilamentProxy.mm @@ -6,8 +6,8 @@ // #import "AppleFilamentProxy.h" -#import "AppleFilamentView.h" #import "AppleChoreographer.h" +#import "AppleFilamentView.h" #import "FilamentMetalView.h" #import "FilamentView.h" #import diff --git a/package/ios/src/DisplayLinkListener.h b/package/ios/src/DisplayLinkListener.h index 8a384443..925417fb 100644 --- a/package/ios/src/DisplayLinkListener.h +++ b/package/ios/src/DisplayLinkListener.h @@ -11,11 +11,11 @@ #import @interface DisplayLinkListener : NSObject -typedef void (^ OnFrameCallback)(double timestamp); +typedef void (^OnFrameCallback)(double timestamp); -- (instancetype) initWithCallback:(OnFrameCallback)callback; -- (void) start; -- (void) stop; -- (void) onFrame:(CADisplayLink*)displayLink; +- (instancetype)initWithCallback:(OnFrameCallback)callback; +- (void)start; +- (void)stop; +- (void)onFrame:(CADisplayLink*)displayLink; @end diff --git a/package/ios/src/DisplayLinkListener.m b/package/ios/src/DisplayLinkListener.m index 7b0f6dc2..d49b071d 100644 --- a/package/ios/src/DisplayLinkListener.m +++ b/package/ios/src/DisplayLinkListener.m @@ -30,7 +30,7 @@ - (void)stop { _displayLink = nil; } -- (void)onFrame:(CADisplayLink *)displayLink { +- (void)onFrame:(CADisplayLink*)displayLink { _callback(displayLink.timestamp); }