d3d12: Use fixed key for keyed mutex acquire and release

Chromium has switched to using a fixed key (0) for quite some time.
Using a fixed key enables future work needed for concurrent access of
external textures both inside Dawn (e.g. importing a video frame twice)
or across Dawn and GL. We want to try scoping the Acquire/Release calls
to command list submission, but with a non-fixed key concurrent access
for the same resource becomes ambiguous.

Bug: chromium:1241533
Change-Id: Ia8ff473b8c9c731c411a3fd59d69213f2d903e61
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/75642
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

src/dawn_native/d3d12/D3D12Backend.cpp

@@ -108,9 +108,7 @@ namespace dawn_native::d3d12 {
 
         Ref<TextureBase> texture = backendDevice->CreateExternalTexture(
             &textureDescriptor, mD3D12Resource, std::move(d3d11on12Resource),
-            ExternalMutexSerial(descriptor->acquireMutexKey),
+            descriptor->isSwapChainTexture, descriptor->isInitialized);
-            ExternalMutexSerial(descriptor->releaseMutexKey), descriptor->isSwapChainTexture,
-            descriptor->isInitialized);
 
         return ToAPI(texture.Detach());
     }

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -534,16 +534,13 @@ namespace dawn_native::d3d12 {
         const TextureDescriptor* descriptor,
         ComPtr<ID3D12Resource> d3d12Texture,
         Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
-        ExternalMutexSerial acquireMutexKey,
-        ExternalMutexSerial releaseMutexKey,
         bool isSwapChainTexture,
         bool isInitialized) {
         Ref<Texture> dawnTexture;
-        if (ConsumedError(
+        if (ConsumedError(Texture::CreateExternalImage(this, descriptor, std::move(d3d12Texture),
-                Texture::CreateExternalImage(this, descriptor, std::move(d3d12Texture),
+                                                       std::move(d3d11on12Resource),
-                                             std::move(d3d11on12Resource), acquireMutexKey,
+                                                       isSwapChainTexture, isInitialized),
-                                             releaseMutexKey, isSwapChainTexture, isInitialized),
+                          &dawnTexture)) {
-                &dawnTexture)) {
             return nullptr;
         }
         return {dawnTexture};

src/dawn_native/d3d12/DeviceD3D12.h

@@ -131,8 +131,6 @@ namespace dawn_native::d3d12 {
         Ref<TextureBase> CreateExternalTexture(const TextureDescriptor* descriptor,
                                                ComPtr<ID3D12Resource> d3d12Texture,
                                                Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
-                                               ExternalMutexSerial acquireMutexKey,
-                                               ExternalMutexSerial releaseMutexKey,
                                                bool isSwapChainTexture,
                                                bool isInitialized);
 

src/dawn_native/d3d12/IntegerTypes.h

@@ -26,9 +26,6 @@ namespace dawn_native::d3d12 {
     // BindGroup allocations.
     using HeapVersionID = TypedInteger<struct HeapVersionIDT, uint64_t>;
 
-    // The monotonically increasing serial for external D3D12 mutexes imported in Dawn.
-    using ExternalMutexSerial = TypedInteger<struct ExternalMutexSerialT, uint64_t>;
-
 }  // namespace dawn_native::d3d12
 
 #endif  // DAWNNATIVE_D3D12_INTEGERTYPES_H_

src/dawn_native/d3d12/TextureD3D12.cpp

@@ -34,6 +34,7 @@
 namespace dawn_native::d3d12 {
 
     namespace {
+
         D3D12_RESOURCE_STATES D3D12TextureUsage(wgpu::TextureUsage usage, const Format& format) {
             D3D12_RESOURCE_STATES resourceState = D3D12_RESOURCE_STATE_COMMON;
 
@@ -517,15 +518,12 @@ namespace dawn_native::d3d12 {
         const TextureDescriptor* descriptor,
         ComPtr<ID3D12Resource> d3d12Texture,
         Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
-        ExternalMutexSerial acquireMutexKey,
-        ExternalMutexSerial releaseMutexKey,
         bool isSwapChainTexture,
         bool isInitialized) {
         Ref<Texture> dawnTexture =
             AcquireRef(new Texture(device, descriptor, TextureState::OwnedExternal));
         DAWN_TRY(dawnTexture->InitializeAsExternalTexture(
-            descriptor, std::move(d3d12Texture), std::move(d3d11on12Resource), acquireMutexKey,
+            descriptor, std::move(d3d12Texture), std::move(d3d11on12Resource), isSwapChainTexture));
-            releaseMutexKey, isSwapChainTexture));
 
         // Importing a multi-planar format must be initialized. This is required because
         // a shared multi-planar format cannot be initialized by Dawn.
@@ -553,15 +551,11 @@ namespace dawn_native::d3d12 {
         const TextureDescriptor* descriptor,
         ComPtr<ID3D12Resource> d3d12Texture,
         Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
-        ExternalMutexSerial acquireMutexKey,
-        ExternalMutexSerial releaseMutexKey,
         bool isSwapChainTexture) {
         DAWN_TRY(CheckHRESULT(d3d11on12Resource->GetDXGIKeyedMutex()->AcquireSync(
-                                  uint64_t(acquireMutexKey), INFINITE),
+                                  kDXGIKeyedMutexAcquireReleaseKey, INFINITE),
                               "D3D12 acquiring shared mutex"));
 
-        mAcquireMutexKey = acquireMutexKey;
-        mReleaseMutexKey = releaseMutexKey;
         mD3D11on12Resource = std::move(d3d11on12Resource);
         mSwapChainTexture = isSwapChainTexture;
 
@@ -680,7 +674,7 @@ namespace dawn_native::d3d12 {
         mSwapChainTexture = false;
 
         if (mD3D11on12Resource != nullptr) {
-            mD3D11on12Resource->GetDXGIKeyedMutex()->ReleaseSync(uint64_t(mReleaseMutexKey));
+            mD3D11on12Resource->GetDXGIKeyedMutex()->ReleaseSync(kDXGIKeyedMutexAcquireReleaseKey);
         }
     }
 

src/dawn_native/d3d12/TextureD3D12.h

@@ -45,8 +45,6 @@ namespace dawn_native::d3d12 {
             const TextureDescriptor* descriptor,
             ComPtr<ID3D12Resource> d3d12Texture,
             Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
-            ExternalMutexSerial acquireMutexKey,
-            ExternalMutexSerial releaseMutexKey,
             bool isSwapChainTexture,
             bool isInitialized);
         static ResultOrError<Ref<Texture>> Create(Device* device,
@@ -96,8 +94,6 @@ namespace dawn_native::d3d12 {
         MaybeError InitializeAsExternalTexture(const TextureDescriptor* descriptor,
                                                ComPtr<ID3D12Resource> d3d12Texture,
                                                Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
-                                               ExternalMutexSerial acquireMutexKey,
-                                               ExternalMutexSerial releaseMutexKey,
                                                bool isSwapChainTexture);
         MaybeError InitializeAsSwapChainTexture(ComPtr<ID3D12Resource> d3d12Texture);
 
@@ -136,8 +132,6 @@ namespace dawn_native::d3d12 {
         bool mSwapChainTexture = false;
         D3D12_RESOURCE_FLAGS mD3D12ResourceFlags;
 
-        ExternalMutexSerial mAcquireMutexKey = ExternalMutexSerial(0);
-        ExternalMutexSerial mReleaseMutexKey = ExternalMutexSerial(0);
         Ref<D3D11on12ResourceCacheEntry> mD3D11on12Resource;
     };
 

src/include/dawn_native/D3D12Backend.h

@@ -55,9 +55,14 @@ namespace dawn_native::d3d12 {
         HANDLE sharedHandle;
     };
 
+    // Keyed mutex acquire/release uses a fixed key of 0 to match Chromium behavior.
+    constexpr UINT64 kDXGIKeyedMutexAcquireReleaseKey = 0;
+
     struct DAWN_NATIVE_EXPORT ExternalImageAccessDescriptorDXGIKeyedMutex
         : ExternalImageAccessDescriptor {
       public:
+        // TODO(chromium:1241533): Remove deprecated keyed mutex params after removing associated
+        // code from Chromium - we use a fixed key of 0 for acquire and release everywhere now.
         uint64_t acquireMutexKey;
         uint64_t releaseMutexKey;
         bool isSwapChainTexture = false;

src/tests/end2end/D3D12ResourceWrappingTests.cpp

@@ -27,6 +27,8 @@ using Microsoft::WRL::ComPtr;
 
 namespace {
 
+    using dawn_native::d3d12::kDXGIKeyedMutexAcquireReleaseKey;
+
     class D3D12ResourceTestBase : public DawnTest {
       protected:
         std::vector<wgpu::FeatureName> GetRequiredFeatures() override {
@@ -126,8 +128,6 @@ namespace {
             }
 
             dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
-            externalAccessDesc.acquireMutexKey = 0;
-            externalAccessDesc.releaseMutexKey = 1;
             externalAccessDesc.usage = static_cast<WGPUTextureUsageFlags>(dawnDesc->usage);
 
             *dawnTexture = wgpu::Texture::Acquire(
@@ -366,7 +366,7 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase {
         hr = mD3d11Device->CreateRenderTargetView(d3d11Texture.Get(), nullptr, &d3d11RTV);
         ASSERT_EQ(hr, S_OK);
 
-        hr = dxgiKeyedMutex->AcquireSync(0, INFINITE);
+        hr = dxgiKeyedMutex->AcquireSync(kDXGIKeyedMutexAcquireReleaseKey, INFINITE);
         ASSERT_EQ(hr, S_OK);
 
         const float colorRGBA[] = {
@@ -374,7 +374,7 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase {
             static_cast<float>(clearColor.b), static_cast<float>(clearColor.a)};
         mD3d11DeviceContext->ClearRenderTargetView(d3d11RTV.Get(), colorRGBA);
 
-        hr = dxgiKeyedMutex->ReleaseSync(1);
+        hr = dxgiKeyedMutex->ReleaseSync(kDXGIKeyedMutexAcquireReleaseKey);
         ASSERT_EQ(hr, S_OK);
 
         dawn_native::d3d12::ExternalImageDescriptorDXGISharedHandle externalImageDesc = {};
@@ -386,8 +386,6 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase {
             dawn_native::d3d12::ExternalImageDXGI::Create(device.Get(), &externalImageDesc);
 
         dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
-        externalAccessDesc.acquireMutexKey = 1;
-        externalAccessDesc.releaseMutexKey = 2;
         externalAccessDesc.isInitialized = isInitialized;
         externalAccessDesc.usage = static_cast<WGPUTextureUsageFlags>(dawnDescriptor->usage);
 
@@ -397,11 +395,10 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase {
         *dxgiKeyedMutexOut = dxgiKeyedMutex.Detach();
     }
 
-    void ExpectPixelRGBA8EQ(UINT64 acquireKey,
+    void ExpectPixelRGBA8EQ(ID3D11Texture2D* d3d11Texture,
-                            ID3D11Texture2D* d3d11Texture,
                             IDXGIKeyedMutex* dxgiKeyedMutex,
                             const wgpu::Color& color) {
-        HRESULT hr = dxgiKeyedMutex->AcquireSync(acquireKey, INFINITE);
+        HRESULT hr = dxgiKeyedMutex->AcquireSync(kDXGIKeyedMutexAcquireReleaseKey, INFINITE);
         ASSERT_EQ(hr, S_OK);
 
         D3D11_TEXTURE2D_DESC texture2DDesc;
@@ -451,7 +448,7 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase {
 
         mD3d11DeviceContext->Unmap(spD3DTextureStaging.Get(), 0);
 
-        hr = dxgiKeyedMutex->ReleaseSync(acquireKey + 1);
+        hr = dxgiKeyedMutex->ReleaseSync(kDXGIKeyedMutexAcquireReleaseKey);
         ASSERT_EQ(hr, S_OK);
     }
 };
@@ -527,7 +524,7 @@ TEST_P(D3D12SharedHandleUsageTests, ClearInD3D12ReadbackInD3D11) {
     // Now that Dawn (via D3D12) has finished writing to the texture, we should be
     // able to read it back by copying it to a staging texture and verifying the
     // color matches the D3D12 clear color.
-    ExpectPixelRGBA8EQ(2, d3d11Texture.Get(), dxgiKeyedMutex.Get(), d3d12ClearColor);
+    ExpectPixelRGBA8EQ(d3d11Texture.Get(), dxgiKeyedMutex.Get(), d3d12ClearColor);
 }
 
 // 1. Create and clear a D3D11 texture
@@ -560,7 +557,7 @@ TEST_P(D3D12SharedHandleUsageTests, ClearTwiceInD3D12ReadbackInD3D11) {
     // Now that Dawn (via D3D12) has finished writing to the texture, we should be
     // able to read it back by copying it to a staging texture and verifying the
     // color matches the last D3D12 clear color.
-    ExpectPixelRGBA8EQ(2, d3d11Texture.Get(), dxgiKeyedMutex.Get(), d3d12ClearColor2);
+    ExpectPixelRGBA8EQ(d3d11Texture.Get(), dxgiKeyedMutex.Get(), d3d12ClearColor2);
 }
 
 // 1. Create and clear a D3D11 texture with clearColor
@@ -608,8 +605,6 @@ TEST_P(D3D12SharedHandleUsageTests, ReuseExternalImage) {
 
     // Create another Dawn texture then clear it with another color.
     dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
-    externalAccessDesc.acquireMutexKey = 1;
-    externalAccessDesc.releaseMutexKey = 2;
     externalAccessDesc.isInitialized = true;
     externalAccessDesc.usage = static_cast<WGPUTextureUsageFlags>(baseDawnDescriptor.usage);
 
@@ -634,8 +629,6 @@ TEST_P(D3D12SharedHandleUsageTests, ExternalImageUsage) {
     DAWN_TEST_UNSUPPORTED_IF(UsesWire());
 
     dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
-    externalAccessDesc.acquireMutexKey = 1;
-    externalAccessDesc.releaseMutexKey = 2;
     externalAccessDesc.isInitialized = true;
 
     wgpu::Texture texture;
@@ -678,8 +671,6 @@ TEST_P(D3D12SharedHandleUsageTests, ReuseExternalImageWithMultipleDevices) {
 
     // Create the Dawn texture then clear it to blue using the second device.
     dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
-    externalAccessDesc.acquireMutexKey = 1;
-    externalAccessDesc.releaseMutexKey = 2;
     externalAccessDesc.usage = static_cast<WGPUTextureUsageFlags>(baseDawnDescriptor.usage);
 
     wgpu::Device otherDevice = wgpu::Device::Acquire(GetAdapter().CreateDevice());
@@ -694,7 +685,6 @@ TEST_P(D3D12SharedHandleUsageTests, ReuseExternalImageWithMultipleDevices) {
     otherTexture.Destroy();
 
     // Re-create the Dawn texture using the first (default) device.
-    externalAccessDesc.acquireMutexKey = 2;
     externalAccessDesc.isInitialized = true;
     texture =
         wgpu::Texture::Acquire(externalImage->ProduceTexture(device.Get(), &externalAccessDesc));

src/tests/end2end/VideoViewsTests_win.cpp

@@ -144,10 +144,11 @@ class VideoViewsTestBackendWin : public VideoViewsTestBackend {
         hr = d3d11Texture.As(&dxgiKeyedMutex);
         ASSERT(hr == S_OK);
 
-        hr = dxgiKeyedMutex->AcquireSync(0, INFINITE);
+        using dawn_native::d3d12::kDXGIKeyedMutexAcquireReleaseKey;
+        hr = dxgiKeyedMutex->AcquireSync(kDXGIKeyedMutexAcquireReleaseKey, INFINITE);
         ASSERT(hr == S_OK);
 
-        hr = dxgiKeyedMutex->ReleaseSync(1);
+        hr = dxgiKeyedMutex->ReleaseSync(kDXGIKeyedMutexAcquireReleaseKey);
         ASSERT(hr == S_OK);
 
         // Open the DX11 texture in Dawn from the shared handle and return it as a WebGPU
@@ -164,8 +165,6 @@ class VideoViewsTestBackendWin : public VideoViewsTestBackend {
         ::CloseHandle(sharedHandle);
 
         dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
-        externalAccessDesc.acquireMutexKey = 1;
-        externalAccessDesc.releaseMutexKey = 2;
         externalAccessDesc.isInitialized = true;
         externalAccessDesc.usage = static_cast<WGPUTextureUsageFlags>(textureDesc.usage);