diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index 6a3cb5cd0d..34cc4b5d59 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -417,6 +417,7 @@ if (APPLE OR LINUX) test/test_MissingRequiredAttributes.cpp test/test_ReadPixels.cpp test/test_BufferUpdates.cpp + test/test_Callbacks.cpp test/test_MRT.cpp test/test_LoadImage.cpp test/test_StencilBuffer.cpp diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index 6132cf64a7..bb64f3d436 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -144,8 +144,7 @@ DECL_DRIVER_API_N(setFrameScheduledCallback, DECL_DRIVER_API_N(setFrameCompletedCallback, backend::SwapChainHandle, sch, backend::CallbackHandler*, handler, - backend::CallbackHandler::Callback, callback, - void*, user) + utils::Invocable&&, callback) DECL_DRIVER_API_N(setPresentationTime, int64_t, monotonic_clock_ns) diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index db9af5b163..0172be1ce1 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -243,10 +243,10 @@ void MetalDriver::setFrameScheduledCallback( swapChain->setFrameScheduledCallback(handler, std::move(callback)); } -void MetalDriver::setFrameCompletedCallback(Handle sch, - CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { +void MetalDriver::setFrameCompletedCallback( + Handle sch, CallbackHandler* handler, utils::Invocable&& callback) { auto* swapChain = handle_cast(sch); - swapChain->setFrameCompletedCallback(handler, callback, user); + swapChain->setFrameCompletedCallback(handler, std::move(callback)); } 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 c6c65e1f7d..05ce7529d2 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -75,7 +75,7 @@ public: void setFrameScheduledCallback(CallbackHandler* handler, FrameScheduledCallback&& callback); void setFrameCompletedCallback( - CallbackHandler* handler, CallbackHandler::Callback callback, void* user); + CallbackHandler* handler, utils::Invocable&& callback); // For CAMetalLayer-backed SwapChains, presents the drawable or schedules a // FrameScheduledCallback. @@ -119,13 +119,12 @@ private: // PresentCallable object. struct { CallbackHandler* handler = nullptr; - FrameScheduledCallback callback = {}; + std::shared_ptr callback = nullptr; } frameScheduled; struct { CallbackHandler* handler = nullptr; - CallbackHandler::Callback callback = {}; - void* user = nullptr; + std::shared_ptr> callback = nullptr; } frameCompleted; }; diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index 15958ab60e..2b770241ab 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -224,14 +224,13 @@ void MetalSwapChain::ensureDepthStencilTexture() { void MetalSwapChain::setFrameScheduledCallback( CallbackHandler* handler, FrameScheduledCallback&& callback) { frameScheduled.handler = handler; - frameScheduled.callback = std::move(callback); + frameScheduled.callback = std::make_shared(std::move(callback)); } -void MetalSwapChain::setFrameCompletedCallback(CallbackHandler* handler, - CallbackHandler::Callback callback, void* user) { +void MetalSwapChain::setFrameCompletedCallback( + CallbackHandler* handler, utils::Invocable&& callback) { frameCompleted.handler = handler; - frameCompleted.callback = callback; - frameCompleted.user = user; + frameCompleted.callback = std::make_shared>(std::move(callback)); } void MetalSwapChain::present() { @@ -304,17 +303,17 @@ void MetalSwapChain::scheduleFrameScheduledCallback() { assert_invariant(drawable); struct Callback { - Callback(FrameScheduledCallback&& callback, id drawable, + Callback(std::shared_ptr callback, id drawable, MetalDriver* driver) - : f(std::move(callback)), data(PresentDrawableData::create(drawable, driver)) {} - FrameScheduledCallback f; + : f(callback), data(PresentDrawableData::create(drawable, driver)) {} + std::shared_ptr f; // PresentDrawableData* is destroyed by maybePresentAndDestroyAsync() later. std::unique_ptr data; static void func(void* user) { auto* const c = reinterpret_cast(user); PresentDrawableData* presentDrawableData = c->data.release(); PresentCallable presentCallable(presentDrawable, presentDrawableData); - c->f(presentCallable); + c->f->operator()(presentCallable); delete c; } }; @@ -322,7 +321,7 @@ void MetalSwapChain::scheduleFrameScheduledCallback() { // This callback pointer will be captured by the block. Even if the scheduled handler is never // called, the unique_ptr will still ensure we don't leak memory. __block auto callback = - std::make_unique(std::move(frameScheduled.callback), drawable, context.driver); + std::make_unique(frameScheduled.callback, drawable, context.driver); backend::CallbackHandler* handler = frameScheduled.handler; MetalDriver* driver = context.driver; @@ -337,13 +336,25 @@ void MetalSwapChain::scheduleFrameCompletedCallback() { return; } - CallbackHandler* handler = frameCompleted.handler; - void* user = frameCompleted.user; - CallbackHandler::Callback callback = frameCompleted.callback; + struct Callback { + Callback(std::shared_ptr> callback) : f(callback) {} + std::shared_ptr> f; + static void func(void* user) { + auto* const c = reinterpret_cast(user); + c->f->operator()(); + delete c; + } + }; + // This callback pointer will be captured by the block. Even if the completed handler is never + // called, the unique_ptr will still ensure we don't leak memory. + __block auto callback = std::make_unique(frameCompleted.callback); + + CallbackHandler* handler = frameCompleted.handler; MetalDriver* driver = context.driver; [getPendingCommandBuffer(&context) addCompletedHandler:^(id cb) { - driver->scheduleCallback(handler, user, callback); + Callback* user = callback.release(); + driver->scheduleCallback(handler, user, &Callback::func); }]; } diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index 145d602798..4b4b73fe18 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -59,7 +59,7 @@ void NoopDriver::setFrameScheduledCallback(Handle sch, } void NoopDriver::setFrameCompletedCallback(Handle sch, - CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { + CallbackHandler* handler, utils::Invocable&& callback) { } diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 5b60a75532..38d82a6866 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -3469,7 +3469,7 @@ void OpenGLDriver::setFrameScheduledCallback(Handle sch, } void OpenGLDriver::setFrameCompletedCallback(Handle sch, - CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { + CallbackHandler* handler, utils::Invocable&& callback) { DEBUG_MARKER() } diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 488243dd77..e0f62822bf 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -398,7 +398,7 @@ void VulkanDriver::setFrameScheduledCallback(Handle sch, } void VulkanDriver::setFrameCompletedCallback(Handle sch, - CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { + CallbackHandler* handler, utils::Invocable&& callback) { } void VulkanDriver::setPresentationTime(int64_t monotonic_clock_ns) { diff --git a/filament/backend/test/test_Callbacks.cpp b/filament/backend/test/test_Callbacks.cpp new file mode 100644 index 0000000000..0f02f1b214 --- /dev/null +++ b/filament/backend/test/test_Callbacks.cpp @@ -0,0 +1,122 @@ +/* + * Copyright (C) 2024 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 "BackendTest.h" + +using namespace filament; +using namespace filament::backend; + +namespace test { + +TEST_F(BackendTest, FrameScheduledCallback) { + auto& api = getDriverApi(); + + // Create a SwapChain. + // In order for the frameScheduledCallback to be called, this must be a real SwapChain (not + // headless) so we obtain a drawable. + auto swapChain = createSwapChain(); + + Handle renderTarget = api.createDefaultRenderTarget(); + + int callbackCountA = 0; + api.setFrameScheduledCallback(swapChain, nullptr, [&callbackCountA](PresentCallable callable) { + callable(); + callbackCountA++; + }); + + // Render the first frame. + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0, 0); + api.beginRenderPass(renderTarget, {}); + api.endRenderPass(0); + api.commit(swapChain); + api.endFrame(0); + + // Render the next frame. The same callback should be called. + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0, 0); + api.beginRenderPass(renderTarget, {}); + api.endRenderPass(0); + api.commit(swapChain); + api.endFrame(0); + + // Now switch out the callback. + int callbackCountB = 0; + api.setFrameScheduledCallback(swapChain, nullptr, [&callbackCountB](PresentCallable callable) { + callable(); + callbackCountB++; + }); + + // Render one final frame. + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0, 0); + api.beginRenderPass(renderTarget, {}); + api.endRenderPass(0); + api.commit(swapChain); + api.endFrame(0); + + api.finish(); + + executeCommands(); + getDriver().purge(); + + EXPECT_EQ(callbackCountA, 2); + EXPECT_EQ(callbackCountB, 1); +} + +TEST_F(BackendTest, FrameCompletedCallback) { + auto& api = getDriverApi(); + + // Create a SwapChain. + auto swapChain = api.createSwapChainHeadless(256, 256, 0); + + int callbackCountA = 0; + api.setFrameCompletedCallback(swapChain, nullptr, + [&callbackCountA]() { callbackCountA++; }); + + // Render the first frame. + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0, 0); + api.commit(swapChain); + api.endFrame(0); + + // Render the next frame. The same callback should be called. + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0, 0); + api.commit(swapChain); + api.endFrame(0); + + // Now switch out the callback. + int callbackCountB = 0; + api.setFrameCompletedCallback(swapChain, nullptr, + [&callbackCountB]() { callbackCountB++; }); + + // Render one final frame. + api.makeCurrent(swapChain, swapChain); + api.beginFrame(0, 0, 0); + api.commit(swapChain); + api.endFrame(0); + + api.finish(); + + executeCommands(); + getDriver().purge(); + + EXPECT_EQ(callbackCountA, 2); + EXPECT_EQ(callbackCountB, 1); +} + +} // namespace test diff --git a/filament/src/details/SwapChain.cpp b/filament/src/details/SwapChain.cpp index 0407d89376..20589c38f1 100644 --- a/filament/src/details/SwapChain.cpp +++ b/filament/src/details/SwapChain.cpp @@ -79,24 +79,11 @@ bool FSwapChain::isFrameScheduledCallbackSet() const noexcept { return mFrameScheduledCallbackIsSet; } -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( - mHwSwapChain, handler, &Callback::func, static_cast(user)); - } else { - mEngine.getDriverApi().setFrameCompletedCallback(mHwSwapChain, nullptr, nullptr, nullptr); - } +void FSwapChain::setFrameCompletedCallback( + backend::CallbackHandler* handler, FrameCompletedCallback&& callback) noexcept { + using namespace std::placeholders; + auto boundCallback = std::bind(std::move(callback), this); + mEngine.getDriverApi().setFrameCompletedCallback(mHwSwapChain, handler, std::move(boundCallback)); } bool FSwapChain::isSRGBSwapChainSupported(FEngine& engine) noexcept {