From 41f7dc384b0b37941f5943fa2166df1a65aa0bbe Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 23 Jan 2025 14:24:16 -0800 Subject: [PATCH] add a version of Engine::flushAndWait that takes a timeout The old version that doesn't take a timeout now always waits forever; before it waited for a few seconds on Android debug builds. The new version has an explicit timeout that works on all platforms and returns success status. FIXES=[384043020] --- .../filament-android/src/main/cpp/Engine.cpp | 6 +-- .../com/google/android/filament/Engine.java | 25 ++++++++++- filament/include/filament/Engine.h | 19 +++++++++ filament/src/Engine.cpp | 4 ++ filament/src/details/Engine.cpp | 42 +++++-------------- filament/src/details/Engine.h | 1 + 6 files changed, 60 insertions(+), 37 deletions(-) diff --git a/android/filament-android/src/main/cpp/Engine.cpp b/android/filament-android/src/main/cpp/Engine.cpp index 56aecc66dd98..d6e941209bcf 100644 --- a/android/filament-android/src/main/cpp/Engine.cpp +++ b/android/filament-android/src/main/cpp/Engine.cpp @@ -392,11 +392,11 @@ Java_com_google_android_filament_Engine_nIsValidSwapChain(JNIEnv*, jclass, return (jboolean)engine->isValid((SwapChain*)nativeSwapChain); } -extern "C" JNIEXPORT void JNICALL +extern "C" JNIEXPORT jboolean JNICALL Java_com_google_android_filament_Engine_nFlushAndWait(JNIEnv*, jclass, - jlong nativeEngine) { + jlong nativeEngine, jlong timeout) { Engine* engine = (Engine*) nativeEngine; - engine->flushAndWait(); + return engine->flushAndWait((uint64_t)timeout); } extern "C" JNIEXPORT void JNICALL diff --git a/android/filament-android/src/main/java/com/google/android/filament/Engine.java b/android/filament-android/src/main/java/com/google/android/filament/Engine.java index 1e49bf54a332..e73e82944c0a 100644 --- a/android/filament-android/src/main/java/com/google/android/filament/Engine.java +++ b/android/filament-android/src/main/java/com/google/android/filament/Engine.java @@ -1271,7 +1271,28 @@ public EntityManager getEntityManager() { * {@link android.view.SurfaceHolder.Callback#surfaceDestroyed surfaceDestroyed}.

