Create VkImage before importing external memory

This CL is part of a chain of CLs that imports dma-bufs as VkImages
to support WebGPU on Chrome OS.

There are currently two steps for importing external memory into
Vulkan:

  1. DeviceVk::ImportExternalImage: calls into
     MemoryServiceOpaqueFD::ImportMemory which in turn calls into
     vkAllocateMemory and outputs a VkDeviceMemory handle to the
     imported memory.
  2. TextureVk::CreateFromExternal: creates the actual TextureVk
     object, creates the VkImage, and binds the VkDeviceMemory from
     ImportExternalImage to the VkImage.

For dma-buf support, however, we need to re-order these two steps
because importing dma-buf memory requires a handle to the VkImage
that will alias it [1].

This CL splits these two steps into three steps to ensure we create
the VkImage first so we can use it in vkAllocateMemory:

  1. TextureVk::CreateFromExternal: creates the TextureVk and
     VkImage (no longer concerns itself with vkBindImageMemory).
  2. DeviceVk::ImportExternalImage: now takes the VkImage as input
     but is otherwise unchanged.
  3. TextureVk::BindExternalMemory: calls vkBindImageMemory with
     handles to VkDeviceMemory and VkImage.

[1] https://www.khronos.org/registry/vulkan/specs/1.1-extensions/man/html/VkMemoryDedicatedAllocateInfo.html

BUG=chromium:996470

Change-Id: Id2d5951e9b573af79c44ce8c63be5210a279f946
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/13780
Commit-Queue: Brian Ho <hob@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp
index 677ae87..18bc3e8 100644
--- a/src/dawn_native/vulkan/DeviceVk.cpp
+++ b/src/dawn_native/vulkan/DeviceVk.cpp
@@ -595,6 +595,7 @@
 
     MaybeError Device::ImportExternalImage(const ExternalImageDescriptor* descriptor,
                                            ExternalMemoryHandle memoryHandle,
+                                           VkImage image,
                                            const std::vector<ExternalSemaphoreHandle>& waitHandles,
                                            VkSemaphore* outSignalSemaphore,
                                            VkDeviceMemory* outAllocation,
@@ -620,9 +621,11 @@
                         mExternalSemaphoreService->CreateExportableSemaphore());
 
         // Import the external image's memory
+        external_memory::MemoryImportParams importParams;
+        DAWN_TRY_ASSIGN(importParams,
+                        mExternalMemoryService->GetMemoryImportParams(descriptor, image));
         DAWN_TRY_ASSIGN(*outAllocation,
-                        mExternalMemoryService->ImportMemory(
-                            memoryHandle, descriptor->allocationSize, descriptor->memoryTypeIndex));
+                        mExternalMemoryService->ImportMemory(memoryHandle, importParams, image));
 
         // Import semaphores we have to wait on before using the texture
         for (const ExternalSemaphoreHandle& handle : waitHandles) {
@@ -671,11 +674,19 @@
         // 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)) ||
-            ConsumedError(Texture::CreateFromExternal(this, descriptor, textureDescriptor,
-                                                      signalSemaphore, allocation, waitSemaphores),
-                          &result)) {
+        // TODO(crbug.com/1026480): Consolidate this into a single CreateFromExternal call.
+        if (ConsumedError(Texture::CreateFromExternal(this, descriptor, textureDescriptor),
+                          &result) ||
+            ConsumedError(ImportExternalImage(descriptor, memoryHandle, result->GetHandle(),
+                                              waitHandles, &signalSemaphore, &allocation,
+                                              &waitSemaphores)) ||
+            ConsumedError(result->BindExternalMemory(descriptor, signalSemaphore, allocation,
+                                                     waitSemaphores))) {
+            // Delete the Texture if it was created
+            if (result != nullptr) {
+                delete result;
+            }
+
             // Clear the signal semaphore
             fn.DestroySemaphore(GetVkDevice(), signalSemaphore, nullptr);
 
diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h
index 0e4feee..74ae79d 100644
--- a/src/dawn_native/vulkan/DeviceVk.h
+++ b/src/dawn_native/vulkan/DeviceVk.h
@@ -171,6 +171,7 @@
 
         MaybeError ImportExternalImage(const ExternalImageDescriptor* descriptor,
                                        ExternalMemoryHandle memoryHandle,
+                                       VkImage image,
                                        const std::vector<ExternalSemaphoreHandle>& waitHandles,
                                        VkSemaphore* outSignalSemaphore,
                                        VkDeviceMemory* outAllocation,
diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp
index a4eb35b..211719d 100644
--- a/src/dawn_native/vulkan/TextureVk.cpp
+++ b/src/dawn_native/vulkan/TextureVk.cpp
@@ -406,16 +406,13 @@
     }
 
     // static
-    ResultOrError<Texture*> Texture::CreateFromExternal(Device* device,
-                                                        const ExternalImageDescriptor* descriptor,
-                                                        const TextureDescriptor* textureDescriptor,
-                                                        VkSemaphore signalSemaphore,
-                                                        VkDeviceMemory externalMemoryAllocation,
-                                                        std::vector<VkSemaphore> waitSemaphores) {
+    ResultOrError<Texture*> Texture::CreateFromExternal(
+        Device* device,
+        const ExternalImageDescriptor* descriptor,
+        const TextureDescriptor* textureDescriptor) {
         std::unique_ptr<Texture> texture =
             std::make_unique<Texture>(device, textureDescriptor, TextureState::OwnedInternal);
-        DAWN_TRY(texture->InitializeFromExternal(
-            descriptor, signalSemaphore, externalMemoryAllocation, std::move((waitSemaphores))));
+        DAWN_TRY(texture->InitializeFromExternal(descriptor));
         return texture.release();
     }
 
@@ -484,10 +481,7 @@
     }
 
     // Internally managed, but imported from external handle
-    MaybeError Texture::InitializeFromExternal(const ExternalImageDescriptor* descriptor,
-                                               VkSemaphore signalSemaphore,
-                                               VkDeviceMemory externalMemoryAllocation,
-                                               std::vector<VkSemaphore> waitSemaphores) {
+    MaybeError Texture::InitializeFromExternal(const ExternalImageDescriptor* descriptor) {
         mExternalState = ExternalState::PendingAcquire;
         Device* device = ToBackend(GetDevice());
 
@@ -519,7 +513,14 @@
             device->fn.CreateImage(device->GetVkDevice(), &createInfo, nullptr, &mHandle),
             "CreateImage"));
 
-        // Create the image memory and associate it with the container
+        return {};
+    }
+
+    MaybeError Texture::BindExternalMemory(const ExternalImageDescriptor* descriptor,
+                                           VkSemaphore signalSemaphore,
+                                           VkDeviceMemory externalMemoryAllocation,
+                                           std::vector<VkSemaphore> waitSemaphores) {
+        Device* device = ToBackend(GetDevice());
         VkMemoryRequirements requirements;
         device->fn.GetImageMemoryRequirements(device->GetVkDevice(), mHandle, &requirements);
 
@@ -538,7 +539,6 @@
         mExternalAllocation = externalMemoryAllocation;
         mSignalSemaphore = signalSemaphore;
         mWaitRequirements = std::move(waitSemaphores);
-
         return {};
     }
 
diff --git a/src/dawn_native/vulkan/TextureVk.h b/src/dawn_native/vulkan/TextureVk.h
index ecb2cda..5174292 100644
--- a/src/dawn_native/vulkan/TextureVk.h
+++ b/src/dawn_native/vulkan/TextureVk.h
@@ -39,16 +39,13 @@
         // Used to create a regular texture from a descriptor.
         static ResultOrError<Texture*> Create(Device* device, const TextureDescriptor* descriptor);
 
-        // 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.
+        // Creates a texture and initializes it with a VkImage that references an external memory
+        // object. Before the texture can be used, the VkDeviceMemory associated with the external
+        // image must be bound via Texture::BindExternalMemory.
         static ResultOrError<Texture*> CreateFromExternal(
             Device* device,
             const ExternalImageDescriptor* descriptor,
-            const TextureDescriptor* textureDescriptor,
-            VkSemaphore signalSemaphore,
-            VkDeviceMemory externalMemoryAllocation,
-            std::vector<VkSemaphore> waitSemaphores);
+            const TextureDescriptor* textureDescriptor);
 
         Texture(Device* device, const TextureDescriptor* descriptor, VkImage nativeImage);
         ~Texture();
@@ -68,14 +65,18 @@
                                                  uint32_t layerCount);
 
         MaybeError SignalAndDestroy(VkSemaphore* outSignalSemaphore);
+        // Binds externally allocated memory to the VkImage and on success, takes ownership of
+        // semaphores.
+        MaybeError BindExternalMemory(const ExternalImageDescriptor* descriptor,
+                                      VkSemaphore signalSemaphore,
+                                      VkDeviceMemory externalMemoryAllocation,
+                                      std::vector<VkSemaphore> waitSemaphores);
 
       private:
         using TextureBase::TextureBase;
         MaybeError InitializeAsInternalTexture();
-        MaybeError InitializeFromExternal(const ExternalImageDescriptor* descriptor,
-                                          VkSemaphore signalSemaphore,
-                                          VkDeviceMemory externalMemoryAllocation,
-                                          std::vector<VkSemaphore> waitSemaphores);
+
+        MaybeError InitializeFromExternal(const ExternalImageDescriptor* descriptor);
 
         void DestroyImpl() override;
         MaybeError ClearTexture(CommandRecordingContext* recordingContext,
diff --git a/src/dawn_native/vulkan/external_memory/MemoryService.h b/src/dawn_native/vulkan/external_memory/MemoryService.h
index e49d3ff..bcd3ea4 100644
--- a/src/dawn_native/vulkan/external_memory/MemoryService.h
+++ b/src/dawn_native/vulkan/external_memory/MemoryService.h
@@ -17,6 +17,7 @@
 
 #include "common/vulkan_platform.h"
 #include "dawn_native/Error.h"
+#include "dawn_native/VulkanBackend.h"
 #include "dawn_native/vulkan/ExternalHandle.h"
 
 namespace dawn_native { namespace vulkan {
@@ -25,6 +26,11 @@
 
 namespace dawn_native { namespace vulkan { namespace external_memory {
 
+    struct MemoryImportParams {
+        VkDeviceSize allocationSize;
+        uint32_t memoryTypeIndex;
+    };
+
     class Service {
       public:
         explicit Service(Device* device);
@@ -37,10 +43,15 @@
                        VkImageUsageFlags usage,
                        VkImageCreateFlags flags);
 
+        // Returns the parameters required for importing memory
+        ResultOrError<MemoryImportParams> GetMemoryImportParams(
+            const ExternalImageDescriptor* descriptor,
+            VkImage image);
+
         // Given an external handle pointing to memory, import it into a VkDeviceMemory
         ResultOrError<VkDeviceMemory> ImportMemory(ExternalMemoryHandle handle,
-                                                   VkDeviceSize allocationSize,
-                                                   uint32_t memoryTypeIndex);
+                                                   const MemoryImportParams& importParams,
+                                                   VkImage image);
 
       private:
         Device* mDevice = nullptr;
diff --git a/src/dawn_native/vulkan/external_memory/MemoryServiceNull.cpp b/src/dawn_native/vulkan/external_memory/MemoryServiceNull.cpp
index 7804ad3..c7417d7 100644
--- a/src/dawn_native/vulkan/external_memory/MemoryServiceNull.cpp
+++ b/src/dawn_native/vulkan/external_memory/MemoryServiceNull.cpp
@@ -32,10 +32,16 @@
         return false;
     }
 
+    ResultOrError<MemoryImportParams> Service::GetMemoryImportParams(
+        const ExternalImageDescriptor* descriptor,
+        VkImage image) {
+        return DAWN_UNIMPLEMENTED_ERROR("Using null memory service to interop inside Vulkan");
+    }
+
     ResultOrError<VkDeviceMemory> Service::ImportMemory(ExternalMemoryHandle handle,
-                                                        VkDeviceSize allocationSize,
-                                                        uint32_t memoryTypeIndex) {
-        return DAWN_UNIMPLEMENTED_ERROR("Using null semaphore service to interop inside Vulkan");
+                                                        const MemoryImportParams& importParams,
+                                                        VkImage image) {
+        return DAWN_UNIMPLEMENTED_ERROR("Using null memory service to interop inside Vulkan");
     }
 
 }}}  // namespace dawn_native::vulkan::external_memory
diff --git a/src/dawn_native/vulkan/external_memory/MemoryServiceOpaqueFD.cpp b/src/dawn_native/vulkan/external_memory/MemoryServiceOpaqueFD.cpp
index d6e0e5a..64cd3a5 100644
--- a/src/dawn_native/vulkan/external_memory/MemoryServiceOpaqueFD.cpp
+++ b/src/dawn_native/vulkan/external_memory/MemoryServiceOpaqueFD.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "common/Assert.h"
 #include "dawn_native/vulkan/AdapterVk.h"
 #include "dawn_native/vulkan/BackendVk.h"
 #include "dawn_native/vulkan/DeviceVk.h"
@@ -79,9 +80,16 @@
                !(memoryFlags & VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT_KHR);
     }
 
+    ResultOrError<MemoryImportParams> Service::GetMemoryImportParams(
+        const ExternalImageDescriptor* descriptor,
+        VkImage image) {
+        MemoryImportParams params = {descriptor->allocationSize, descriptor->memoryTypeIndex};
+        return params;
+    }
+
     ResultOrError<VkDeviceMemory> Service::ImportMemory(ExternalMemoryHandle handle,
-                                                        VkDeviceSize allocationSize,
-                                                        uint32_t memoryTypeIndex) {
+                                                        const MemoryImportParams& importParams,
+                                                        VkImage image) {
         if (handle < 0) {
             return DAWN_VALIDATION_ERROR("Trying to import memory with invalid handle");
         }
@@ -95,8 +103,8 @@
         VkMemoryAllocateInfo allocateInfo;
         allocateInfo.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO;
         allocateInfo.pNext = &importMemoryFdInfo;
-        allocateInfo.allocationSize = allocationSize;
-        allocateInfo.memoryTypeIndex = memoryTypeIndex;
+        allocateInfo.allocationSize = importParams.allocationSize;
+        allocateInfo.memoryTypeIndex = importParams.memoryTypeIndex;
 
         VkDeviceMemory allocatedMemory = VK_NULL_HANDLE;
         DAWN_TRY(CheckVkSuccess(mDevice->fn.AllocateMemory(mDevice->GetVkDevice(), &allocateInfo,
diff --git a/src/dawn_native/vulkan/external_memory/MemoryServiceZirconHandle.cpp b/src/dawn_native/vulkan/external_memory/MemoryServiceZirconHandle.cpp
index 1788f70..d461d44 100644
--- a/src/dawn_native/vulkan/external_memory/MemoryServiceZirconHandle.cpp
+++ b/src/dawn_native/vulkan/external_memory/MemoryServiceZirconHandle.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "common/Assert.h"
 #include "dawn_native/vulkan/AdapterVk.h"
 #include "dawn_native/vulkan/BackendVk.h"
 #include "dawn_native/vulkan/DeviceVk.h"
@@ -79,9 +80,16 @@
                !(memoryFlags & VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT_KHR);
     }
 
+    ResultOrError<MemoryImportParams> Service::GetMemoryImportParams(
+        const ExternalImageDescriptor* descriptor,
+        VkImage image) {
+        MemoryImportParams params = {descriptor->allocationSize, descriptor->memoryTypeIndex};
+        return params;
+    }
+
     ResultOrError<VkDeviceMemory> Service::ImportMemory(ExternalMemoryHandle handle,
-                                                        VkDeviceSize allocationSize,
-                                                        uint32_t memoryTypeIndex) {
+                                                        const MemoryImportParams& importParams,
+                                                        VkImage image) {
         if (handle == ZX_HANDLE_INVALID) {
             return DAWN_VALIDATION_ERROR("Trying to import memory with invalid handle");
         }
@@ -97,8 +105,8 @@
         VkMemoryAllocateInfo allocateInfo;
         allocateInfo.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO;
         allocateInfo.pNext = &importMemoryHandleInfo;
-        allocateInfo.allocationSize = allocationSize;
-        allocateInfo.memoryTypeIndex = memoryTypeIndex;
+        allocateInfo.allocationSize = importParams.allocationSize;
+        allocateInfo.memoryTypeIndex = importParams.memoryTypeIndex;
 
         VkDeviceMemory allocatedMemory = VK_NULL_HANDLE;
         DAWN_TRY(CheckVkSuccess(mDevice->fn.AllocateMemory(mDevice->GetVkDevice(), &allocateInfo,