Explicitly clear attachments used with RenderArea When using RenderArea only a portion of the attachments of a render pass were being cleared, but we were marking the full subresource as initialized. This change performs an explicit clear on the full subresource if a partial RenderArea has been set to enforce that the attachments are always properly initialized. Bug: 501780768 Fixed: 501780768 Change-Id: I79e8f03fed101df9a4baa3c0a89c361454919a19 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/302577 Reviewed-by: Kai Ninomiya <kainino@chromium.org> Commit-Queue: Brandon Jones <bajones@chromium.org>
diff --git a/src/dawn/native/CommandBuffer.cpp b/src/dawn/native/CommandBuffer.cpp index b7b346d..27b3ce2 100644 --- a/src/dawn/native/CommandBuffer.cpp +++ b/src/dawn/native/CommandBuffer.cpp
@@ -163,11 +163,18 @@ MaybeError LazyClearRenderPassAttachments(DeviceBase* device, BeginRenderPassCmd* renderPass, - LazyClearTexture3DHelper clearTexture3D) { + LazyClearTextureHelper clearTexture) { if (!device->IsToggleEnabled(Toggle::LazyClearResourceOnFirstUse)) { return {}; } + // Detect if a renderArea has been set that only covers part of the render pass attachments. + // If so we'll be performing explicit clears for any uninitialized attachments below, since a + // render pass clear won't initialize the entire attachment. + bool partialRenderArea = (renderPass->renderArea.x != 0 || renderPass->renderArea.y != 0 || + renderPass->renderArea.width != renderPass->width || + renderPass->renderArea.height != renderPass->height); + for (auto i : renderPass->attachmentState->GetColorAttachmentsMask()) { auto& attachmentInfo = renderPass->colorAttachments[i]; TextureViewBase* view = attachmentInfo.view.Get(); @@ -178,31 +185,45 @@ SubresourceRange range = view->GetSubresourceRange(); TextureBase* texture = view->GetTexture(); - // If the loadOp is Load, but the subresource is not initialized, use Clear instead. - if (attachmentInfo.loadOp == wgpu::LoadOp::Load && - !texture->IsSubresourceContentInitialized(range)) { - attachmentInfo.loadOp = wgpu::LoadOp::Clear; - attachmentInfo.clearColor = {0.f, 0.f, 0.f, 0.f}; - } - - // For 3D textures, rendering to a single depthSlice marks the entire mip level as - // initialized. If it wasn't already initialized, we must clear the other slices - // before the render pass starts. - // TODO(500975625): Optimize this. - if (texture->GetDimension() == wgpu::TextureDimension::e3D && - !texture->IsSubresourceContentInitialized(range)) { - DAWN_TRY(clearTexture3D(texture, range)); + if (!texture->IsSubresourceContentInitialized(range)) { + if (partialRenderArea || texture->GetDimension() == wgpu::TextureDimension::e3D) { + // Using a renderArea that only partially covers the attachment still marks the full + // subresource as initialized. If it wasn't already initialized we must clear the + // full subresource before the render pass starts. + // + // For 3D textures, rendering to a single depthSlice marks the entire mip level as + // initialized. If it wasn't already initialized, we must clear the other slices + // before the render pass starts. + // + // TODO(500975625): Optimize this. + DAWN_TRY(clearTexture(texture, range)); + } else if (attachmentInfo.loadOp == wgpu::LoadOp::Load) { + // If the loadOp is Load, but the subresource is not initialized, use Clear instead. + attachmentInfo.loadOp = wgpu::LoadOp::Clear; + attachmentInfo.clearColor = {0.f, 0.f, 0.f, 0.f}; + } } if (hasResolveTarget) { - // We need to set the resolve target to initialized so that it does not get - // cleared later in the pipeline. The texture will be resolved from the - // source color attachment, which will be correctly initialized. TextureViewBase* resolveView = attachmentInfo.resolveTarget.Get(); DAWN_ASSERT(resolveView->GetLayerCount() == 1); DAWN_ASSERT(resolveView->GetLevelCount() == 1); - resolveView->GetTexture()->SetIsSubresourceContentInitialized( - true, resolveView->GetSubresourceRange()); + if (!resolveView->GetTexture()->IsSubresourceContentInitialized( + resolveView->GetSubresourceRange())) { + if (partialRenderArea) { + // Using a renderArea that only partially covers the attachment means that the + // resolve target won't have the full subresource copied over. If it wasn't + // already initialized we must clear the full subresource before the pass. + DAWN_TRY(clearTexture(resolveView->GetTexture(), + resolveView->GetSubresourceRange())); + } else { + // Otherwise we need to set the resolve target to initialized so that it does + // not get cleared later in the pipeline. The texture will be resolved from the + // source color attachment, which will be correctly initialized. + resolveView->GetTexture()->SetIsSubresourceContentInitialized( + true, resolveView->GetSubresourceRange()); + } + } } switch (attachmentInfo.storeOp) { @@ -235,16 +256,22 @@ // If the depth stencil texture has not been initialized, we want to use loadop // clear to init the contents to 0's - if (!view->GetTexture()->IsSubresourceContentInitialized(depthRange) && - attachmentInfo.depthLoadOp == wgpu::LoadOp::Load) { - attachmentInfo.clearDepth = 0.0f; - attachmentInfo.depthLoadOp = wgpu::LoadOp::Clear; + if (!view->GetTexture()->IsSubresourceContentInitialized(depthRange)) { + if (partialRenderArea) { + DAWN_TRY(clearTexture(view->GetTexture(), depthRange)); + } else if (attachmentInfo.depthLoadOp == wgpu::LoadOp::Load) { + attachmentInfo.clearDepth = 0.0f; + attachmentInfo.depthLoadOp = wgpu::LoadOp::Clear; + } } - if (!view->GetTexture()->IsSubresourceContentInitialized(stencilRange) && - attachmentInfo.stencilLoadOp == wgpu::LoadOp::Load) { - attachmentInfo.clearStencil = 0u; - attachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear; + if (!view->GetTexture()->IsSubresourceContentInitialized(stencilRange)) { + if (partialRenderArea) { + DAWN_TRY(clearTexture(view->GetTexture(), stencilRange)); + } else if (attachmentInfo.stencilLoadOp == wgpu::LoadOp::Load) { + attachmentInfo.clearStencil = 0u; + attachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear; + } } view->GetTexture()->SetIsSubresourceContentInitialized( @@ -266,11 +293,15 @@ DAWN_ASSERT(view->GetLevelCount() == 1); const SubresourceRange& range = view->GetSubresourceRange(); - // If the loadOp is Load, but the subresource is not initialized, use Clear instead. - if (attachmentInfo.loadOp == wgpu::LoadOp::Load && - !view->GetTexture()->IsSubresourceContentInitialized(range)) { - attachmentInfo.loadOp = wgpu::LoadOp::Clear; - attachmentInfo.clearColor = {0.f, 0.f, 0.f, 0.f}; + if (!view->GetTexture()->IsSubresourceContentInitialized(range)) { + if (partialRenderArea) { + DAWN_TRY(clearTexture(view->GetTexture(), range)); + } else if (attachmentInfo.loadOp == wgpu::LoadOp::Load) { + // If the loadOp is Load, but the subresource is not initialized, use Clear + // instead. + attachmentInfo.loadOp = wgpu::LoadOp::Clear; + attachmentInfo.clearColor = {0.f, 0.f, 0.f, 0.f}; + } } switch (attachmentInfo.storeOp) {
diff --git a/src/dawn/native/CommandBuffer.h b/src/dawn/native/CommandBuffer.h index 907115e..e63fa93 100644 --- a/src/dawn/native/CommandBuffer.h +++ b/src/dawn/native/CommandBuffer.h
@@ -92,10 +92,10 @@ SubresourceRange GetSubresourcesAffectedByCopy(const TextureCopy& copy, const TexelExtent3D& copySize); -using LazyClearTexture3DHelper = std::function<MaybeError(TextureBase*, const SubresourceRange&)>; +using LazyClearTextureHelper = std::function<MaybeError(TextureBase*, const SubresourceRange&)>; MaybeError LazyClearRenderPassAttachments(DeviceBase* device, BeginRenderPassCmd* renderPass, - LazyClearTexture3DHelper clearTexture); + LazyClearTextureHelper clearTexture); bool IsFullBufferOverwrittenInTextureToBufferCopy(const CopyTextureToBufferCmd* copy); bool IsFullBufferOverwrittenInTextureToBufferCopy(const TextureCopy& source,
diff --git a/src/dawn/native/vulkan/CommandBufferVk.cpp b/src/dawn/native/vulkan/CommandBufferVk.cpp index 35712df..bd0de87 100644 --- a/src/dawn/native/vulkan/CommandBufferVk.cpp +++ b/src/dawn/native/vulkan/CommandBufferVk.cpp
@@ -1313,9 +1313,18 @@ GetResourceUsages().renderPasses[nextRenderPassNumber])); DAWN_TRY(LazyClearRenderPassAttachments( - device, cmd, [&](TextureBase* texture, const SubresourceRange& range) { - return ToBackend(texture)->EnsureSubresourceContentInitialized( - recordingContext, range); + device, cmd, + [&](TextureBase* texture, const SubresourceRange& range) -> MaybeError { + Texture* textureVk = ToBackend(texture); + DAWN_TRY(textureVk->EnsureSubresourceContentInitialized(recordingContext, + range)); + // EnsureSubresourceContentInitialized may transition some textures to a + // different usage, so ensure they are transitioned back to RenderAttachment + // after clearing. + textureVk->TransitionUsageNow(recordingContext, + wgpu::TextureUsage::RenderAttachment, + wgpu::ShaderStage::None, range); + return {}; })); DAWN_TRY(RecordRenderPass(recordingContext, cmd));
diff --git a/src/dawn/tests/end2end/TextureZeroInitTests.cpp b/src/dawn/tests/end2end/TextureZeroInitTests.cpp index c1fd178..160914b 100644 --- a/src/dawn/tests/end2end/TextureZeroInitTests.cpp +++ b/src/dawn/tests/end2end/TextureZeroInitTests.cpp
@@ -2195,6 +2195,222 @@ "use_blit_for_buffer_to_stencil_texture_copy"}), VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"})); +// ============================================================================= +// LazyClearRenderPassAttachments must take sub-rect RenderPassRenderArea into +// account. Tests based on a Project Fortify-produced POC. +// ============================================================================= +class TextureZeroInitRenderAreaTest : public TextureZeroInitTest { + protected: + void SetUp() override { + TextureZeroInitTest::SetUp(); + DAWN_TEST_UNSUPPORTED_IF(!mRenderAreaSupported); + } + + std::vector<wgpu::FeatureName> GetRequiredFeatures() override { + mRenderAreaSupported = SupportsFeatures({wgpu::FeatureName::RenderPassRenderArea}); + if (!mRenderAreaSupported) { + return {}; + } + return {wgpu::FeatureName::RenderPassRenderArea}; + } + + bool mRenderAreaSupported = false; +}; + +// A fresh texture rendered with a sub-rect renderArea + LoadOp::Clear must be fully zero on +// subsequent readback. If pixels outside renderArea are not properly initialized they will read +// back as the nonzero "garbage" fill value, indicating Dawn improperly marked the whole mip +// initialized while only clearing the sub-rect. +TEST_P(TextureZeroInitRenderAreaTest, SubRectClearInitializesFullSubresource) { + // Use a large texture so that even a 32x32 render-area granularity (the + // common Vulkan max) cannot expand the sub-rect to cover the whole mip. + constexpr uint32_t kTexSize = 128; + constexpr uint32_t kAreaSize = 32; + + wgpu::TextureDescriptor descriptor; + descriptor.dimension = wgpu::TextureDimension::e2D; + descriptor.size = {kTexSize, kTexSize, 1}; + descriptor.format = kColorFormat; + descriptor.mipLevelCount = 1; + descriptor.usage = wgpu::TextureUsage::RenderAttachment | wgpu::TextureUsage::CopySrc; + wgpu::Texture texture = device.CreateTexture(&descriptor); + + // Texture is freshly created -> uninitialized (and pre-filled with 0xFF by + // the nonzero_clear_resources_on_creation_for_testing toggle to simulate + // recycled GPU heap garbage). + EXPECT_FALSE(native::IsTextureSubresourceInitialized(texture.Get(), 0, 1, 0, 1)); + + utils::ComboRenderPassDescriptor renderPass({texture.CreateView()}); + renderPass.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear; + renderPass.cColorAttachments[0].storeOp = wgpu::StoreOp::Store; + renderPass.cColorAttachments[0].clearValue = {0.0f, 0.0f, 0.0f, 0.0f}; + + wgpu::RenderPassRenderAreaRect renderArea; + renderArea.origin = {0, 0}; + renderArea.size = {kAreaSize, kAreaSize}; + renderPass.nextInChain = &renderArea; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.BeginRenderPass(&renderPass).End(); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + // Render target should be marked fully initialized + EXPECT_TRUE(native::IsTextureSubresourceInitialized(texture.Get(), 0, 1, 0, 1)); + + { + // Full subresource must be cleared + std::vector<utils::RGBA8> expected(kTexSize * kTexSize, {0, 0, 0, 0}); + EXPECT_TEXTURE_EQ(expected.data(), texture, {0, 0}, {kTexSize, kTexSize}, 0); + } +} + +TEST_P(TextureZeroInitRenderAreaTest, SubRectLoadInitializesFullSubresource) { + // Use a large texture so that even a 32x32 render-area granularity (the + // common Vulkan max) cannot expand the sub-rect to cover the whole mip. + constexpr uint32_t kTexSize = 128; + constexpr uint32_t kAreaSize = 32; + + wgpu::TextureDescriptor descriptor; + descriptor.dimension = wgpu::TextureDimension::e2D; + descriptor.size = {kTexSize, kTexSize, 1}; + descriptor.format = kColorFormat; + descriptor.mipLevelCount = 1; + descriptor.usage = wgpu::TextureUsage::RenderAttachment | wgpu::TextureUsage::CopySrc; + wgpu::Texture texture = device.CreateTexture(&descriptor); + + // Texture is freshly created -> uninitialized (and pre-filled with 0xFF by + // the nonzero_clear_resources_on_creation_for_testing toggle to simulate + // recycled GPU heap garbage). + EXPECT_FALSE(native::IsTextureSubresourceInitialized(texture.Get(), 0, 1, 0, 1)); + + utils::ComboRenderPassDescriptor renderPass({texture.CreateView()}); + renderPass.cColorAttachments[0].loadOp = wgpu::LoadOp::Load; + renderPass.cColorAttachments[0].storeOp = wgpu::StoreOp::Store; + + wgpu::RenderPassRenderAreaRect renderArea; + renderArea.origin = {0, 0}; + renderArea.size = {kAreaSize, kAreaSize}; + renderPass.nextInChain = &renderArea; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.BeginRenderPass(&renderPass).End(); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + // Render target should be marked fully initialized + EXPECT_TRUE(native::IsTextureSubresourceInitialized(texture.Get(), 0, 1, 0, 1)); + + { + // Full subresource must be cleared + std::vector<utils::RGBA8> expected(kTexSize * kTexSize, {0, 0, 0, 0}); + EXPECT_TEXTURE_EQ(expected.data(), texture, {0, 0}, {kTexSize, kTexSize}, 0); + } +} + +TEST_P(TextureZeroInitRenderAreaTest, SubRectDepthStencilInitializesFullSubresource) { + // Use a large texture so that even a 32x32 render-area granularity (the + // common Vulkan max) cannot expand the sub-rect to cover the whole mip. + constexpr uint32_t kTexSize = 128; + constexpr uint32_t kAreaSize = 32; + + wgpu::TextureDescriptor descriptor; + descriptor.dimension = wgpu::TextureDimension::e2D; + descriptor.size = {kTexSize, kTexSize, 1}; + descriptor.format = kDepthStencilFormat; + descriptor.mipLevelCount = 1; + descriptor.usage = wgpu::TextureUsage::RenderAttachment | wgpu::TextureUsage::CopySrc; + wgpu::Texture depthStencilTexture = device.CreateTexture(&descriptor); + + // Texture is freshly created -> uninitialized (and pre-filled with 0xFF by + // the nonzero_clear_resources_on_creation_for_testing toggle to simulate + // recycled GPU heap garbage). + EXPECT_FALSE(native::IsTextureSubresourceInitialized(depthStencilTexture.Get(), 0, 1, 0, 1)); + + utils::ComboRenderPassDescriptor renderPass({}, depthStencilTexture.CreateView()); + renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Clear; + renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear; + renderPass.cDepthStencilAttachmentInfo.depthClearValue = 0.0f; + renderPass.cDepthStencilAttachmentInfo.stencilClearValue = 0u; + renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store; + renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store; + + wgpu::RenderPassRenderAreaRect renderArea; + renderArea.origin = {0, 0}; + renderArea.size = {kAreaSize, kAreaSize}; + renderPass.nextInChain = &renderArea; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.BeginRenderPass(&renderPass).End(); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + // Render target should be marked fully initialized + EXPECT_TRUE(native::IsTextureSubresourceInitialized(depthStencilTexture.Get(), 0, 1, 0, 1)); + + { + // Full subresource must be cleared + std::vector<uint8_t> expected(kTexSize * kTexSize, 0); + EXPECT_TEXTURE_EQ(expected.data(), depthStencilTexture, {0, 0}, {kSize, kSize}, 0, + wgpu::TextureAspect::StencilOnly); + } +} + +TEST_P(TextureZeroInitRenderAreaTest, SubRectResolveInitializesFullSubresource) { + constexpr uint32_t kTexSize = 128; + constexpr uint32_t kAreaSize = 32; + + wgpu::TextureDescriptor msaaDesc; + msaaDesc.dimension = wgpu::TextureDimension::e2D; + msaaDesc.size = {kTexSize, kTexSize, 1}; + msaaDesc.format = kColorFormat; + msaaDesc.sampleCount = 4; + msaaDesc.usage = wgpu::TextureUsage::RenderAttachment; + wgpu::Texture msaaTex = device.CreateTexture(&msaaDesc); + + wgpu::TextureDescriptor resolveDesc; + resolveDesc.dimension = wgpu::TextureDimension::e2D; + resolveDesc.size = {kTexSize, kTexSize, 1}; + resolveDesc.format = kColorFormat; + resolveDesc.usage = wgpu::TextureUsage::RenderAttachment | wgpu::TextureUsage::CopySrc; + wgpu::Texture resolveTex = device.CreateTexture(&resolveDesc); + + EXPECT_FALSE(native::IsTextureSubresourceInitialized(resolveTex.Get(), 0, 1, 0, 1)); + + utils::ComboRenderPassDescriptor renderPass({msaaTex.CreateView()}); + renderPass.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear; + renderPass.cColorAttachments[0].storeOp = wgpu::StoreOp::Discard; + renderPass.cColorAttachments[0].clearValue = {0.0f, 0.0f, 0.0f, 0.0f}; + renderPass.cColorAttachments[0].resolveTarget = resolveTex.CreateView(); + + wgpu::RenderPassRenderAreaRect renderArea; + renderArea.origin = {0, 0}; + renderArea.size = {kAreaSize, kAreaSize}; + renderPass.nextInChain = &renderArea; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.BeginRenderPass(&renderPass).End(); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + // Resolve target should be marked fully initialized. + EXPECT_TRUE(native::IsTextureSubresourceInitialized(resolveTex.Get(), 0, 1, 0, 1)); + + { + // Full subresource must be cleared + std::vector<utils::RGBA8> expected(kTexSize * kTexSize, {0, 0, 0, 0}); + EXPECT_TEXTURE_EQ(expected.data(), resolveTex, {0, 0}, {kTexSize, kTexSize}, 0); + } +} + +DAWN_INSTANTIATE_TEST(TextureZeroInitRenderAreaTest, + D3D11Backend({"nonzero_clear_resources_on_creation_for_testing"}), + D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}), + OpenGLBackend({"nonzero_clear_resources_on_creation_for_testing"}), + OpenGLESBackend({"nonzero_clear_resources_on_creation_for_testing"}), + MetalBackend({"nonzero_clear_resources_on_creation_for_testing"}), + VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"})); + class CompressedTextureZeroInitTest : public TextureZeroInitTest { protected: void SetUp() override {