*/ public void flushAndWait() { - nFlushAndWait(getNativeObject()); + flushAndWait(Fence.WAIT_FOR_EVER); + } + + /** + * Kicks the hardware thread (e.g. the OpenGL, Vulkan or Metal thread) and blocks until + * all commands to this point are executed. Note that does guarantee that the + * hardware is actually finished. + * + * A timeout can be specified, if for some reason this flushAndWait doesn't complete before the timeout, it will + * return false, true otherwise. + * + *

This is typically used right after destroying the SwapChain, + * in cases where a guarantee about the SwapChain destruction is needed in a + * timely fashion, such as when responding to Android's + * android.view.SurfaceHolder.Callback.surfaceDestroyed

+ * + * @param timeout A timeout in nanoseconds + * @return true if successful, false if flushAndWait timed out, in which case it wasn't successful and commands + * might still be executing on both the CPU and GPU sides. + */ + public boolean flushAndWait(long timeout) { + return nFlushAndWait(getNativeObject(), timeout); } /** @@ -1437,7 +1458,7 @@ private static void assertDestroy(boolean success) { private static native boolean nIsValidRenderTarget(long nativeEngine, long nativeTarget); private static native boolean nIsValidSwapChain(long nativeEngine, long nativeSwapChain); private static native void nDestroyEntity(long nativeEngine, int entity); - private static native void nFlushAndWait(long nativeEngine); + private static native boolean nFlushAndWait(long nativeEngine, long timeout); private static native void nFlush(long nativeEngine); private static native boolean nIsPaused(long nativeEngine); private static native void nSetPaused(long nativeEngine, boolean paused); diff --git a/filament/include/filament/Engine.h b/filament/include/filament/Engine.h index 8fb44af5dc35..5092e16c385b 100644 --- a/filament/include/filament/Engine.h +++ b/filament/include/filament/Engine.h @@ -970,6 +970,25 @@ class UTILS_PUBLIC Engine { */ void flushAndWait(); + /** + * Kicks the hardware thread (e.g. the OpenGL, Vulkan or Metal thread) and blocks until + * all commands to this point are executed. Note that does guarantee that the + * hardware is actually finished. + * + * A timeout can be specified, if for some reason this flushAndWait doesn't complete before the timeout, it will + * return false, true otherwise. + * + *

This is typically used right after destroying the SwapChain, + * in cases where a guarantee about the SwapChain destruction is needed in a + * timely fashion, such as when responding to Android's + * android.view.SurfaceHolder.Callback.surfaceDestroyed

+ * + * @param timeout A timeout in nanoseconds + * @return true if successful, false if flushAndWait timed out, in which case it wasn't successful and commands + * might still be executing on both the CPU and GPU sides. + */ + bool flushAndWait(uint64_t timeout); + /** * Kicks the hardware thread (e.g. the OpenGL, Vulkan or Metal thread) but does not wait * for commands to be either executed or the hardware finished. diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp index 5f1af80aaf52..455067739ce3 100644 --- a/filament/src/Engine.cpp +++ b/filament/src/Engine.cpp @@ -338,6 +338,10 @@ void Engine::flushAndWait() { downcast(this)->flushAndWait(); } +bool Engine::flushAndWait(uint64_t timeout) { + return downcast(this)->flushAndWait(timeout); +} + void Engine::flush() { downcast(this)->flush(); } diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index 1cc9a951be11..2116fae4c1b6 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -665,50 +665,28 @@ void FEngine::flush() { } void FEngine::flushAndWait() { - FILAMENT_CHECK_PRECONDITION(!mCommandBufferQueue.isPaused()) - << "Cannot call flushAndWait() when rendering thread is paused!"; + flushAndWait(FENCE_WAIT_FOR_EVER); +} -#if defined(__ANDROID__) +bool FEngine::flushAndWait(uint64_t timeout) { + FILAMENT_CHECK_PRECONDITION(!mCommandBufferQueue.isPaused()) + << "Cannot call Engine::flushAndWait() when rendering thread is paused!"; // first make sure we've not terminated filament FILAMENT_CHECK_PRECONDITION(!mCommandBufferQueue.isExitRequested()) - << "calling Engine::flushAndWait() after Engine::shutdown()!"; - -#endif + << "Calling Engine::flushAndWait() after Engine::shutdown()!"; // enqueue finish command -- this will stall in the driver until the GPU is done getDriverApi().finish(); -#if defined(__ANDROID__) && !defined(NDEBUG) - - // then create a fence that will trigger when we're past the finish() above - size_t tryCount = 8; - FFence* fence = FEngine::createFence(); - UTILS_NOUNROLL - do { - FenceStatus status = fence->wait(FFence::Mode::FLUSH,250000000u); - // if the fence didn't trigger after 250ms, check that the command queue thread is still - // running (otherwise indicating a precondition violation). - if (UTILS_UNLIKELY(status == FenceStatus::TIMEOUT_EXPIRED)) { - FILAMENT_CHECK_PRECONDITION(!mCommandBufferQueue.isExitRequested()) - << "called Engine::shutdown() WHILE in Engine::flushAndWait()!"; - tryCount--; - FILAMENT_CHECK_POSTCONDITION(tryCount) << "flushAndWait() failed inexplicably after 2s"; - // if the thread is still running, maybe we just need to give it more time - continue; - } - break; - } while (true); + FFence* fence = createFence(); + FenceStatus const status = fence->wait(FFence::Mode::FLUSH, timeout); destroy(fence); -#else - - FFence::waitAndDestroy(createFence(), FFence::Mode::FLUSH); - -#endif - // finally, execute callbacks that might have been scheduled getDriver().purge(); + + return status == FenceStatus::CONDITION_SATISFIED; } // ----------------------------------------------------------------------------------------------- diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index d52907dc760a..28376a5b6bee 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -394,6 +394,7 @@ class FEngine : public Engine { void setPaused(bool paused); void flushAndWait(); + bool flushAndWait(uint64_t timeout); // flush the current buffer void flush();