diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index a0b7778b40..b98708c69d 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -234,7 +234,6 @@ source_set("dawn_native_sources") { "ObjectBase.h", "ObjectContentHasher.cpp", "ObjectContentHasher.h", - "PassResourceUsage.cpp", "PassResourceUsage.h", "PassResourceUsageTracker.cpp", "PassResourceUsageTracker.h", diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index 9a80bf9215..d65ead6b37 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -107,7 +107,6 @@ target_sources(dawn_native PRIVATE "IntegerTypes.h" "ObjectBase.cpp" "ObjectBase.h" - "PassResourceUsage.cpp" "PassResourceUsage.h" "PassResourceUsageTracker.cpp" "PassResourceUsageTracker.h" diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index c576517e5c..e0093dd4b3 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -332,15 +332,14 @@ namespace dawn_native { // combination of readonly usages. for (const PassTextureUsage& textureUsage : pass.textureUsages) { MaybeError error = {}; - textureUsage.subresourceUsages.Iterate( - [&](const SubresourceRange&, const wgpu::TextureUsage& usage) { - bool readOnly = IsSubset(usage, kReadOnlyTextureUsages); - bool singleUse = wgpu::HasZeroOrOneBits(usage); - if (!readOnly && !singleUse && !error.IsError()) { - error = DAWN_VALIDATION_ERROR( - "Texture used as writable usage and another usage in render pass"); - } - }); + textureUsage.Iterate([&](const SubresourceRange&, const wgpu::TextureUsage& usage) { + bool readOnly = IsSubset(usage, kReadOnlyTextureUsages); + bool singleUse = wgpu::HasZeroOrOneBits(usage); + if (!readOnly && !singleUse && !error.IsError()) { + error = DAWN_VALIDATION_ERROR( + "Texture used as writable usage and another usage in render pass"); + } + }); DAWN_TRY(std::move(error)); } return {}; diff --git a/src/dawn_native/PassResourceUsage.cpp b/src/dawn_native/PassResourceUsage.cpp deleted file mode 100644 index e56833afeb..0000000000 --- a/src/dawn_native/PassResourceUsage.cpp +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2021 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "dawn_native/PassResourceUsage.h" - -#include "dawn_native/Format.h" -#include "dawn_native/Texture.h" - -namespace dawn_native { - - PassTextureUsage::PassTextureUsage(const TextureBase* texture) - : subresourceUsages(texture->GetFormat().aspects, - texture->GetArrayLayers(), - texture->GetNumMipLevels(), - wgpu::TextureUsage::None) { - } - -} // namespace dawn_native diff --git a/src/dawn_native/PassResourceUsage.h b/src/dawn_native/PassResourceUsage.h index 9132f652f2..f0aa43a059 100644 --- a/src/dawn_native/PassResourceUsage.h +++ b/src/dawn_native/PassResourceUsage.h @@ -29,20 +29,8 @@ namespace dawn_native { enum class PassType { Render, Compute }; - // Describe the usage of the whole texture and its subresources. - // - // - usage variable is used the track the whole texture even though it can be deduced from - // subresources' usages. This is designed deliberately to track texture usage in a fast path - // at frontend. - // - // - subresourceUsages is used to track every subresource's usage within a texture. - struct PassTextureUsage { - // Constructor used to size subresourceUsages correctly. - PassTextureUsage(const TextureBase* texture); - - wgpu::TextureUsage usage = wgpu::TextureUsage::None; - SubresourceStorage subresourceUsages; - }; + // The texture usage inside passes must be tracked per-subresource. + using PassTextureUsage = SubresourceStorage; // Which resources are used by pass and how they are used. The command buffer validation // pre-computes this information so that backends with explicit barriers don't have to diff --git a/src/dawn_native/PassResourceUsageTracker.cpp b/src/dawn_native/PassResourceUsageTracker.cpp index 4a53ae7392..5816280a2a 100644 --- a/src/dawn_native/PassResourceUsageTracker.cpp +++ b/src/dawn_native/PassResourceUsageTracker.cpp @@ -19,6 +19,8 @@ #include "dawn_native/Format.h" #include "dawn_native/Texture.h" +#include + namespace dawn_native { PassResourceUsageTracker::PassResourceUsageTracker(PassType passType) : mPassType(passType) { } @@ -36,29 +38,31 @@ namespace dawn_native { // Get or create a new PassTextureUsage for that texture (initially filled with // wgpu::TextureUsage::None) - auto it = mTextureUsages.emplace(texture, texture); + auto it = mTextureUsages.emplace( + std::piecewise_construct, std::forward_as_tuple(texture), + std::forward_as_tuple(texture->GetFormat().aspects, texture->GetArrayLayers(), + texture->GetNumMipLevels(), wgpu::TextureUsage::None)); PassTextureUsage& textureUsage = it.first->second; - textureUsage.usage |= usage; - textureUsage.subresourceUsages.Update( - range, [usage](const SubresourceRange&, wgpu::TextureUsage* storedUsage) { - *storedUsage |= usage; - }); + textureUsage.Update(range, + [usage](const SubresourceRange&, wgpu::TextureUsage* storedUsage) { + *storedUsage |= usage; + }); } void PassResourceUsageTracker::AddTextureUsage(TextureBase* texture, const PassTextureUsage& textureUsage) { // Get or create a new PassTextureUsage for that texture (initially filled with // wgpu::TextureUsage::None) - auto it = mTextureUsages.emplace(texture, texture); + auto it = mTextureUsages.emplace( + std::piecewise_construct, std::forward_as_tuple(texture), + std::forward_as_tuple(texture->GetFormat().aspects, texture->GetArrayLayers(), + texture->GetNumMipLevels(), wgpu::TextureUsage::None)); PassTextureUsage* passTextureUsage = &it.first->second; - passTextureUsage->usage |= textureUsage.usage; - - passTextureUsage->subresourceUsages.Merge( - textureUsage.subresourceUsages, - [](const SubresourceRange&, wgpu::TextureUsage* storedUsage, - const wgpu::TextureUsage& addedUsage) { *storedUsage |= addedUsage; }); + passTextureUsage->Merge( + textureUsage, [](const SubresourceRange&, wgpu::TextureUsage* storedUsage, + const wgpu::TextureUsage& addedUsage) { *storedUsage |= addedUsage; }); } // Returns the per-pass usage for use by backends for APIs with explicit barriers. diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp index 2ccf9569f8..861d674654 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp @@ -599,18 +599,21 @@ namespace dawn_native { namespace d3d12 { for (size_t i = 0; i < usages.textures.size(); ++i) { Texture* texture = ToBackend(usages.textures[i]); - // Clear textures that are not output attachments. Output attachments will be - // cleared during record render pass if the texture subresource has not been - // initialized before the render pass. - if (!(usages.textureUsages[i].usage & wgpu::TextureUsage::RenderAttachment)) { - texture->EnsureSubresourceContentInitialized(commandContext, - texture->GetAllSubresources()); - } + + // Clear subresources that are not render attachments. Render attachments will be + // cleared in RecordBeginRenderPass by setting the loadop to clear when the texture + // subresource has not been initialized before the render pass. + usages.textureUsages[i].Iterate( + [&](const SubresourceRange& range, wgpu::TextureUsage usage) { + if (usage & ~wgpu::TextureUsage::RenderAttachment) { + texture->EnsureSubresourceContentInitialized(commandContext, range); + } + textureUsages |= usage; + }); ToBackend(usages.textures[i]) ->TrackUsageAndGetResourceBarrierForPass(commandContext, &barriers, usages.textureUsages[i]); - textureUsages |= usages.textureUsages[i].usage; } if (barriers.size()) { diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index c4b61ea019..e6730c29ed 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -764,8 +764,8 @@ namespace dawn_native { namespace d3d12 { ASSERT(GetDimension() == wgpu::TextureDimension::e2D); mSubresourceStateAndDecay.Merge( - textureUsages.subresourceUsages, [&](const SubresourceRange& mergeRange, - StateAndDecay* state, wgpu::TextureUsage usage) { + textureUsages, [&](const SubresourceRange& mergeRange, StateAndDecay* state, + wgpu::TextureUsage usage) { // Skip if this subresource is not used during the current pass if (usage == wgpu::TextureUsage::None) { return; diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index aaf250e4e8..cae5ffec86 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -545,12 +545,16 @@ namespace dawn_native { namespace metal { CommandRecordingContext* commandContext) { for (size_t i = 0; i < usages.textures.size(); ++i) { Texture* texture = ToBackend(usages.textures[i]); - // Clear textures that are not output attachments. Output attachments will be - // cleared in CreateMTLRenderPassDescriptor by setting the loadop to clear when the - // texture subresource has not been initialized before the render pass. - if (!(usages.textureUsages[i].usage & wgpu::TextureUsage::RenderAttachment)) { - texture->EnsureSubresourceContentInitialized(texture->GetAllSubresources()); - } + + // Clear subresources that are not render attachments. Render attachments will be + // cleared in RecordBeginRenderPass by setting the loadop to clear when the texture + // subresource has not been initialized before the render pass. + usages.textureUsages[i].Iterate( + [&](const SubresourceRange& range, wgpu::TextureUsage usage) { + if (usage & ~wgpu::TextureUsage::RenderAttachment) { + texture->EnsureSubresourceContentInitialized(range); + } + }); } for (BufferBase* bufferBase : usages.buffers) { ToBackend(bufferBase)->EnsureDataInitialized(commandContext); diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp index 239b10813d..ee42593170 100644 --- a/src/dawn_native/opengl/CommandBufferGL.cpp +++ b/src/dawn_native/opengl/CommandBufferGL.cpp @@ -452,12 +452,16 @@ namespace dawn_native { namespace opengl { auto TransitionForPass = [](const PassResourceUsage& usages) { for (size_t i = 0; i < usages.textures.size(); i++) { Texture* texture = ToBackend(usages.textures[i]); - // Clear textures that are not output attachments. Output attachments will be - // cleared in BeginRenderPass by setting the loadop to clear when the - // texture subresource has not been initialized before the render pass. - if (!(usages.textureUsages[i].usage & wgpu::TextureUsage::RenderAttachment)) { - texture->EnsureSubresourceContentInitialized(texture->GetAllSubresources()); - } + + // Clear subresources that are not render attachments. Render attachments will be + // cleared in RecordBeginRenderPass by setting the loadop to clear when the texture + // subresource has not been initialized before the render pass. + usages.textureUsages[i].Iterate( + [&](const SubresourceRange& range, wgpu::TextureUsage usage) { + if (usage & ~wgpu::TextureUsage::RenderAttachment) { + texture->EnsureSubresourceContentInitialized(range); + } + }); } for (BufferBase* bufferBase : usages.buffers) { diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index 44ed9c65e9..47adf797c0 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/src/dawn_native/vulkan/CommandBufferVk.cpp @@ -480,13 +480,16 @@ namespace dawn_native { namespace vulkan { for (size_t i = 0; i < usages.textures.size(); ++i) { Texture* texture = ToBackend(usages.textures[i]); - // Clear textures that are not output attachments. Output attachments will be - // cleared in RecordBeginRenderPass by setting the loadop to clear when the - // texture subresource has not been initialized before the render pass. - if (!(usages.textureUsages[i].usage & wgpu::TextureUsage::RenderAttachment)) { - texture->EnsureSubresourceContentInitialized(recordingContext, - texture->GetAllSubresources()); - } + + // Clear subresources that are not render attachments. Render attachments will be + // cleared in RecordBeginRenderPass by setting the loadop to clear when the texture + // subresource has not been initialized before the render pass. + usages.textureUsages[i].Iterate( + [&](const SubresourceRange& range, wgpu::TextureUsage usage) { + if (usage & ~wgpu::TextureUsage::RenderAttachment) { + texture->EnsureSubresourceContentInitialized(recordingContext, range); + } + }); texture->TransitionUsageForPass(recordingContext, usages.textureUsages[i], &imageBarriers, &srcStages, &dstStages); } diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index d14510448a..d8abfe9c1d 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/src/dawn_native/vulkan/TextureVk.cpp @@ -864,8 +864,7 @@ namespace dawn_native { namespace vulkan { if (ShouldCombineDepthStencilBarriers()) { SubresourceStorage combinedUsages( Aspect::CombinedDepthStencil, GetArrayLayers(), GetNumMipLevels()); - textureUsages.subresourceUsages.Iterate([&](const SubresourceRange& range, - wgpu::TextureUsage usage) { + textureUsages.Iterate([&](const SubresourceRange& range, wgpu::TextureUsage usage) { SubresourceRange updateRange = range; updateRange.aspects = Aspect::CombinedDepthStencil; @@ -878,8 +877,8 @@ namespace dawn_native { namespace vulkan { TransitionUsageForPassImpl(recordingContext, combinedUsages, imageBarriers, srcStages, dstStages); } else { - TransitionUsageForPassImpl(recordingContext, textureUsages.subresourceUsages, - imageBarriers, srcStages, dstStages); + TransitionUsageForPassImpl(recordingContext, textureUsages, imageBarriers, srcStages, + dstStages); } } diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp index 87a71a634c..90fc5fac73 100644 --- a/src/tests/end2end/TextureZeroInitTests.cpp +++ b/src/tests/end2end/TextureZeroInitTests.cpp @@ -889,6 +889,67 @@ TEST_P(TextureZeroInitTest, RenderPassSampledTextureClear) { EXPECT_EQ(true, dawn_native::IsTextureSubresourceInitialized(renderTexture.Get(), 0, 1, 0, 1)); } +// This is a regression test for a bug where a texture wouldn't get clear for a pass if at least +// one of its subresources was used as an attachment. It tests that if a texture is used as both +// sampled and attachment (with LoadOp::Clear so the lazy clear can be skipped) then the sampled +// subresource is correctly cleared. +TEST_P(TextureZeroInitTest, TextureBothSampledAndAttachmentClear) { + // Create a 2D array texture, layer 0 will be used as attachment, layer 1 as sampled. + wgpu::TextureDescriptor texDesc; + texDesc.usage = wgpu::TextureUsage::Sampled | wgpu::TextureUsage::RenderAttachment | + wgpu::TextureUsage::CopySrc; + texDesc.size = {1, 1, 2}; + texDesc.format = wgpu::TextureFormat::RGBA8Unorm; + wgpu::Texture texture = device.CreateTexture(&texDesc); + + wgpu::TextureViewDescriptor viewDesc; + viewDesc.dimension = wgpu::TextureViewDimension::e2D; + viewDesc.arrayLayerCount = 1; + + viewDesc.baseArrayLayer = 0; + wgpu::TextureView attachmentView = texture.CreateView(&viewDesc); + + viewDesc.baseArrayLayer = 1; + wgpu::TextureView sampleView = texture.CreateView(&viewDesc); + + // Create render pipeline + utils::ComboRenderPipelineDescriptor renderPipelineDescriptor(device); + renderPipelineDescriptor.cColorStates[0].format = wgpu::TextureFormat::RGBA8Unorm; + renderPipelineDescriptor.vertexStage.module = CreateBasicVertexShaderForTest(); + renderPipelineDescriptor.cFragmentStage.module = CreateSampledTextureFragmentShaderForTest(); + wgpu::RenderPipeline renderPipeline = device.CreateRenderPipeline(&renderPipelineDescriptor); + + // Create bindgroup + wgpu::SamplerDescriptor samplerDesc = utils::GetDefaultSamplerDescriptor(); + wgpu::Sampler sampler = device.CreateSampler(&samplerDesc); + + wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, renderPipeline.GetBindGroupLayout(0), + {{0, sampler}, {1, sampleView}}); + + // Encode pass and submit + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + utils::ComboRenderPassDescriptor renderPassDesc({attachmentView}); + renderPassDesc.cColorAttachments[0].clearColor = {1.0, 1.0, 1.0, 1.0}; + renderPassDesc.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear; + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc); + pass.SetPipeline(renderPipeline); + pass.SetBindGroup(0, bindGroup); + pass.Draw(6); + pass.EndPass(); + wgpu::CommandBuffer commands = encoder.Finish(); + + // Expect the lazy clear for the sampled subresource. + EXPECT_LAZY_CLEAR(1u, queue.Submit(1, &commands)); + + // Expect both subresources to be zero: the sampled one with lazy-clearing and the attachment + // because it sampled the lazy-cleared sampled subresource. + EXPECT_TEXTURE_RGBA8_EQ(&RGBA8::kZero, texture, 0, 0, 1, 1, 0, 0); + EXPECT_TEXTURE_RGBA8_EQ(&RGBA8::kZero, texture, 0, 0, 1, 1, 0, 1); + + // The whole texture is now initialized. + EXPECT_EQ(true, dawn_native::IsTextureSubresourceInitialized(texture.Get(), 0, 1, 0, 2)); +} + // This tests the clearing of sampled textures during compute pass TEST_P(TextureZeroInitTest, ComputePassSampledTextureClear) { // Create needed resources