Do not track global texture usage in PassResourceUsage.
The global resource usage was used for two things:
- Validating that the texture had the required usage, but this was
removed in a previous commit in favor of checking the texture's usage at
the encoding entrypoint.
- Skipping laz-clearing of the texture if it was used as
RenderAttachment. This was incorrect and would skip clearing of all of
the texture's subresource as long as one of them was used as
RenderAttachment.
This commit make PassTextureUsage exactly a
SubresourceStorage<TextureUsage> and fixes the logic for skipping
the clearing or RenderAttachment. It also adds a regression test for the
lazy clearing fix.
Bug: dawn:635
Change-Id: I5d984febb3e5a5f9ae15b632cac68e294555c4e6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38382
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn
index a0b7778..b98708c 100644
--- a/src/dawn_native/BUILD.gn
+++ b/src/dawn_native/BUILD.gn
@@ -234,7 +234,6 @@
"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 9a80bf9..d65ead6 100644
--- a/src/dawn_native/CMakeLists.txt
+++ b/src/dawn_native/CMakeLists.txt
@@ -107,7 +107,6 @@
"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 c576517..e0093dd 100644
--- a/src/dawn_native/CommandValidation.cpp
+++ b/src/dawn_native/CommandValidation.cpp
@@ -332,15 +332,14 @@
// 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 e56833a..0000000
--- 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 9132f65..f0aa43a 100644
--- a/src/dawn_native/PassResourceUsage.h
+++ b/src/dawn_native/PassResourceUsage.h
@@ -29,20 +29,8 @@
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<wgpu::TextureUsage> subresourceUsages;
- };
+ // The texture usage inside passes must be tracked per-subresource.
+ using PassTextureUsage = SubresourceStorage<wgpu::TextureUsage>;
// 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 4a53ae7..5816280 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 <utility>
+
namespace dawn_native {
PassResourceUsageTracker::PassResourceUsageTracker(PassType passType) : mPassType(passType) {
}
@@ -36,29 +38,31 @@
// 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 2ccf956..861d674 100644
--- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp
+++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
@@ -599,18 +599,21 @@
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 c4b61ea..e6730c2 100644
--- a/src/dawn_native/d3d12/TextureD3D12.cpp
+++ b/src/dawn_native/d3d12/TextureD3D12.cpp
@@ -764,8 +764,8 @@
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 aaf250e..cae5ffe 100644
--- a/src/dawn_native/metal/CommandBufferMTL.mm
+++ b/src/dawn_native/metal/CommandBufferMTL.mm
@@ -545,12 +545,16 @@
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 239b108..ee42593 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -452,12 +452,16 @@
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 44ed9c6..47adf79 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn_native/vulkan/CommandBufferVk.cpp
@@ -480,13 +480,16 @@
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 d145104..d8abfe9 100644
--- a/src/dawn_native/vulkan/TextureVk.cpp
+++ b/src/dawn_native/vulkan/TextureVk.cpp
@@ -864,8 +864,7 @@
if (ShouldCombineDepthStencilBarriers()) {
SubresourceStorage<wgpu::TextureUsage> 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 @@
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 87a71a6..90fc5fa 100644
--- a/src/tests/end2end/TextureZeroInitTests.cpp
+++ b/src/tests/end2end/TextureZeroInitTests.cpp
@@ -889,6 +889,67 @@
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
