Vulkan: Handle errors when wrapping external images

This also introduces another combinator to ConsumeError for
ResultOrError.

BUG=dawn:19

Change-Id: Ic204313436f5e919473d604efd049fe3d3c27a66
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11862
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

src/dawn_native/Device.h

@@ -56,6 +56,16 @@ namespace dawn_native {
             return false;
         }
 
+        template <typename T>
+        bool ConsumedError(ResultOrError<T> resultOrError, T* result) {
+            if (DAWN_UNLIKELY(resultOrError.IsError())) {
+                ConsumeError(resultOrError.AcquireError());
+                return true;
+            }
+            *result = resultOrError.AcquireSuccess();
+            return false;
+        }
+
         MaybeError ValidateObject(const ObjectBase* object) const;
 
         AdapterBase* GetAdapter() const;

src/dawn_native/vulkan/DeviceVk.cpp

@@ -672,9 +672,14 @@ namespace dawn_native { namespace vulkan {
         std::vector<VkSemaphore> waitSemaphores;
         waitSemaphores.reserve(waitHandles.size());
 
-        // If failed, cleanup
+        // Cleanup in case of a failure, the image creation doesn't acquire the external objects
+        // if a failure happems.
+        Texture* result = nullptr;
         if (ConsumedError(ImportExternalImage(descriptor, memoryHandle, waitHandles,
-                                              &signalSemaphore, &allocation, &waitSemaphores))) {
+                                              &signalSemaphore, &allocation, &waitSemaphores)) ||
+            ConsumedError(Texture::CreateFromExternal(this, descriptor, textureDescriptor,
+                                                      signalSemaphore, allocation, waitSemaphores),
+                          &result)) {
             // Clear the signal semaphore
             fn.DestroySemaphore(GetVkDevice(), signalSemaphore, nullptr);
 
@@ -688,8 +693,7 @@ namespace dawn_native { namespace vulkan {
             return nullptr;
         }
 
-        return new Texture(this, descriptor, textureDescriptor, signalSemaphore, allocation,
-                           waitSemaphores);
+        return result;
     }
 
     ResultOrError<ResourceMemoryAllocation> Device::AllocateMemory(

src/dawn_native/vulkan/TextureVk.cpp

@@ -404,6 +404,20 @@ namespace dawn_native { namespace vulkan {
         return texture.release();
     }
 
+    // static
+    ResultOrError<Texture*> Texture::CreateFromExternal(Device* device,
+                                                        const ExternalImageDescriptor* descriptor,
+                                                        const TextureDescriptor* textureDescriptor,
+                                                        VkSemaphore signalSemaphore,
+                                                        VkDeviceMemory externalMemoryAllocation,
+                                                        std::vector<VkSemaphore> waitSemaphores) {
+        std::unique_ptr<Texture> texture =
+            std::make_unique<Texture>(device, textureDescriptor, TextureState::OwnedInternal);
+        DAWN_TRY(texture->InitializeFromExternal(
+            descriptor, signalSemaphore, externalMemoryAllocation, std::move((waitSemaphores))));
+        return texture.release();
+    }
+
     MaybeError Texture::InitializeAsInternalTexture() {
         Device* device = ToBackend(GetDevice());
 
@@ -469,18 +483,14 @@ namespace dawn_native { namespace vulkan {
         : TextureBase(device, descriptor, TextureState::OwnedExternal), mHandle(nativeImage) {
     }
 
-    // Internally managed, but imported from file descriptor
-    Texture::Texture(Device* device,
-                     const ExternalImageDescriptor* descriptor,
-                     const TextureDescriptor* textureDescriptor,
+    // Internally managed, but imported from external handle
+    MaybeError Texture::InitializeFromExternal(const ExternalImageDescriptor* descriptor,
                                                VkSemaphore signalSemaphore,
                                                VkDeviceMemory externalMemoryAllocation,
-                     std::vector<VkSemaphore> waitSemaphores)
-        : TextureBase(device, textureDescriptor, TextureState::OwnedInternal),
-          mExternalAllocation(externalMemoryAllocation),
-          mExternalState(ExternalState::PendingAcquire),
-          mSignalSemaphore(signalSemaphore),
-          mWaitRequirements(std::move(waitSemaphores)) {
+                                               std::vector<VkSemaphore> waitSemaphores) {
+        mExternalState = ExternalState::PendingAcquire;
+        Device* device = ToBackend(GetDevice());
+
         VkImageCreateInfo createInfo = {};
         createInfo.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO;
         createInfo.pNext = nullptr;
@@ -505,10 +515,9 @@ namespace dawn_native { namespace vulkan {
         // also required for the implementation of robust resource initialization.
         createInfo.usage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT;
 
-        if (device->fn.CreateImage(device->GetVkDevice(), &createInfo, nullptr, &mHandle) !=
-            VK_SUCCESS) {
-            ASSERT(false);
-        }
+        DAWN_TRY(CheckVkSuccess(
+            device->fn.CreateImage(device->GetVkDevice(), &createInfo, nullptr, &mHandle),
+            "CreateImage"));
 
         // Create the image memory and associate it with the container
         VkMemoryRequirements requirements;
@@ -516,15 +525,21 @@ namespace dawn_native { namespace vulkan {
 
         ASSERT(requirements.size <= descriptor->allocationSize);
 
-        if (device->fn.BindImageMemory(device->GetVkDevice(), mHandle, mExternalAllocation, 0) !=
-            VK_SUCCESS) {
-            ASSERT(false);
-        }
+        DAWN_TRY(CheckVkSuccess(
+            device->fn.BindImageMemory(device->GetVkDevice(), mHandle, externalMemoryAllocation, 0),
+            "BindImageMemory (external)"));
 
         // Don't clear imported texture if already cleared
         if (descriptor->isCleared) {
             SetIsSubresourceContentInitialized(true, 0, 1, 0, 1);
         }
+
+        // Success, acquire all the external objects.
+        mExternalAllocation = externalMemoryAllocation;
+        mSignalSemaphore = signalSemaphore;
+        mWaitRequirements = std::move(waitSemaphores);
+
+        return {};
     }
 
     MaybeError Texture::SignalAndDestroy(VkSemaphore* outSignalSemaphore) {

src/dawn_native/vulkan/TextureVk.h

@@ -35,14 +35,21 @@ namespace dawn_native { namespace vulkan {
 
     class Texture : public TextureBase {
       public:
+        // Used to create a regular texture from a descriptor.
         static ResultOrError<Texture*> Create(Device* device, const TextureDescriptor* descriptor);
-        Texture(Device* device, const TextureDescriptor* descriptor, VkImage nativeImage);
-        Texture(Device* device,
+
+        // Used to create a texture from Vulkan external memory objects.
+        // Ownership of semaphores and the memory allocation is taken only if the creation is
+        // a success.
+        static ResultOrError<Texture*> CreateFromExternal(
+            Device* device,
             const ExternalImageDescriptor* descriptor,
             const TextureDescriptor* textureDescriptor,
             VkSemaphore signalSemaphore,
             VkDeviceMemory externalMemoryAllocation,
             std::vector<VkSemaphore> waitSemaphores);
+
+        Texture(Device* device, const TextureDescriptor* descriptor, VkImage nativeImage);
         ~Texture();
 
         VkImage GetHandle() const;
@@ -64,6 +71,10 @@ namespace dawn_native { namespace vulkan {
       private:
         using TextureBase::TextureBase;
         MaybeError InitializeAsInternalTexture();
+        MaybeError InitializeFromExternal(const ExternalImageDescriptor* descriptor,
+                                          VkSemaphore signalSemaphore,
+                                          VkDeviceMemory externalMemoryAllocation,
+                                          std::vector<VkSemaphore> waitSemaphores);
 
         void DestroyImpl() override;
         MaybeError ClearTexture(CommandRecordingContext* recordingContext,