Improve setFrameScheduled threading (#8139)

Author: bejado

filament/backend/include/private/backend/DriverAPI.inc

@@ -139,7 +139,8 @@ DECL_DRIVER_API_N(beginFrame,
 DECL_DRIVER_API_N(setFrameScheduledCallback,
         backend::SwapChainHandle, sch,
         backend::CallbackHandler*, handler,
-        backend::FrameScheduledCallback&&, callback)
+        backend::FrameScheduledCallback&&, callback,
+        uint64_t, flags)
 
 DECL_DRIVER_API_N(setFrameCompletedCallback,
         backend::SwapChainHandle, sch,

filament/backend/src/metal/MetalDriver.mm

@@ -19,6 +19,8 @@
 #include "CommandStreamDispatcher.h"
 #include "metal/MetalDriver.h"
 
+#include <filament/SwapChain.h>
+
 #include "MetalBlitter.h"
 #include "MetalBufferPool.h"
 #include "MetalContext.h"
@@ -240,10 +242,14 @@
     }
 }
 
-void MetalDriver::setFrameScheduledCallback(
-        Handle<HwSwapChain> sch, CallbackHandler* handler, FrameScheduledCallback&& callback) {
+void MetalDriver::setFrameScheduledCallback(Handle<HwSwapChain> sch, CallbackHandler* handler,
+        FrameScheduledCallback&& callback, uint64_t flags) {
+    // Turn off the CALLBACK_DEFAULT_USE_METAL_COMPLETION_HANDLER flag if a custom handler is provided.
+    if (handler) {
+        flags &= ~SwapChain::CALLBACK_DEFAULT_USE_METAL_COMPLETION_HANDLER;
+    }
     auto* swapChain = handle_cast<MetalSwapChain>(sch);
-    swapChain->setFrameScheduledCallback(handler, std::move(callback));
+    swapChain->setFrameScheduledCallback(handler, std::move(callback), flags);
 }
 
 void MetalDriver::setFrameCompletedCallback(

filament/backend/src/metal/MetalHandles.h

@@ -73,7 +73,8 @@ class MetalSwapChain : public HwSwapChain {
 
     void releaseDrawable();
 
-    void setFrameScheduledCallback(CallbackHandler* handler, FrameScheduledCallback&& callback);
+    void setFrameScheduledCallback(
+            CallbackHandler* handler, FrameScheduledCallback&& callback, uint64_t flags);
     void setFrameCompletedCallback(
             CallbackHandler* handler, utils::Invocable<void(void)>&& callback);
 
@@ -121,6 +122,7 @@ class MetalSwapChain : public HwSwapChain {
     struct {
         CallbackHandler* handler = nullptr;
         std::shared_ptr<FrameScheduledCallback> callback = nullptr;
+        uint64_t flags = 0;
     } frameScheduled;
 
     struct {

filament/backend/src/metal/MetalHandles.mm

@@ -233,9 +233,10 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
 }
 
 void MetalSwapChain::setFrameScheduledCallback(
-        CallbackHandler* handler, FrameScheduledCallback&& callback) {
+        CallbackHandler* handler, FrameScheduledCallback&& callback, uint64_t flags) {
     frameScheduled.handler = handler;
     frameScheduled.callback = std::make_shared<FrameScheduledCallback>(std::move(callback));
+    frameScheduled.flags = flags;
 }
 
 void MetalSwapChain::setFrameCompletedCallback(
@@ -264,27 +265,34 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
     PresentDrawableData& operator=(const PresentDrawableData&) = delete;
 
     static PresentDrawableData* create(id<CAMetalDrawable> drawable,
-            std::shared_ptr<std::mutex> drawableMutex, MetalDriver* driver) {
+            std::shared_ptr<std::mutex> drawableMutex, MetalDriver* driver, uint64_t flags) {
         assert_invariant(drawableMutex);
         assert_invariant(driver);
-        return new PresentDrawableData(drawable, drawableMutex, driver);
+        return new PresentDrawableData(drawable, drawableMutex, driver, flags);
     }
 
     static void maybePresentAndDestroyAsync(PresentDrawableData* that, bool shouldPresent) {
         if (shouldPresent) {
            [that->mDrawable present];
         }
 
-        // mDrawable is acquired on the driver thread. Typically, we would release this object on
-        // the same thread, but after receiving consistent crash reports from within
-        // [CAMetalDrawable dealloc], we suspect this object requires releasing on the main thread.
-        dispatch_async(dispatch_get_main_queue(), ^{ cleanupAndDestroy(that); });
+        if (that->mFlags & SwapChain::CALLBACK_DEFAULT_USE_METAL_COMPLETION_HANDLER) {
+            cleanupAndDestroy(that);
+        } else {
+            // mDrawable is acquired on the driver thread. Typically, we would release this object
+            // on the same thread, but after receiving consistent crash reports from within
+            // [CAMetalDrawable dealloc], we suspect this object requires releasing on the main
+            // thread.
+            dispatch_async(dispatch_get_main_queue(), ^{
+                cleanupAndDestroy(that);
+            });
+        }
     }
 
 private:
     PresentDrawableData(id<CAMetalDrawable> drawable, std::shared_ptr<std::mutex> drawableMutex,
-            MetalDriver* driver)
-        : mDrawable(drawable), mDrawableMutex(drawableMutex), mDriver(driver) {}
+            MetalDriver* driver, uint64_t flags)
+        : mDrawable(drawable), mDrawableMutex(drawableMutex), mDriver(driver), mFlags(flags) {}
 
     static void cleanupAndDestroy(PresentDrawableData *that) {
         if (that->mDrawable) {
@@ -299,6 +307,7 @@ static void cleanupAndDestroy(PresentDrawableData *that) {
     id<CAMetalDrawable> mDrawable;
     std::shared_ptr<std::mutex> mDrawableMutex;
     MetalDriver* mDriver = nullptr;
+    uint64_t mFlags = 0;
 };
 
 void presentDrawable(bool presentFrame, void* user) {
@@ -315,8 +324,8 @@ void presentDrawable(bool presentFrame, void* user) {
 
     struct Callback {
         Callback(std::shared_ptr<FrameScheduledCallback> callback, id<CAMetalDrawable> drawable,
-                 std::shared_ptr<std::mutex> drawableMutex, MetalDriver* driver)
-            : f(callback), data(PresentDrawableData::create(drawable, drawableMutex, driver)) {}
+                 std::shared_ptr<std::mutex> drawableMutex, MetalDriver* driver, uint64_t flags)
+            : f(callback), data(PresentDrawableData::create(drawable, drawableMutex, driver, flags)) {}
         std::shared_ptr<FrameScheduledCallback> f;
         // PresentDrawableData* is destroyed by maybePresentAndDestroyAsync() later.
         std::unique_ptr<PresentDrawableData> data;
@@ -331,14 +340,19 @@ static void func(void* user) {
 
     // 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.
+    uint64_t const flags = frameScheduled.flags;
     __block auto callback = std::make_unique<Callback>(
-        frameScheduled.callback, drawable, layerDrawableMutex, context.driver);
+            frameScheduled.callback, drawable, layerDrawableMutex, context.driver, flags);
 
     backend::CallbackHandler* handler = frameScheduled.handler;
     MetalDriver* driver = context.driver;
     [getPendingCommandBuffer(&context) addScheduledHandler:^(id<MTLCommandBuffer> cb) {
         Callback* user = callback.release();
-        driver->scheduleCallback(handler, user, &Callback::func);
+        if (flags & SwapChain::CALLBACK_DEFAULT_USE_METAL_COMPLETION_HANDLER) {
+            Callback::func(user);
+        } else {
+            driver->scheduleCallback(handler, user, &Callback::func);
+        }
     }];
 }
 

filament/backend/src/noop/NoopDriver.cpp

@@ -54,7 +54,7 @@ void NoopDriver::beginFrame(int64_t monotonic_clock_ns,
 }
 
 void NoopDriver::setFrameScheduledCallback(Handle<HwSwapChain> sch,
-        CallbackHandler* handler, FrameScheduledCallback&& callback) {
+        CallbackHandler* handler, FrameScheduledCallback&& callback, uint64_t flags) {
 
 }
 

filament/backend/src/opengl/OpenGLDriver.cpp

@@ -3528,8 +3528,8 @@ void OpenGLDriver::beginFrame(
     }
 }
 
-void OpenGLDriver::setFrameScheduledCallback(Handle<HwSwapChain> sch,
-        CallbackHandler* handler, FrameScheduledCallback&& callback) {
+void OpenGLDriver::setFrameScheduledCallback(Handle<HwSwapChain> sch, CallbackHandler* handler,
+        FrameScheduledCallback&& callback, uint64_t flags) {
     DEBUG_MARKER()
 }
 

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -403,8 +403,8 @@ void VulkanDriver::beginFrame(int64_t monotonic_clock_ns,
     FVK_SYSTRACE_END();
 }
 
-void VulkanDriver::setFrameScheduledCallback(Handle<HwSwapChain> sch,
-        CallbackHandler* handler, FrameScheduledCallback&& callback) {
+void VulkanDriver::setFrameScheduledCallback(Handle<HwSwapChain> sch, CallbackHandler* handler,
+        FrameScheduledCallback&& callback, uint64_t flags) {
 }
 
 void VulkanDriver::setFrameCompletedCallback(Handle<HwSwapChain> sch,

filament/backend/test/test_Callbacks.cpp

@@ -35,7 +35,7 @@ TEST_F(BackendTest, FrameScheduledCallback) {
     api.setFrameScheduledCallback(swapChain, nullptr, [&callbackCountA](PresentCallable callable) {
         callable();
         callbackCountA++;
-    });
+    }, 0);
 
     // Render the first frame.
     api.makeCurrent(swapChain, swapChain);
@@ -58,7 +58,7 @@ TEST_F(BackendTest, FrameScheduledCallback) {
     api.setFrameScheduledCallback(swapChain, nullptr, [&callbackCountB](PresentCallable callable) {
         callable();
         callbackCountB++;
-    });
+    }, 0);
 
     // Render one final frame.
     api.makeCurrent(swapChain, swapChain);

filament/include/filament/SwapChain.h

@@ -254,6 +254,19 @@ class UTILS_PUBLIC SwapChain : public FilamentAPI {
 
     void* UTILS_NULLABLE getNativeWindow() const noexcept;
 
+    /**
+     * If this flag is passed to setFrameScheduledCallback, then the behavior of the default
+     * CallbackHandler (when nullptr is passed as the handler argument) is altered to call the
+     * callback on the Metal completion handler thread (as opposed to the main Filament thread).
+     * This flag also instructs the Metal backend to release the associated CAMetalDrawable on the
+     * completion handler thread.
+     *
+     * This flag has no effect if a custom CallbackHandler is passed.
+     *
+     * @see setFrameScheduledCallback
+     */
+    static constexpr uint64_t CALLBACK_DEFAULT_USE_METAL_COMPLETION_HANDLER = 1;
+
     /**
      * FrameScheduledCallback is a callback function that notifies an application when Filament has
      * completed processing a frame and that frame is ready to be scheduled for presentation.
@@ -298,7 +311,7 @@ class UTILS_PUBLIC SwapChain : public FilamentAPI {
      * @see PresentCallable
      */
     void setFrameScheduledCallback(backend::CallbackHandler* UTILS_NULLABLE handler = nullptr,
-            FrameScheduledCallback&& callback = {});
+            FrameScheduledCallback&& callback = {}, uint64_t flags = 0);
 
     /**
      * Returns whether or not this SwapChain currently has a FrameScheduledCallback set.

filament/src/SwapChain.cpp

@@ -29,8 +29,8 @@ void* SwapChain::getNativeWindow() const noexcept {
 }
 
 void SwapChain::setFrameScheduledCallback(
-        backend::CallbackHandler* handler, FrameScheduledCallback&& callback) {
-    downcast(this)->setFrameScheduledCallback(handler, std::move(callback));
+        backend::CallbackHandler* handler, FrameScheduledCallback&& callback, uint64_t flags) {
+    downcast(this)->setFrameScheduledCallback(handler, std::move(callback), flags);
 }
 
 bool SwapChain::isFrameScheduledCallbackSet() const noexcept {

filament/src/details/SwapChain.cpp

@@ -70,9 +70,10 @@ void FSwapChain::terminate(FEngine& engine) noexcept {
 }
 
 void FSwapChain::setFrameScheduledCallback(
-        backend::CallbackHandler* handler, FrameScheduledCallback&& callback) {
+        backend::CallbackHandler* handler, FrameScheduledCallback&& callback, uint64_t flags) {
     mFrameScheduledCallbackIsSet = bool(callback);
-    mEngine.getDriverApi().setFrameScheduledCallback(mHwSwapChain, handler, std::move(callback));
+    mEngine.getDriverApi().setFrameScheduledCallback(
+            mHwSwapChain, handler, std::move(callback), flags);
 }
 
 bool FSwapChain::isFrameScheduledCallbackSet() const noexcept {

filament/src/details/SwapChain.h

@@ -79,7 +79,7 @@ class FSwapChain : public SwapChain {
     }
 
     void setFrameScheduledCallback(
-            backend::CallbackHandler* handler, FrameScheduledCallback&& callback);
+            backend::CallbackHandler* handler, FrameScheduledCallback&& callback, uint64_t flags);
 
     bool isFrameScheduledCallbackSet() const noexcept;