From 7d20b4450182b8e25dfd315019623d7a746cc290 Mon Sep 17 00:00:00 2001 From: Natasha Lee Date: Fri, 6 Mar 2020 19:05:15 +0000 Subject: [PATCH] Respect external clear status for Textures Use ExternalImageDescriptor->isCleared to set the clear status of subresources so it can be correctly lazy cleared when used. Also remove old Wrap path that uses regular texture descriptors since we have moved to use ExternalImageDescriptor. Bug: chromium:1036080 Change-Id: Icb605dbf3cf3f0dc8a30287e8b9b8d9134805112 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16320 Commit-Queue: Natasha Lee Reviewed-by: Corentin Wallez --- src/dawn_native/d3d12/D3D12Backend.cpp | 11 ------- src/dawn_native/d3d12/TextureD3D12.cpp | 7 +++-- src/dawn_native/metal/MetalBackend.mm | 12 -------- src/dawn_native/metal/TextureMTL.mm | 3 +- src/include/dawn_native/D3D12Backend.h | 5 ---- src/include/dawn_native/MetalBackend.h | 5 ---- .../end2end/D3D12ResourceWrappingTests.cpp | 22 +++++++++++++- src/tests/end2end/IOSurfaceWrappingTests.cpp | 29 ++++++++++++++++++- 8 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/dawn_native/d3d12/D3D12Backend.cpp b/src/dawn_native/d3d12/D3D12Backend.cpp index 4e57b541ef..8adea50651 100644 --- a/src/dawn_native/d3d12/D3D12Backend.cpp +++ b/src/dawn_native/d3d12/D3D12Backend.cpp @@ -58,15 +58,4 @@ namespace dawn_native { namespace d3d12 { return reinterpret_cast(texture); } - WGPUTexture WrapSharedHandle(WGPUDevice device, - const WGPUTextureDescriptor* descriptor, - HANDLE sharedHandle, - uint64_t acquireMutexKey) { - Device* backendDevice = reinterpret_cast(device); - ExternalImageDescriptorDXGISharedHandle externalDescriptor = {}; - externalDescriptor.cTextureDescriptor = descriptor; - TextureBase* texture = - backendDevice->WrapSharedHandle(&externalDescriptor, sharedHandle, acquireMutexKey); - return reinterpret_cast(texture); - } }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index 01652324d1..b07c3edf56 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -290,6 +290,10 @@ namespace dawn_native { namespace d3d12 { AcquireRef(new Texture(device, textureDescriptor, TextureState::OwnedExternal)); DAWN_TRY(dawnTexture->InitializeAsExternalTexture(textureDescriptor, sharedHandle, acquireMutexKey)); + + dawnTexture->SetIsSubresourceContentInitialized(descriptor->isCleared, 0, + textureDescriptor->mipLevelCount, 0, + textureDescriptor->arrayLayerCount); return dawnTexture.Detach(); } @@ -321,9 +325,6 @@ namespace dawn_native { namespace d3d12 { info.mMethod = AllocationMethod::kDirect; mResourceAllocation = {info, 0, std::move(d3d12Resource)}; - SetIsSubresourceContentInitialized(true, 0, descriptor->mipLevelCount, 0, - descriptor->arrayLayerCount); - return {}; } diff --git a/src/dawn_native/metal/MetalBackend.mm b/src/dawn_native/metal/MetalBackend.mm index ac65399fd9..24c44810e9 100644 --- a/src/dawn_native/metal/MetalBackend.mm +++ b/src/dawn_native/metal/MetalBackend.mm @@ -39,18 +39,6 @@ namespace dawn_native { namespace metal { return reinterpret_cast(texture); } - WGPUTexture WrapIOSurface(WGPUDevice cDevice, - const WGPUTextureDescriptor* cDescriptor, - IOSurfaceRef ioSurface, - uint32_t plane) { - Device* device = reinterpret_cast(cDevice); - ExternalImageDescriptorIOSurface descriptor = {}; - descriptor.cTextureDescriptor = cDescriptor; - TextureBase* texture = - device->CreateTextureWrappingIOSurface(&descriptor, ioSurface, plane); - return reinterpret_cast(texture); - } - void WaitForCommandsToBeScheduled(WGPUDevice cDevice) { Device* device = reinterpret_cast(cDevice); device->WaitForCommandsToBeScheduled(); diff --git a/src/dawn_native/metal/TextureMTL.mm b/src/dawn_native/metal/TextureMTL.mm index fbaf1fcb74..fd18dca30a 100644 --- a/src/dawn_native/metal/TextureMTL.mm +++ b/src/dawn_native/metal/TextureMTL.mm @@ -349,8 +349,7 @@ namespace dawn_native { namespace metal { plane:plane]; [mtlDesc release]; - // TODO(enga): Set as uninitialized if IOSurface isn't initialized. - SetIsSubresourceContentInitialized(true, 0, 1, 0, 1); + SetIsSubresourceContentInitialized(descriptor->isCleared, 0, 1, 0, 1); } Texture::~Texture() { diff --git a/src/include/dawn_native/D3D12Backend.h b/src/include/dawn_native/D3D12Backend.h index 1506e91fcf..9c20eadd5c 100644 --- a/src/include/dawn_native/D3D12Backend.h +++ b/src/include/dawn_native/D3D12Backend.h @@ -42,11 +42,6 @@ namespace dawn_native { namespace d3d12 { DAWN_NATIVE_EXPORT WGPUTexture WrapSharedHandle(WGPUDevice device, const ExternalImageDescriptorDXGISharedHandle* descriptor); - // Note: SharedHandle must be a handle to a texture object. - DAWN_NATIVE_EXPORT WGPUTexture WrapSharedHandle(WGPUDevice device, - const WGPUTextureDescriptor* descriptor, - HANDLE sharedHandle, - uint64_t acquireMutexKey); }} // namespace dawn_native::d3d12 #endif // DAWNNATIVE_D3D12BACKEND_H_ diff --git a/src/include/dawn_native/MetalBackend.h b/src/include/dawn_native/MetalBackend.h index 7ed458e983..90884ee7f2 100644 --- a/src/include/dawn_native/MetalBackend.h +++ b/src/include/dawn_native/MetalBackend.h @@ -44,11 +44,6 @@ namespace dawn_native { namespace metal { DAWN_NATIVE_EXPORT WGPUTexture WrapIOSurface(WGPUDevice device, const ExternalImageDescriptorIOSurface* descriptor); - DAWN_NATIVE_EXPORT WGPUTexture WrapIOSurface(WGPUDevice device, - const WGPUTextureDescriptor* descriptor, - IOSurfaceRef ioSurface, - uint32_t plane); - // When making Metal interop with other APIs, we need to be careful that QueueSubmit doesn't // mean that the operations will be visible to other APIs/Metal devices right away. macOS // does have a global queue of graphics operations, but the command buffers are inserted there diff --git a/src/tests/end2end/D3D12ResourceWrappingTests.cpp b/src/tests/end2end/D3D12ResourceWrappingTests.cpp index 4aced27815..745edb7de8 100644 --- a/src/tests/end2end/D3D12ResourceWrappingTests.cpp +++ b/src/tests/end2end/D3D12ResourceWrappingTests.cpp @@ -301,7 +301,8 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase { wgpu::Texture* dawnTextureOut, const wgpu::Color& clearColor, ID3D11Texture2D** d3d11TextureOut, - IDXGIKeyedMutex** dxgiKeyedMutexOut) const { + IDXGIKeyedMutex** dxgiKeyedMutexOut, + bool isCleared = true) const { ComPtr d3d11Texture; HRESULT hr = mD3d11Device->CreateTexture2D(d3dDescriptor, nullptr, &d3d11Texture); ASSERT_EQ(hr, S_OK); @@ -338,6 +339,7 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase { reinterpret_cast(dawnDescriptor); externDesc.sharedHandle = sharedHandle; externDesc.acquireMutexKey = 1; + externDesc.isCleared = isCleared; WGPUTexture dawnTexture = dawn_native::d3d12::WrapSharedHandle(device.Get(), &externDesc); *dawnTextureOut = wgpu::Texture::Acquire(dawnTexture); @@ -499,5 +501,23 @@ TEST_P(D3D12SharedHandleUsageTests, ClearTwiceInD3D12ReadbackInD3D11) { ExpectPixelRGBA8EQ(2, d3d11Texture.Get(), dxgiKeyedMutex.Get(), d3d12ClearColor2); } +// 1. Create and clear a D3D11 texture with clearColor +// 2. Import the texture with isCleared = false +// 3. Verify clearColor is not visible in wrapped texture +TEST_P(D3D12SharedHandleUsageTests, UnclearedTextureIsCleared) { + DAWN_SKIP_TEST_IF(UsesWire()); + + const wgpu::Color clearColor{1.0f, 0.0f, 0.0f, 1.0f}; + wgpu::Texture dawnTexture; + ComPtr d3d11Texture; + ComPtr dxgiKeyedMutex; + WrapAndClearD3D11Texture(&dawnDescriptor, &d3dDescriptor, &dawnTexture, clearColor, + &d3d11Texture, &dxgiKeyedMutex, false); + + // Readback the destination texture and ensure it contains the colors we used + // to clear the source texture on the D3D device. + EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 0, 0, 0), dawnTexture, 0, 0); +} + DAWN_INSTANTIATE_TEST(D3D12SharedHandleValidation, D3D12Backend()); DAWN_INSTANTIATE_TEST(D3D12SharedHandleUsageTests, D3D12Backend()); diff --git a/src/tests/end2end/IOSurfaceWrappingTests.cpp b/src/tests/end2end/IOSurfaceWrappingTests.cpp index 1119309862..29d5fda2d8 100644 --- a/src/tests/end2end/IOSurfaceWrappingTests.cpp +++ b/src/tests/end2end/IOSurfaceWrappingTests.cpp @@ -95,12 +95,14 @@ namespace { public: wgpu::Texture WrapIOSurface(const wgpu::TextureDescriptor* descriptor, IOSurfaceRef ioSurface, - uint32_t plane) { + uint32_t plane, + bool isCleared = true) { dawn_native::metal::ExternalImageDescriptorIOSurface externDesc; externDesc.cTextureDescriptor = reinterpret_cast(descriptor); externDesc.ioSurface = ioSurface; externDesc.plane = plane; + externDesc.isCleared = isCleared; WGPUTexture texture = dawn_native::metal::WrapIOSurface(device.Get(), &externDesc); return wgpu::Texture::Acquire(texture); } @@ -442,5 +444,30 @@ TEST_P(IOSurfaceUsageTests, ClearRGBA8IOSurface) { DoClearTest(ioSurface.get(), wgpu::TextureFormat::RGBA8Unorm, &data, sizeof(data)); } +// Test that texture with color is cleared when isCleared = false +TEST_P(IOSurfaceUsageTests, UnclearedTextureIsCleared) { + DAWN_SKIP_TEST_IF(UsesWire()); + + ScopedIOSurfaceRef ioSurface = CreateSinglePlaneIOSurface(1, 1, 'RGBA', 4); + uint32_t data = 0x04030201; + + IOSurfaceLock(ioSurface.get(), 0, nullptr); + memcpy(IOSurfaceGetBaseAddress(ioSurface.get()), &data, sizeof(data)); + IOSurfaceUnlock(ioSurface.get(), 0, nullptr); + + wgpu::TextureDescriptor textureDescriptor; + textureDescriptor.dimension = wgpu::TextureDimension::e2D; + textureDescriptor.format = wgpu::TextureFormat::RGBA8Unorm; + textureDescriptor.size = {1, 1, 1}; + textureDescriptor.sampleCount = 1; + textureDescriptor.arrayLayerCount = 1; + textureDescriptor.mipLevelCount = 1; + textureDescriptor.usage = wgpu::TextureUsage::OutputAttachment | wgpu::TextureUsage::CopySrc; + + // wrap ioSurface and ensure color is not visible when isCleared set to false + wgpu::Texture ioSurfaceTexture = WrapIOSurface(&textureDescriptor, ioSurface.get(), 0, false); + EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 0, 0, 0), ioSurfaceTexture, 0, 0); +} + DAWN_INSTANTIATE_TEST(IOSurfaceValidationTests, MetalBackend()); DAWN_INSTANTIATE_TEST(IOSurfaceUsageTests, MetalBackend());