Metal: annotate memory allocation failures with buffer tag (#8082)

filament/backend/src/metal/MetalBuffer.h

@@ -160,6 +160,8 @@ class MetalBuffer {
          size_t size, bool forceGpuBuffer = false);
     ~MetalBuffer();
 
+    [[nodiscard]] bool wasAllocationSuccessful() const noexcept { return mBuffer || mCpuBuffer; }
+
     MetalBuffer(const MetalBuffer& rhs) = delete;
     MetalBuffer& operator=(const MetalBuffer& rhs) = delete;
 

filament/backend/src/metal/MetalBuffer.mm

@@ -53,8 +53,8 @@
         mBuffer = { [context.device newBufferWithLength:size options:MTLResourceStorageModePrivate],
             TrackedMetalBuffer::Type::GENERIC };
     }
-    FILAMENT_CHECK_POSTCONDITION(mBuffer)
-            << "Could not allocate Metal buffer of size " << size << ".";
+    // mBuffer might fail to be allocated. Clients can check for this by calling
+    // wasAllocationSuccessful().
 }
 
 MetalBuffer::~MetalBuffer() {

filament/backend/src/metal/MetalDriver.h

@@ -57,7 +57,6 @@ class MetalDriver final : public DriverBase {
 
 public:
     static Driver* create(MetalPlatform* platform, const Platform::DriverConfig& driverConfig);
-    void runAtNextTick(const std::function<void()>& fn) noexcept;
 
 private:
 
@@ -73,10 +72,12 @@ class MetalDriver final : public DriverBase {
 
     /*
      * Tasks run regularly on the driver thread.
+     * Not thread-safe; tasks are run from the driver thead and must be enqueued from the driver
+     * thread.
      */
+    void runAtNextTick(const std::function<void()>& fn) noexcept;
     void executeTickOps() noexcept;
     std::vector<std::function<void()>> mTickOps;
-    std::mutex mTickOpsLock;
 
     /*
      * Driver interface

filament/backend/src/metal/MetalDriver.mm

@@ -320,17 +320,40 @@
         uint32_t vertexCount, Handle<HwVertexBufferInfo> vbih) {
     MetalVertexBufferInfo const* const vbi = handle_cast<const MetalVertexBufferInfo>(vbih);
     construct_handle<MetalVertexBuffer>(vbh, *mContext, vertexCount, vbi->bufferCount, vbih);
+    // No actual GPU memory is allocated here, so no need to check for allocation success.
 }
 
 void MetalDriver::createIndexBufferR(Handle<HwIndexBuffer> ibh, ElementType elementType,
         uint32_t indexCount, BufferUsage usage) {
-    auto elementSize = (uint8_t) getElementTypeSize(elementType);
-    construct_handle<MetalIndexBuffer>(ibh, *mContext, usage, elementSize, indexCount);
+    auto elementSize = (uint8_t)getElementTypeSize(elementType);
+    auto* indexBuffer =
+            construct_handle<MetalIndexBuffer>(ibh, *mContext, usage, elementSize, indexCount);
+    auto& buffer = indexBuffer->buffer;
+    // If the allocation was not successful, postpone the error message until the next tick, to give
+    // Filament a chance to call setDebugTag on the handle; this way we get a nicer error message.
+    if (UTILS_UNLIKELY(!buffer.wasAllocationSuccessful())) {
+        const size_t byteCount = buffer.getSize();
+        runAtNextTick([byteCount, this, ibh]() {
+            FILAMENT_CHECK_POSTCONDITION(false)
+                    << "Could not allocate Metal index buffer of size " << byteCount
+                    << ", tag=" << mHandleAllocator.getHandleTag(ibh.getId()).c_str_safe();
+        });
+    }
 }
 
 void MetalDriver::createBufferObjectR(Handle<HwBufferObject> boh, uint32_t byteCount,
         BufferObjectBinding bindingType, BufferUsage usage) {
-    construct_handle<MetalBufferObject>(boh, *mContext, bindingType, usage, byteCount);
+    auto* bufferObject =
+            construct_handle<MetalBufferObject>(boh, *mContext, bindingType, usage, byteCount);
+    // If the allocation was not successful, postpone the error message until the next tick, to give
+    // Filament a chance to call setDebugTag on the handle; this way we get a nicer error message.
+    if (UTILS_UNLIKELY(!bufferObject->getBuffer()->wasAllocationSuccessful())) {
+        runAtNextTick([byteCount, this, boh]() {
+            FILAMENT_CHECK_POSTCONDITION(false)
+                    << "Could not allocate Metal buffer of size " << byteCount
+                    << ", tag=" << mHandleAllocator.getHandleTag(boh.getId()).c_str_safe();
+        });
+    }
 }
 
 void MetalDriver::createTextureR(Handle<HwTexture> th, SamplerType target, uint8_t levels,
@@ -2066,15 +2089,12 @@
 }
 
 void MetalDriver::runAtNextTick(const std::function<void()>& fn) noexcept {
-    std::lock_guard<std::mutex> const lock(mTickOpsLock);
     mTickOps.push_back(fn);
 }
 
 void MetalDriver::executeTickOps() noexcept {
     std::vector<std::function<void()>> ops;
-    mTickOpsLock.lock();
     std::swap(ops, mTickOps);
-    mTickOpsLock.unlock();
     for (const auto& f : ops) {
         f();
     }

filament/backend/src/metal/MetalHandles.mm

@@ -257,10 +257,6 @@ static inline MTLTextureUsage getMetalTextureUsage(TextureUsage usage) {
     }
 }
 
-#ifndef FILAMENT_RELEASE_PRESENT_DRAWABLE_MAIN_THREAD
-#define FILAMENT_RELEASE_PRESENT_DRAWABLE_MAIN_THREAD 1
-#endif
-
 class PresentDrawableData {
 public:
     PresentDrawableData() = delete;
@@ -279,14 +275,10 @@ static void maybePresentAndDestroyAsync(PresentDrawableData* that, bool shouldPr
            [that->mDrawable present];
         }
 
-#if FILAMENT_RELEASE_PRESENT_DRAWABLE_MAIN_THREAD == 1
         // 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); });
-#else
-        that->mDriver->runAtNextTick([that]() { cleanupAndDestroy(that); });
-#endif
     }
 
 private: