commit 65ee6497d620110546181c1dfbd0cc3064f9f7f9
author Austin Eng <enga@chromium.org> Fri May 22 00:23:39 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Fri May 22 00:23:39 2020 +0000
tree 01adff44d7b6a11e6e2681a520d4d47d27432358
parent bd0ad7921d3d6314898872029900fd964c2a6ed0

Fix VulkanImageWrappingUsageTests.ClearImageAcrossDevicesAliased

In Vulkan, importing memory by file descriptor takes ownership of
the file descriptor. It is necessary to dup it in
ClearImageAcrossDevicesAliased because the texture is imported
twice. This fixes these tests on SwiftShader.

Bug: dawn:417
Change-Id: I08b6464c4b8bd31f738037678e29fd6d066e7888
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22020
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/include/dawn_native/VulkanBackend.h

diff --git a/src/include/dawn_native/VulkanBackend.h b/src/include/dawn_native/VulkanBackend.h
index b144e4e..0965871 100644
--- a/src/include/dawn_native/VulkanBackend.h
+++ b/src/include/dawn_native/VulkanBackend.h
@@ -34,7 +34,10 @@
 
 // Can't use DAWN_PLATFORM_LINUX since header included in both dawn and chrome
 #ifdef __linux__
-        // Common properties of external images represented by FDs
+        // Common properties of external images represented by FDs. On successful import the file
+        // descriptor's ownership is transferred to the Dawn implementation and they shouldn't be
+        // used outside of Dawn again. TODO(enga): Also transfer ownership in the error case so the
+        // caller can assume the FD is always consumed.
         struct DAWN_NATIVE_EXPORT ExternalImageDescriptorFD : ExternalImageDescriptor {
           public:
             int memoryFD;  // A file descriptor from an export of the memory of the image

src/tests/white_box/VulkanImageWrappingTestsOpaqueFD.cpp

diff --git a/src/tests/white_box/VulkanImageWrappingTestsOpaqueFD.cpp b/src/tests/white_box/VulkanImageWrappingTestsOpaqueFD.cpp
index 8e3dc8a..3fc37cf 100644
--- a/src/tests/white_box/VulkanImageWrappingTestsOpaqueFD.cpp
+++ b/src/tests/white_box/VulkanImageWrappingTestsOpaqueFD.cpp
@@ -34,7 +34,6 @@
             void SetUp() override {
                 DawnTest::SetUp();
                 DAWN_SKIP_TEST_IF(UsesWire());
-                DAWN_SKIP_TEST_IF(IsSwiftshader());
 
                 deviceVk = reinterpret_cast<dawn_native::vulkan::Device*>(device.Get());
             }
@@ -202,7 +201,7 @@
       public:
         void SetUp() override {
             VulkanImageWrappingTestBase::SetUp();
-            if (UsesWire() || IsSwiftshader()) {
+            if (UsesWire()) {
                 return;
             }
 
@@ -220,7 +219,7 @@
         }
 
         void TearDown() override {
-            if (UsesWire() || IsSwiftshader()) {
+            if (UsesWire()) {
                 VulkanImageWrappingTestBase::TearDown();
                 return;
             }
@@ -354,7 +353,7 @@
       public:
         void SetUp() override {
             VulkanImageWrappingTestBase::SetUp();
-            if (UsesWire() || IsSwiftshader()) {
+            if (UsesWire()) {
                 return;
             }
 
@@ -382,7 +381,7 @@
         }
 
         void TearDown() override {
-            if (UsesWire() || IsSwiftshader()) {
+            if (UsesWire()) {
                 VulkanImageWrappingTestBase::TearDown();
                 return;
             }
@@ -488,13 +487,17 @@
     // alias the same memory
     TEST_P(VulkanImageWrappingUsageTests, ClearImageAcrossDevicesAliased) {
         DAWN_SKIP_TEST_IF(UsesWire());
+
+        // WrapVulkanImage consumes the file descriptor so we can't import defaultFd twice.
+        // Duplicate the file descriptor so we can import it twice.
+        int defaultFdCopy = dup(defaultFd);
+        ASSERT(defaultFdCopy != -1);
+
         // Import the image on |device
         wgpu::Texture wrappedTextureAlias =
-            WrapVulkanImage(device, &defaultDescriptor, defaultFd, defaultAllocationSize,
+            WrapVulkanImage(device, &defaultDescriptor, defaultFdCopy, defaultAllocationSize,
                             defaultMemoryTypeIndex, {});
 
-        int memoryFd = GetMemoryFd(deviceVk, defaultAllocation);
-
         // Import the image on |secondDevice|
         wgpu::Texture wrappedTexture =
             WrapVulkanImage(secondDevice, &defaultDescriptor, defaultFd, defaultAllocationSize,
@@ -507,7 +510,7 @@
                                                                           wrappedTexture.Get());
 
         // Import the image to |device|, making sure we wait on signalFd
-        memoryFd = GetMemoryFd(deviceVk, defaultAllocation);
+        int memoryFd = GetMemoryFd(deviceVk, defaultAllocation);
         wgpu::Texture nextWrappedTexture =
             WrapVulkanImage(device, &defaultDescriptor, memoryFd, defaultAllocationSize,
                             defaultMemoryTypeIndex, {signalFd});