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>

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",

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"

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 {};

src/dawn_native/PassResourceUsage.cpp

@@ -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

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<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

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 @@ 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.

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()) {

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;

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);

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) {

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);
             }

src/dawn_native/vulkan/TextureVk.cpp

@@ -864,8 +864,7 @@ namespace dawn_native { namespace vulkan {
         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 @@ 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);
         }
     }
 

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