Skip to content

Commit

Permalink
fix: Fix Choreographer breaking
Browse files Browse the repository at this point in the history
  • Loading branch information
mrousavy committed Feb 23, 2024
1 parent 10ea497 commit 4a6e778
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 12 deletions.
2 changes: 2 additions & 0 deletions package/android/src/main/cpp/Filament.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "FilamentInstaller.h"
#include "JChoreographer.h"
#include "JFilamentProxy.h"
#include "JFilamentView.h"
#include "JSurfaceProvider.h"
Expand All @@ -11,5 +12,6 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void*) {
margelo::JFilamentProxy::registerNatives();
margelo::JSurfaceProvider::registerNatives();
margelo::JFilamentView::registerNatives();
margelo::JChoreographer::registerNatives();
});
}
2 changes: 0 additions & 2 deletions package/android/src/main/cpp/java-bindings/JChoreographer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
4 changes: 3 additions & 1 deletion package/cpp/Choreographer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Listener> Choreographer::addOnFrameListener(Choreographer::OnFrameCallback onFrameCallback) {
Expand Down
1 change: 1 addition & 0 deletions package/cpp/FilamentProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion package/cpp/Listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<void()> remove);
~Listener();
void remove();
Expand Down
2 changes: 2 additions & 0 deletions package/src/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { testChoreographer } from './test/TestChoreographer'
import { testHybridObject } from './test/TestHybridObject'

export * from './FilamentView'

const TEST_HYBRID_OBJECT = true
if (__DEV__ && TEST_HYBRID_OBJECT) {
testHybridObject()
testChoreographer().catch(console.error)
}
15 changes: 14 additions & 1 deletion package/src/native/FilamentProxy.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
*/
Expand Down
29 changes: 29 additions & 0 deletions package/src/test/TestChoreographer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { FilamentProxy } from '../native/FilamentProxy'

function timeout(ms: number): Promise<void> {
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!')
}

0 comments on commit 4a6e778

Please sign in to comment.