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>
diff --git a/src/dawn_native/d3d12/D3D12Backend.cpp b/src/dawn_native/d3d12/D3D12Backend.cpp
index e6ce0f6..c0cbe48 100644
--- a/src/dawn_native/d3d12/D3D12Backend.cpp
+++ b/src/dawn_native/d3d12/D3D12Backend.cpp
@@ -108,9 +108,7 @@
Ref<TextureBase> texture = backendDevice->CreateExternalTexture(
&textureDescriptor, mD3D12Resource, std::move(d3d11on12Resource),
- ExternalMutexSerial(descriptor->acquireMutexKey),
- ExternalMutexSerial(descriptor->releaseMutexKey), descriptor->isSwapChainTexture,
- descriptor->isInitialized);
+ descriptor->isSwapChainTexture, descriptor->isInitialized);
return ToAPI(texture.Detach());
}
diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index 7dd2a53..54c2a71 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -534,16 +534,13 @@
const TextureDescriptor* descriptor,
ComPtr<ID3D12Resource> d3d12Texture,
Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
- ExternalMutexSerial acquireMutexKey,
- ExternalMutexSerial releaseMutexKey,
bool isSwapChainTexture,
bool isInitialized) {
Ref<Texture> dawnTexture;
- if (ConsumedError(
- Texture::CreateExternalImage(this, descriptor, std::move(d3d12Texture),
- std::move(d3d11on12Resource), acquireMutexKey,
- releaseMutexKey, isSwapChainTexture, isInitialized),
- &dawnTexture)) {
+ if (ConsumedError(Texture::CreateExternalImage(this, descriptor, std::move(d3d12Texture),
+ std::move(d3d11on12Resource),
+ isSwapChainTexture, isInitialized),
+ &dawnTexture)) {
return nullptr;
}
return {dawnTexture};
diff --git a/src/dawn_native/d3d12/DeviceD3D12.h b/src/dawn_native/d3d12/DeviceD3D12.h
index 26851d7..64c0e35 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.h
+++ b/src/dawn_native/d3d12/DeviceD3D12.h
@@ -131,8 +131,6 @@
Ref<TextureBase> CreateExternalTexture(const TextureDescriptor* descriptor,
ComPtr<ID3D12Resource> d3d12Texture,
Ref<D3D11on12ResourceCacheEntry> d3d11on12Resource,
- ExternalMutexSerial acquireMutexKey,
- ExternalMutexSerial releaseMutexKey,
bool isSwapChainTexture,
bool isInitialized);
diff --git a/src/dawn_native/d3d12/IntegerTypes.h b/src/dawn_native/d3d12/IntegerTypes.h
index df00f1e..b2f3309 100644
--- a/src/dawn_native/d3d12/IntegerTypes.h
+++ b/src/dawn_native/d3d12/IntegerTypes.h
@@ -26,9 +26,6 @@
// 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_
diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp
index 3c8178f..bf07cad 100644
--- a/src/dawn_native/d3d12/TextureD3D12.cpp
+++ b/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 @@
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,
- releaseMutexKey, isSwapChainTexture));
+ descriptor, std::move(d3d12Texture), std::move(d3d11on12Resource), 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 @@
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 @@
mSwapChainTexture = false;
if (mD3D11on12Resource != nullptr) {
- mD3D11on12Resource->GetDXGIKeyedMutex()->ReleaseSync(uint64_t(mReleaseMutexKey));
+ mD3D11on12Resource->GetDXGIKeyedMutex()->ReleaseSync(kDXGIKeyedMutexAcquireReleaseKey);
}
}
diff --git a/src/dawn_native/d3d12/TextureD3D12.h b/src/dawn_native/d3d12/TextureD3D12.h
index 569c355..fc08940 100644
--- a/src/dawn_native/d3d12/TextureD3D12.h
+++ b/src/dawn_native/d3d12/TextureD3D12.h
@@ -45,8 +45,6 @@
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 @@
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 @@
bool mSwapChainTexture = false;
D3D12_RESOURCE_FLAGS mD3D12ResourceFlags;
- ExternalMutexSerial mAcquireMutexKey = ExternalMutexSerial(0);
- ExternalMutexSerial mReleaseMutexKey = ExternalMutexSerial(0);
Ref<D3D11on12ResourceCacheEntry> mD3D11on12Resource;
};
diff --git a/src/include/dawn_native/D3D12Backend.h b/src/include/dawn_native/D3D12Backend.h
index 8973af1..d614b16 100644
--- a/src/include/dawn_native/D3D12Backend.h
+++ b/src/include/dawn_native/D3D12Backend.h
@@ -55,9 +55,14 @@
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;
diff --git a/src/tests/end2end/D3D12ResourceWrappingTests.cpp b/src/tests/end2end/D3D12ResourceWrappingTests.cpp
index 34f7a58..f12e98c 100644
--- a/src/tests/end2end/D3D12ResourceWrappingTests.cpp
+++ b/src/tests/end2end/D3D12ResourceWrappingTests.cpp
@@ -27,6 +27,8 @@
namespace {
+ using dawn_native::d3d12::kDXGIKeyedMutexAcquireReleaseKey;
+
class D3D12ResourceTestBase : public DawnTest {
protected:
std::vector<wgpu::FeatureName> GetRequiredFeatures() override {
@@ -126,8 +128,6 @@
}
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 @@
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 @@
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 @@
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 @@
*dxgiKeyedMutexOut = dxgiKeyedMutex.Detach();
}
- void ExpectPixelRGBA8EQ(UINT64 acquireKey,
- ID3D11Texture2D* d3d11Texture,
+ void ExpectPixelRGBA8EQ(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 @@
mD3d11DeviceContext->Unmap(spD3DTextureStaging.Get(), 0);
- hr = dxgiKeyedMutex->ReleaseSync(acquireKey + 1);
+ hr = dxgiKeyedMutex->ReleaseSync(kDXGIKeyedMutexAcquireReleaseKey);
ASSERT_EQ(hr, S_OK);
}
};
@@ -527,7 +524,7 @@
// 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 @@
// 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 @@
// 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 @@
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 @@
// 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 @@
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));
diff --git a/src/tests/end2end/VideoViewsTests_win.cpp b/src/tests/end2end/VideoViewsTests_win.cpp
index 9439609..18b63a0 100644
--- a/src/tests/end2end/VideoViewsTests_win.cpp
+++ b/src/tests/end2end/VideoViewsTests_win.cpp
@@ -144,10 +144,11 @@
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 @@
::CloseHandle(sharedHandle);
dawn_native::d3d12::ExternalImageAccessDescriptorDXGIKeyedMutex externalAccessDesc;
- externalAccessDesc.acquireMutexKey = 1;
- externalAccessDesc.releaseMutexKey = 2;
externalAccessDesc.isInitialized = true;
externalAccessDesc.usage = static_cast<WGPUTextureUsageFlags>(textureDesc.usage);
