From c7e16e351f37d0809ab7618efed5402fde46273f Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Mon, 16 Mar 2020 17:48:26 +0000 Subject: [PATCH] Fix D3D12 over-eager lazy zero initialization for textures Bug: dawn:145, dawn:348 Change-Id: Iafa1644424e67020b004765a0c9ccff2e077ead3 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/16980 Reviewed-by: Kai Ninomiya Reviewed-by: Corentin Wallez Commit-Queue: Austin Eng --- src/dawn_native/d3d12/TextureD3D12.cpp | 118 +++++++++++------- src/dawn_native/d3d12/TextureD3D12.h | 6 +- .../end2end/NonzeroTextureCreationTests.cpp | 6 +- src/tests/end2end/TextureZeroInitTests.cpp | 4 +- 4 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index e78d9a67d2..cb570793fd 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -538,7 +538,7 @@ namespace dawn_native { namespace d3d12 { return true; } - D3D12_RENDER_TARGET_VIEW_DESC Texture::GetRTVDescriptor(uint32_t baseMipLevel, + D3D12_RENDER_TARGET_VIEW_DESC Texture::GetRTVDescriptor(uint32_t mipLevel, uint32_t baseArrayLayer, uint32_t layerCount) const { ASSERT(GetDimension() == wgpu::TextureDimension::e2D); @@ -548,7 +548,7 @@ namespace dawn_native { namespace d3d12 { ASSERT(GetNumMipLevels() == 1); ASSERT(layerCount == 1); ASSERT(baseArrayLayer == 0); - ASSERT(baseMipLevel == 0); + ASSERT(mipLevel == 0); rtvDesc.ViewDimension = D3D12_RTV_DIMENSION_TEXTURE2DMS; } else { // Currently we always use D3D12_TEX2D_ARRAY_RTV because we cannot specify base array @@ -560,23 +560,30 @@ namespace dawn_native { namespace d3d12 { rtvDesc.ViewDimension = D3D12_RTV_DIMENSION_TEXTURE2DARRAY; rtvDesc.Texture2DArray.FirstArraySlice = baseArrayLayer; rtvDesc.Texture2DArray.ArraySize = layerCount; - rtvDesc.Texture2DArray.MipSlice = baseMipLevel; + rtvDesc.Texture2DArray.MipSlice = mipLevel; rtvDesc.Texture2DArray.PlaneSlice = 0; } return rtvDesc; } - D3D12_DEPTH_STENCIL_VIEW_DESC Texture::GetDSVDescriptor(uint32_t baseMipLevel) const { + D3D12_DEPTH_STENCIL_VIEW_DESC Texture::GetDSVDescriptor(uint32_t mipLevel, + uint32_t baseArrayLayer, + uint32_t layerCount) const { D3D12_DEPTH_STENCIL_VIEW_DESC dsvDesc; dsvDesc.Format = GetD3D12Format(); dsvDesc.Flags = D3D12_DSV_FLAG_NONE; - ASSERT(baseMipLevel == 0); if (IsMultisampledTexture()) { + ASSERT(GetNumMipLevels() == 1); + ASSERT(layerCount == 1); + ASSERT(baseArrayLayer == 0); + ASSERT(mipLevel == 0); dsvDesc.ViewDimension = D3D12_DSV_DIMENSION_TEXTURE2DMS; } else { - dsvDesc.ViewDimension = D3D12_DSV_DIMENSION_TEXTURE2D; - dsvDesc.Texture2D.MipSlice = baseMipLevel; + dsvDesc.ViewDimension = D3D12_DSV_DIMENSION_TEXTURE2DARRAY; + dsvDesc.Texture2DArray.FirstArraySlice = baseArrayLayer; + dsvDesc.Texture2DArray.ArraySize = layerCount; + dsvDesc.Texture2DArray.MipSlice = mipLevel; } return dsvDesc; @@ -606,40 +613,62 @@ namespace dawn_native { namespace d3d12 { if (GetFormat().isRenderable) { if (GetFormat().HasDepthOrStencil()) { TrackUsageAndTransitionNow(commandContext, D3D12_RESOURCE_STATE_DEPTH_WRITE); - DescriptorHeapHandle dsvHeap; - DAWN_TRY_ASSIGN(dsvHeap, descriptorHeapAllocator->AllocateCPUHeap( - D3D12_DESCRIPTOR_HEAP_TYPE_DSV, 1)); - D3D12_CPU_DESCRIPTOR_HANDLE dsvHandle = dsvHeap.GetCPUHandle(0); - D3D12_DEPTH_STENCIL_VIEW_DESC dsvDesc = GetDSVDescriptor(baseMipLevel); - device->GetD3D12Device()->CreateDepthStencilView(GetD3D12Resource(), &dsvDesc, - dsvHandle); D3D12_CLEAR_FLAGS clearFlags = {}; - if (GetFormat().HasDepth()) { - clearFlags |= D3D12_CLEAR_FLAG_DEPTH; - } - if (GetFormat().HasStencil()) { - clearFlags |= D3D12_CLEAR_FLAG_STENCIL; - } - commandList->ClearDepthStencilView(dsvHandle, clearFlags, fClearColor, clearColor, - 0, nullptr); + for (uint32_t level = baseMipLevel; level < baseMipLevel + levelCount; ++level) { + for (uint32_t layer = baseArrayLayer; layer < baseArrayLayer + layerCount; + ++layer) { + if (clearValue == TextureBase::ClearValue::Zero && + IsSubresourceContentInitialized(level, 1, layer, 1)) { + // Skip lazy clears if already initialized. + continue; + } + + DescriptorHeapHandle dsvHeap; + DAWN_TRY_ASSIGN(dsvHeap, descriptorHeapAllocator->AllocateCPUHeap( + D3D12_DESCRIPTOR_HEAP_TYPE_DSV, 1)); + D3D12_CPU_DESCRIPTOR_HANDLE dsvHandle = dsvHeap.GetCPUHandle(0); + D3D12_DEPTH_STENCIL_VIEW_DESC dsvDesc = GetDSVDescriptor(level, layer, 1); + device->GetD3D12Device()->CreateDepthStencilView(GetD3D12Resource(), + &dsvDesc, dsvHandle); + + if (GetFormat().HasDepth()) { + clearFlags |= D3D12_CLEAR_FLAG_DEPTH; + } + if (GetFormat().HasStencil()) { + clearFlags |= D3D12_CLEAR_FLAG_STENCIL; + } + + commandList->ClearDepthStencilView(dsvHandle, clearFlags, fClearColor, + clearColor, 0, nullptr); + } + } } else { TrackUsageAndTransitionNow(commandContext, D3D12_RESOURCE_STATE_RENDER_TARGET); - DescriptorHeapHandle rtvHeap; - DAWN_TRY_ASSIGN(rtvHeap, descriptorHeapAllocator->AllocateCPUHeap( - D3D12_DESCRIPTOR_HEAP_TYPE_RTV, 1)); - D3D12_CPU_DESCRIPTOR_HANDLE rtvHandle = rtvHeap.GetCPUHandle(0); + const float clearColorRGBA[4] = {fClearColor, fClearColor, fClearColor, fClearColor}; - // TODO(natlee@microsoft.com): clear all array layers for 2D array textures - for (uint32_t i = baseMipLevel; i < baseMipLevel + levelCount; i++) { - D3D12_RENDER_TARGET_VIEW_DESC rtvDesc = - GetRTVDescriptor(i, baseArrayLayer, layerCount); - device->GetD3D12Device()->CreateRenderTargetView(GetD3D12Resource(), &rtvDesc, - rtvHandle); - commandList->ClearRenderTargetView(rtvHandle, clearColorRGBA, 0, nullptr); + for (uint32_t level = baseMipLevel; level < baseMipLevel + levelCount; ++level) { + for (uint32_t layer = baseArrayLayer; layer < baseArrayLayer + layerCount; + ++layer) { + if (clearValue == TextureBase::ClearValue::Zero && + IsSubresourceContentInitialized(level, 1, layer, 1)) { + // Skip lazy clears if already initialized. + continue; + } + + DescriptorHeapHandle rtvHeap; + DAWN_TRY_ASSIGN(rtvHeap, descriptorHeapAllocator->AllocateCPUHeap( + D3D12_DESCRIPTOR_HEAP_TYPE_RTV, 1)); + D3D12_CPU_DESCRIPTOR_HANDLE rtvHandle = rtvHeap.GetCPUHandle(0); + + D3D12_RENDER_TARGET_VIEW_DESC rtvDesc = GetRTVDescriptor(level, layer, 1); + device->GetD3D12Device()->CreateRenderTargetView(GetD3D12Resource(), + &rtvDesc, rtvHandle); + commandList->ClearRenderTargetView(rtvHandle, clearColorRGBA, 0, nullptr); + } } } } else { @@ -661,14 +690,20 @@ namespace dawn_native { namespace d3d12 { TrackUsageAndTransitionNow(commandContext, D3D12_RESOURCE_STATE_COPY_DEST); - // compute d3d12 texture copy locations for texture and buffer - Extent3D copySize = {GetSize().width, GetSize().height, 1}; - TextureCopySplit copySplit = ComputeTextureCopySplit( - {0, 0, 0}, copySize, GetFormat(), uploadHandle.startOffset, rowPitch, 0); - for (uint32_t level = baseMipLevel; level < baseMipLevel + levelCount; ++level) { + // compute d3d12 texture copy locations for texture and buffer + Extent3D copySize = GetMipLevelVirtualSize(level); + TextureCopySplit copySplit = ComputeTextureCopySplit( + {0, 0, 0}, copySize, GetFormat(), uploadHandle.startOffset, rowPitch, 0); + for (uint32_t layer = baseArrayLayer; layer < baseArrayLayer + layerCount; ++layer) { + if (clearValue == TextureBase::ClearValue::Zero && + IsSubresourceContentInitialized(level, 1, layer, 1)) { + // Skip lazy clears if already initialized. + continue; + } + D3D12_TEXTURE_COPY_LOCATION textureLocation = ComputeTextureCopyLocationForTexture(this, level, layer); for (uint32_t i = 0; i < copySplit.count; ++i) { @@ -770,11 +805,10 @@ namespace dawn_native { namespace d3d12 { D3D12_DEPTH_STENCIL_VIEW_DESC TextureView::GetDSVDescriptor() const { // TODO(jiawei.shao@intel.com): support rendering into a layer of a texture. - ASSERT(GetLayerCount() == 1); ASSERT(GetLevelCount() == 1); - ASSERT(GetBaseMipLevel() == 0); - ASSERT(GetBaseArrayLayer() == 0); - return ToBackend(GetTexture())->GetDSVDescriptor(GetBaseMipLevel()); + uint32_t mipLevel = GetBaseMipLevel(); + return ToBackend(GetTexture()) + ->GetDSVDescriptor(mipLevel, GetBaseArrayLayer(), GetLayerCount()); } }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/TextureD3D12.h b/src/dawn_native/d3d12/TextureD3D12.h index 17a7d6cd8d..69656903bb 100644 --- a/src/dawn_native/d3d12/TextureD3D12.h +++ b/src/dawn_native/d3d12/TextureD3D12.h @@ -49,10 +49,12 @@ namespace dawn_native { namespace d3d12 { DXGI_FORMAT GetD3D12Format() const; ID3D12Resource* GetD3D12Resource() const; - D3D12_RENDER_TARGET_VIEW_DESC GetRTVDescriptor(uint32_t baseMipLevel, + D3D12_RENDER_TARGET_VIEW_DESC GetRTVDescriptor(uint32_t mipLevel, + uint32_t baseArrayLayer, + uint32_t layerCount) const; + D3D12_DEPTH_STENCIL_VIEW_DESC GetDSVDescriptor(uint32_t mipLevel, uint32_t baseArrayLayer, uint32_t layerCount) const; - D3D12_DEPTH_STENCIL_VIEW_DESC GetDSVDescriptor(uint32_t baseMipLevel) const; void EnsureSubresourceContentInitialized(CommandRecordingContext* commandContext, uint32_t baseMipLevel, uint32_t levelCount, diff --git a/src/tests/end2end/NonzeroTextureCreationTests.cpp b/src/tests/end2end/NonzeroTextureCreationTests.cpp index 59c01b92ae..7f384cc121 100644 --- a/src/tests/end2end/NonzeroTextureCreationTests.cpp +++ b/src/tests/end2end/NonzeroTextureCreationTests.cpp @@ -195,7 +195,7 @@ TEST_P(NonzeroTextureCreationTests, NonRenderableTextureClearWithMultiArrayLayer // Test that all subresources of a renderable texture are filled because the toggle is enabled. TEST_P(NonzeroTextureCreationTests, AllSubresourcesFilled) { // TODO(crbug.com/dawn/145): Implement on other platforms. - DAWN_SKIP_TEST_IF(!IsMetal()); + DAWN_SKIP_TEST_IF(!IsMetal() && !IsD3D12()); wgpu::TextureDescriptor baseDescriptor; baseDescriptor.dimension = wgpu::TextureDimension::e2D; @@ -251,7 +251,7 @@ TEST_P(NonzeroTextureCreationTests, AllSubresourcesFilled) { // Test that all subresources of a nonrenderable texture are filled because the toggle is enabled. TEST_P(NonzeroTextureCreationTests, NonRenderableAllSubresourcesFilled) { // TODO(crbug.com/dawn/145): Implement on other platforms. - DAWN_SKIP_TEST_IF(!IsMetal()); + DAWN_SKIP_TEST_IF(!IsMetal() && !IsD3D12()); wgpu::TextureDescriptor baseDescriptor; baseDescriptor.dimension = wgpu::TextureDimension::e2D; @@ -259,7 +259,7 @@ TEST_P(NonzeroTextureCreationTests, NonRenderableAllSubresourcesFilled) { baseDescriptor.size.height = kSize; baseDescriptor.size.depth = 1; baseDescriptor.sampleCount = 1; - baseDescriptor.format = wgpu::TextureFormat::RGBA8Unorm; + baseDescriptor.format = wgpu::TextureFormat::RGBA8Snorm; baseDescriptor.mipLevelCount = 1; baseDescriptor.arrayLayerCount = 1; baseDescriptor.usage = wgpu::TextureUsage::CopySrc; diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp index cb582266f1..0ceaed5195 100644 --- a/src/tests/end2end/TextureZeroInitTests.cpp +++ b/src/tests/end2end/TextureZeroInitTests.cpp @@ -872,7 +872,7 @@ TEST_P(TextureZeroInitTest, RenderingLoadingDepthStencilStoreOpClear) { // uninitialized mip does not clear the initialized mip. TEST_P(TextureZeroInitTest, PreservesInitializedMip) { // TODO(crbug.com/dawn/145): Fix this on other backends - DAWN_SKIP_TEST_IF(!IsMetal()); + DAWN_SKIP_TEST_IF(!IsMetal() && !IsD3D12()); wgpu::TextureDescriptor sampleTextureDescriptor = CreateTextureDescriptor( 2, 1, @@ -953,7 +953,7 @@ TEST_P(TextureZeroInitTest, PreservesInitializedMip) { // the uninitialized layer does not clear the initialized layer. TEST_P(TextureZeroInitTest, PreservesInitializedArrayLayer) { // TODO(crbug.com/dawn/145): Fix this on other backends - DAWN_SKIP_TEST_IF(!IsMetal()); + DAWN_SKIP_TEST_IF(!IsMetal() && !IsD3D12()); wgpu::TextureDescriptor sampleTextureDescriptor = CreateTextureDescriptor( 1, 2,