Revert "d3d11: Use DiscardView for attachments with storeOp=Discard" This reverts commit b0cca8a144479db0c04f849ec50a5ee4ed611062. Reason for revert: Suspected to cause regressions in https://crbug.com/485540062 Original change's description: > d3d11: Use DiscardView for attachments with storeOp=Discard > > This patch updates CommandBufferD3D11 to call > ID3D11DeviceContext::DiscardView for any color or depth/stencil > attachment that has wgpu::StoreOp::Discard. > > Bug: 479416037 > Change-Id: I58d08afa9d403e7c5a4c719f210d8ee939626acd > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/290255 > Reviewed-by: Corentin Wallez <cwallez@chromium.org> > Commit-Queue: Quyen Le <lehoangquyen@chromium.org> # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 479416037 Change-Id: I94c585b6bda85007d5542ec7972c49b4eea8e609 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/292875 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Quyen Le <lehoangquyen@chromium.org>
diff --git a/src/dawn/native/d3d11/CommandBufferD3D11.cpp b/src/dawn/native/d3d11/CommandBufferD3D11.cpp index 505a182..8b06047 100644 --- a/src/dawn/native/d3d11/CommandBufferD3D11.cpp +++ b/src/dawn/native/d3d11/CommandBufferD3D11.cpp
@@ -892,53 +892,35 @@ switch (type) { case Command::EndRenderPass: { mCommands.NextCommand<EndRenderPassCmd>(); - d3d11DeviceContext->OMSetRenderTargets(0, nullptr, nullptr); - if (renderPass->attachmentState->GetSampleCount() > 1) { - // Resolve multisampled textures. - for (auto i : renderPass->attachmentState->GetColorAttachmentsMask()) { - const auto& attachment = renderPass->colorAttachments[i]; - if (!attachment.resolveTarget.Get()) { - continue; - } - - DAWN_ASSERT(attachment.view->GetAspects() == Aspect::Color); - DAWN_ASSERT(attachment.resolveTarget->GetAspects() == Aspect::Color); - - Texture* resolveTexture = ToBackend(attachment.resolveTarget->GetTexture()); - Texture* colorTexture = ToBackend(attachment.view->GetTexture()); - uint32_t dstSubresource = resolveTexture->GetSubresourceIndex( - attachment.resolveTarget->GetBaseMipLevel(), - attachment.resolveTarget->GetBaseArrayLayer(), Aspect::Color); - uint32_t srcSubresource = colorTexture->GetSubresourceIndex( - attachment.view->GetBaseMipLevel(), - attachment.view->GetBaseArrayLayer(), Aspect::Color); - d3d11DeviceContext->ResolveSubresource( - resolveTexture->GetD3D11Resource(), dstSubresource, - colorTexture->GetD3D11Resource(), srcSubresource, - d3d::DXGITextureFormat(GetDevice(), - attachment.resolveTarget->GetFormat().format)); - } + if (renderPass->attachmentState->GetSampleCount() <= 1) { + return {}; } + // Resolve multisampled textures. for (auto i : renderPass->attachmentState->GetColorAttachmentsMask()) { - if (renderPass->colorAttachments[i].storeOp == wgpu::StoreOp::Discard) { - d3d11DeviceContext->DiscardView(d3d11RenderTargetViews[i]); + const auto& attachment = renderPass->colorAttachments[i]; + if (!attachment.resolveTarget.Get()) { + continue; } - } - if (renderPass->attachmentState->HasDepthStencilAttachment()) { - auto* attachmentInfo = &renderPass->depthStencilAttachment; - const Format& attachmentFormat = - attachmentInfo->view->GetTexture()->GetFormat(); - bool discardDepth = !attachmentFormat.HasDepth() || - attachmentInfo->depthStoreOp == wgpu::StoreOp::Discard; - bool discardStencil = !attachmentFormat.HasStencil() || - attachmentInfo->stencilStoreOp == wgpu::StoreOp::Discard; - if (discardDepth && discardStencil) { - d3d11DeviceContext->DiscardView(d3d11DepthStencilView); - } + DAWN_ASSERT(attachment.view->GetAspects() == Aspect::Color); + DAWN_ASSERT(attachment.resolveTarget->GetAspects() == Aspect::Color); + + Texture* resolveTexture = ToBackend(attachment.resolveTarget->GetTexture()); + Texture* colorTexture = ToBackend(attachment.view->GetTexture()); + uint32_t dstSubresource = resolveTexture->GetSubresourceIndex( + attachment.resolveTarget->GetBaseMipLevel(), + attachment.resolveTarget->GetBaseArrayLayer(), Aspect::Color); + uint32_t srcSubresource = colorTexture->GetSubresourceIndex( + attachment.view->GetBaseMipLevel(), attachment.view->GetBaseArrayLayer(), + Aspect::Color); + d3d11DeviceContext->ResolveSubresource( + resolveTexture->GetD3D11Resource(), dstSubresource, + colorTexture->GetD3D11Resource(), srcSubresource, + d3d::DXGITextureFormat(GetDevice(), + attachment.resolveTarget->GetFormat().format)); } return {};
diff --git a/src/dawn/tests/end2end/TextureZeroInitTests.cpp b/src/dawn/tests/end2end/TextureZeroInitTests.cpp index 5851e5f..ed2aeb2 100644 --- a/src/dawn/tests/end2end/TextureZeroInitTests.cpp +++ b/src/dawn/tests/end2end/TextureZeroInitTests.cpp
@@ -925,9 +925,6 @@ // Test that a stencil texture that is written via copy, then discarded, sees // zero contents when it is read via copy. TEST_P(TextureZeroInitTest, StencilCopyThenDiscardAndReadByCopy) { - // TODO(crbug.com/479416037): QC's D3D11's DiscardView seems to have some bugs. - DAWN_SUPPRESS_TEST_IF(IsD3D11() && IsQualcomm()); - for (wgpu::TextureFormat format : {wgpu::TextureFormat::Stencil8, wgpu::TextureFormat::Depth24PlusStencil8}) { wgpu::Texture depthStencilTexture = CreateAndFillStencilTexture(format); @@ -958,9 +955,6 @@ // Test that a stencil texture that is written via copy, then discarded, then copied to // another texture, sees zero contents when it is read via copy. TEST_P(TextureZeroInitTest, StencilCopyThenDiscardAndCopyToTextureThenReadByCopy) { - // TODO(crbug.com/479416037): QC's D3D11's DiscardView seems to have some bugs. - DAWN_SUPPRESS_TEST_IF(IsD3D11() && IsQualcomm()); - // TODO(crbug.com/468047554): Fails on Win11/NVIDIA GTX 1660. DAWN_SUPPRESS_TEST_IF(IsWindows11() && IsNvidia() && IsD3D12() && IsBackendValidationEnabled()); @@ -1391,9 +1385,6 @@ // Then expect the render texture to not store the data from sample texture // because it will be lazy cleared by the EXPECT_TEXTURE_EQ call. TEST_P(TextureZeroInitTest, RenderPassStoreOpClear) { - // TODO(crbug.com/479416037): QC's D3D11's DiscardView seems to have some bugs. - DAWN_SUPPRESS_TEST_IF(IsD3D11() && IsQualcomm()); - // Create needed resources wgpu::TextureDescriptor descriptor = CreateTextureDescriptor( 1, 1, wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::CopyDst, kColorFormat); @@ -1538,9 +1529,6 @@ // Test that if one mip of a texture is initialized and another is uninitialized, lazy clearing the // uninitialized mip does not clear the initialized mip. TEST_P(TextureZeroInitTest, PreservesInitializedMip) { - // TODO(crbug.com/479416037): QC's D3D11's DiscardView seems to have some bugs. - DAWN_SUPPRESS_TEST_IF(IsD3D11() && IsQualcomm()); - wgpu::TextureDescriptor sampleTextureDescriptor = CreateTextureDescriptor(2, 1, wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst | @@ -1617,9 +1605,6 @@ // Test that if one layer of a texture is initialized and another is uninitialized, lazy clearing // the uninitialized layer does not clear the initialized layer. TEST_P(TextureZeroInitTest, PreservesInitializedArrayLayer) { - // TODO(crbug.com/479416037): QC's D3D11's DiscardView seems to have some bugs. - DAWN_SUPPRESS_TEST_IF(IsD3D11() && IsQualcomm()); - // TODO(crbug.com/346362367): Compatibility mode does not support binding a `2d-array` texture // to a WGSL variable of type `texture_2d`. DAWN_TEST_UNSUPPORTED_IF(IsCompatibilityMode());