From 0aa0efe1599798d887fa6e33c412c09e81bea1bf Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Mon, 28 Aug 2023 10:27:38 -0700 Subject: [PATCH] Transition setFrameCompletedCallback to take a CallbackHandler (#7103) --- NEW_RELEASE_NOTES.md | 2 + .../src/main/cpp/SwapChain.cpp | 7 ++-- .../google/android/filament/SwapChain.java | 4 -- filament/backend/CMakeLists.txt | 1 - .../backend/include/backend/CallbackHandler.h | 2 +- .../backend/include/backend/DriverEnums.h | 2 - .../include/private/backend/DriverAPI.inc | 3 +- filament/backend/src/CallbackHandler.cpp | 23 ------------ filament/backend/src/metal/MetalDriver.mm | 4 +- filament/backend/src/metal/MetalHandles.h | 10 +++-- filament/backend/src/metal/MetalHandles.mm | 37 +++++++------------ filament/backend/src/noop/NoopDriver.cpp | 2 +- filament/backend/src/opengl/OpenGLDriver.cpp | 2 +- filament/backend/src/vulkan/VulkanDriver.cpp | 2 +- filament/include/filament/SwapChain.h | 21 ++++++++--- filament/src/SwapChain.cpp | 5 ++- filament/src/details/SwapChain.cpp | 20 +++++++++- filament/src/details/SwapChain.h | 6 ++- 18 files changed, 74 insertions(+), 79 deletions(-) delete mode 100644 filament/backend/src/CallbackHandler.cpp diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4a1a9c7fa7..df4c465690 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,3 +7,5 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut + +- `setFrameCompletedCallback` now takes a `backend::CallbackHandler`. diff --git a/android/filament-android/src/main/cpp/SwapChain.cpp b/android/filament-android/src/main/cpp/SwapChain.cpp index 3693803ce1..27e006ae87 100644 --- a/android/filament-android/src/main/cpp/SwapChain.cpp +++ b/android/filament-android/src/main/cpp/SwapChain.cpp @@ -27,11 +27,10 @@ extern "C" JNIEXPORT void JNICALL Java_com_google_android_filament_SwapChain_nSetFrameCompletedCallback(JNIEnv* env, jclass, jlong nativeSwapChain, jobject handler, jobject runnable) { SwapChain* swapChain = (SwapChain*) nativeSwapChain; - auto *callback = JniCallback::make(env, handler, runnable); - swapChain->setFrameCompletedCallback([](void* user) { - JniCallback* callback = (JniCallback*)user; + auto* callback = JniCallback::make(env, handler, runnable); + swapChain->setFrameCompletedCallback(nullptr, [callback](SwapChain* swapChain) { JniCallback::postToJavaAndDestroy(callback); - }, callback); + }); } extern "C" JNIEXPORT jboolean JNICALL diff --git a/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java b/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java index 6d621f02ff..9c0867fee2 100644 --- a/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java +++ b/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java @@ -137,10 +137,6 @@ public class SwapChain { *

* *

- * The FrameCompletedCallback is guaranteed to be called on the main Filament thread. - *

- * - *

* Warning: Only Filament's Metal backend supports frame callbacks. Other backends ignore the * callback (which will never be called) and proceed normally. *

diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index 108f65c537..9129099800 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -27,7 +27,6 @@ set(SRCS src/BackendUtils.cpp src/BlobCacheKey.cpp src/Callable.cpp - src/CallbackHandler.cpp src/CircularBuffer.cpp src/CommandBufferQueue.cpp src/CommandStream.cpp diff --git a/filament/backend/include/backend/CallbackHandler.h b/filament/backend/include/backend/CallbackHandler.h index dee3aaa251..3ffc707cdd 100644 --- a/filament/backend/include/backend/CallbackHandler.h +++ b/filament/backend/include/backend/CallbackHandler.h @@ -66,7 +66,7 @@ public: virtual void post(void* user, Callback callback) = 0; protected: - virtual ~CallbackHandler(); + virtual ~CallbackHandler() = default; }; } // namespace filament::backend diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 08f6abe94d..e25f18dafd 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -1147,8 +1147,6 @@ static_assert(sizeof(StencilState) == 12u, using FrameScheduledCallback = void(*)(PresentCallable callable, void* user); -using FrameCompletedCallback = void(*)(void* user); - enum class Workaround : uint16_t { // The EASU pass must split because shader compiler flattens early-exit branch SPLIT_EASU, diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index 053b342591..b447d765e8 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -142,7 +142,8 @@ DECL_DRIVER_API_N(setFrameScheduledCallback, DECL_DRIVER_API_N(setFrameCompletedCallback, backend::SwapChainHandle, sch, - backend::FrameCompletedCallback, callback, + backend::CallbackHandler*, handler, + backend::CallbackHandler::Callback, callback, void*, user) DECL_DRIVER_API_N(setPresentationTime, diff --git a/filament/backend/src/CallbackHandler.cpp b/filament/backend/src/CallbackHandler.cpp deleted file mode 100644 index a1c067b6d2..0000000000 --- a/filament/backend/src/CallbackHandler.cpp +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright (C) 2021 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include - -namespace filament::backend { - -CallbackHandler::~CallbackHandler() = default; - -} // namespace filament::backend diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 32ae1aa08c..1d036d90a0 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -176,9 +176,9 @@ void MetalDriver::setFrameScheduledCallback(Handle sch, } void MetalDriver::setFrameCompletedCallback(Handle sch, - FrameCompletedCallback callback, void* user) { + CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { auto* swapChain = handle_cast(sch); - swapChain->setFrameCompletedCallback(callback, user); + swapChain->setFrameCompletedCallback(handler, callback, user); } void MetalDriver::execute(std::function const& fn) noexcept { diff --git a/filament/backend/src/metal/MetalHandles.h b/filament/backend/src/metal/MetalHandles.h index 9ffa1a0bda..b129d478d7 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -70,7 +70,8 @@ public: void releaseDrawable(); void setFrameScheduledCallback(FrameScheduledCallback callback, void* user); - void setFrameCompletedCallback(FrameCompletedCallback callback, void* user); + void setFrameCompletedCallback(CallbackHandler* handler, + CallbackHandler::Callback callback, void* user); // For CAMetalLayer-backed SwapChains, presents the drawable or schedules a // FrameScheduledCallback. @@ -112,8 +113,11 @@ private: FrameScheduledCallback frameScheduledCallback = nullptr; void* frameScheduledUserData = nullptr; - FrameCompletedCallback frameCompletedCallback = nullptr; - void* frameCompletedUserData = nullptr; + struct { + CallbackHandler* handler = nullptr; + CallbackHandler::Callback callback = {}; + void* user = nullptr; + } frameCompleted; }; class MetalBufferObject : public HwBufferObject { diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index 99e8b36227..0b4d0b3c4d 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -194,13 +194,15 @@ void MetalSwapChain::setFrameScheduledCallback(FrameScheduledCallback callback, frameScheduledUserData = user; } -void MetalSwapChain::setFrameCompletedCallback(FrameCompletedCallback callback, void* user) { - frameCompletedCallback = callback; - frameCompletedUserData = user; +void MetalSwapChain::setFrameCompletedCallback(CallbackHandler* handler, + CallbackHandler::Callback callback, void* user) { + frameCompleted.handler = handler; + frameCompleted.callback = callback; + frameCompleted.user = user; } void MetalSwapChain::present() { - if (frameCompletedCallback) { + if (frameCompleted.callback) { scheduleFrameCompletedCallback(); } if (drawable) { @@ -244,30 +246,17 @@ void MetalSwapChain::scheduleFrameScheduledCallback() { } void MetalSwapChain::scheduleFrameCompletedCallback() { - if (!frameCompletedCallback) { + if (!frameCompleted.callback) { return; } - FrameCompletedCallback callback = frameCompletedCallback; - void* userData = frameCompletedUserData; - [getPendingCommandBuffer(&context) addCompletedHandler:^(id cb) { - struct CallbackData { - void* userData; - FrameCompletedCallback callback; - }; - CallbackData* data = new CallbackData(); - data->userData = userData; - data->callback = callback; + CallbackHandler* handler = frameCompleted.handler; + void* user = frameCompleted.user; + CallbackHandler::Callback callback = frameCompleted.callback; - // Instantiate a BufferDescriptor with a callback for the sole purpose of passing it to - // scheduleDestroy. This forces the BufferDescriptor callback (and thus the - // FrameCompletedCallback) to be called on the user thread. - BufferDescriptor b(nullptr, 0u, [](void* buffer, size_t size, void* user) { - CallbackData* data = (CallbackData*) user; - data->callback(data->userData); - free(data); - }, data); - context.driver->scheduleDestroy(std::move(b)); + MetalDriver* driver = context.driver; + [getPendingCommandBuffer(&context) addCompletedHandler:^(id cb) { + driver->scheduleCallback(handler, user, callback); }]; } diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index a2c6ef8d98..3d1a9cdc32 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -58,7 +58,7 @@ void NoopDriver::setFrameScheduledCallback(Handle sch, } void NoopDriver::setFrameCompletedCallback(Handle sch, - FrameCompletedCallback callback, void* user) { + CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { } diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index d53343499e..2f0e6f618a 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -3270,7 +3270,7 @@ void OpenGLDriver::setFrameScheduledCallback(Handle sch, } void OpenGLDriver::setFrameCompletedCallback(Handle sch, - FrameCompletedCallback callback, void* user) { + CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { DEBUG_MARKER() } diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 053d2665d7..3145d5df4a 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -293,7 +293,7 @@ void VulkanDriver::setFrameScheduledCallback(Handle sch, } void VulkanDriver::setFrameCompletedCallback(Handle sch, - FrameCompletedCallback callback, void* user) { + CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { } void VulkanDriver::setPresentationTime(int64_t monotonic_clock_ns) { diff --git a/filament/include/filament/SwapChain.h b/filament/include/filament/SwapChain.h index 9f7a328199..baa9ae58ca 100644 --- a/filament/include/filament/SwapChain.h +++ b/filament/include/filament/SwapChain.h @@ -18,10 +18,13 @@ #define TNT_FILAMENT_SWAPCHAIN_H #include + +#include #include #include #include +#include namespace filament { @@ -148,7 +151,7 @@ class Engine; class UTILS_PUBLIC SwapChain : public FilamentAPI { public: using FrameScheduledCallback = backend::FrameScheduledCallback; - using FrameCompletedCallback = backend::FrameCompletedCallback; + using FrameCompletedCallback = backend::CallbackHandler::Callback; /** * Requests a SwapChain with an alpha channel. @@ -241,17 +244,23 @@ public: * contents have completed rendering on the GPU. * * Use SwapChain::setFrameCompletedCallback to set a callback on an individual SwapChain. Each - * time a frame completes GPU rendering, the callback will be called with optional user data. + * time a frame completes GPU rendering, the callback will be called. * - * The FrameCompletedCallback is guaranteed to be called on the main Filament thread. + * If handler is nullptr, the callback is guaranteed to be called on the main Filament thread. * - * @param callback A callback, or nullptr to unset. - * @param user An optional pointer to user data passed to the callback function. + * Use \c setFrameCompletedCallback() (with default arguments) to unset the callback. + * + * @param handler Handler to dispatch the callback or nullptr for the default handler. + * @param callback Callback called when each frame completes. * * @remark Only Filament's Metal backend supports frame callbacks. Other backends ignore the * callback (which will never be called) and proceed normally. + * + * @see CallbackHandler */ - void setFrameCompletedCallback(FrameCompletedCallback callback, void* user = nullptr); + void setFrameCompletedCallback(backend::CallbackHandler* handler = nullptr, + utils::Invocable&& callback = {}) noexcept; + }; } // namespace filament diff --git a/filament/src/SwapChain.cpp b/filament/src/SwapChain.cpp index ae1498cc91..c30bce6941 100644 --- a/filament/src/SwapChain.cpp +++ b/filament/src/SwapChain.cpp @@ -28,8 +28,9 @@ void SwapChain::setFrameScheduledCallback(FrameScheduledCallback callback, void* return downcast(this)->setFrameScheduledCallback(callback, user); } -void SwapChain::setFrameCompletedCallback(FrameCompletedCallback callback, void* user) { - return downcast(this)->setFrameCompletedCallback(callback, user); +void SwapChain::setFrameCompletedCallback(backend::CallbackHandler* handler, + utils::Invocable&& callback) noexcept { + return downcast(this)->setFrameCompletedCallback(handler, std::move(callback)); } bool SwapChain::isSRGBSwapChainSupported(Engine& engine) noexcept { diff --git a/filament/src/details/SwapChain.cpp b/filament/src/details/SwapChain.cpp index ba13be2e2d..d9cb80911d 100644 --- a/filament/src/details/SwapChain.cpp +++ b/filament/src/details/SwapChain.cpp @@ -38,8 +38,24 @@ void FSwapChain::setFrameScheduledCallback(FrameScheduledCallback callback, void mEngine.getDriverApi().setFrameScheduledCallback(mSwapChain, callback, user); } -void FSwapChain::setFrameCompletedCallback(FrameCompletedCallback callback, void* user) { - mEngine.getDriverApi().setFrameCompletedCallback(mSwapChain, callback, user); +void FSwapChain::setFrameCompletedCallback(backend::CallbackHandler* handler, + utils::Invocable&& callback) noexcept { + struct Callback { + utils::Invocable f; + SwapChain* s; + static void func(void* user) { + auto* const c = reinterpret_cast(user); + c->f(c->s); + delete c; + } + }; + if (callback) { + auto* const user = new(std::nothrow) Callback{ std::move(callback), this }; + mEngine.getDriverApi().setFrameCompletedCallback( + mSwapChain, handler, &Callback::func, static_cast(user)); + } else { + mEngine.getDriverApi().setFrameCompletedCallback(mSwapChain, nullptr, nullptr, nullptr); + } } bool FSwapChain::isSRGBSwapChainSupported(FEngine& engine) noexcept { diff --git a/filament/src/details/SwapChain.h b/filament/src/details/SwapChain.h index c1a3f436d2..032b5e3f91 100644 --- a/filament/src/details/SwapChain.h +++ b/filament/src/details/SwapChain.h @@ -23,6 +23,9 @@ #include +#include + +#include #include namespace filament { @@ -61,7 +64,8 @@ public: void setFrameScheduledCallback(FrameScheduledCallback callback, void* user); - void setFrameCompletedCallback(FrameCompletedCallback callback, void* user); + void setFrameCompletedCallback(backend::CallbackHandler* handler, + utils::Invocable&& callback) noexcept; static bool isSRGBSwapChainSupported(FEngine& engine) noexcept;