Vulkan: clear nonrenderable texture color formats

Clears nonrenderable color formats and adds a clearValue enum to help
share the code path between zero and nonzero clears.

Bug: dawn:145
Change-Id: I285521cae0ee71625602b949888b21481a05fb8e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10021
Commit-Queue: Natasha Lee <natlee@microsoft.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

src/dawn_native/Texture.h

@@ -43,7 +43,7 @@ namespace dawn_native {
     class TextureBase : public ObjectBase {
       public:
         enum class TextureState { OwnedInternal, OwnedExternal, Destroyed };
-
+        enum class ClearValue { Zero, NonZero };
         TextureBase(DeviceBase* device, const TextureDescriptor* descriptor, TextureState state);
 
         static TextureBase* MakeError(DeviceBase* device);

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -27,6 +27,7 @@
 #include "dawn_native/vulkan/RenderPassCache.h"
 #include "dawn_native/vulkan/RenderPipelineVk.h"
 #include "dawn_native/vulkan/TextureVk.h"
+#include "dawn_native/vulkan/UtilsVulkan.h"
 
 namespace dawn_native { namespace vulkan {
 
@@ -43,59 +44,6 @@ namespace dawn_native { namespace vulkan {
             }
         }
 
-        // Vulkan SPEC requires the source/destination region specified by each element of
-        // pRegions must be a region that is contained within srcImage/dstImage. Here the size of
-        // the image refers to the virtual size, while Dawn validates texture copy extent with the
-        // physical size, so we need to re-calculate the texture copy extent to ensure it should fit
-        // in the virtual size of the subresource.
-        Extent3D ComputeTextureCopyExtent(const TextureCopy& textureCopy,
-                                          const Extent3D& copySize) {
-            Extent3D validTextureCopyExtent = copySize;
-            const TextureBase* texture = textureCopy.texture.Get();
-            Extent3D virtualSizeAtLevel = texture->GetMipLevelVirtualSize(textureCopy.mipLevel);
-            if (textureCopy.origin.x + copySize.width > virtualSizeAtLevel.width) {
-                ASSERT(texture->GetFormat().isCompressed);
-                validTextureCopyExtent.width = virtualSizeAtLevel.width - textureCopy.origin.x;
-            }
-            if (textureCopy.origin.y + copySize.height > virtualSizeAtLevel.height) {
-                ASSERT(texture->GetFormat().isCompressed);
-                validTextureCopyExtent.height = virtualSizeAtLevel.height - textureCopy.origin.y;
-            }
-
-            return validTextureCopyExtent;
-        }
-
-        VkBufferImageCopy ComputeBufferImageCopyRegion(const BufferCopy& bufferCopy,
-                                                       const TextureCopy& textureCopy,
-                                                       const Extent3D& copySize) {
-            const Texture* texture = ToBackend(textureCopy.texture.Get());
-
-            VkBufferImageCopy region;
-
-            region.bufferOffset = bufferCopy.offset;
-            // In Vulkan the row length is in texels while it is in bytes for Dawn
-            const Format& format = texture->GetFormat();
-            ASSERT(bufferCopy.rowPitch % format.blockByteSize == 0);
-            region.bufferRowLength = bufferCopy.rowPitch / format.blockByteSize * format.blockWidth;
-            region.bufferImageHeight = bufferCopy.imageHeight;
-
-            region.imageSubresource.aspectMask = texture->GetVkAspectMask();
-            region.imageSubresource.mipLevel = textureCopy.mipLevel;
-            region.imageSubresource.baseArrayLayer = textureCopy.arrayLayer;
-            region.imageSubresource.layerCount = 1;
-
-            region.imageOffset.x = textureCopy.origin.x;
-            region.imageOffset.y = textureCopy.origin.y;
-            region.imageOffset.z = textureCopy.origin.z;
-
-            Extent3D imageExtent = ComputeTextureCopyExtent(textureCopy, copySize);
-            region.imageExtent.width = imageExtent.width;
-            region.imageExtent.height = imageExtent.height;
-            region.imageExtent.depth = copySize.depth;
-
-            return region;
-        }
-
         VkImageCopy ComputeImageCopyRegion(const TextureCopy& srcCopy,
                                            const TextureCopy& dstCopy,
                                            const Extent3D& copySize) {

src/dawn_native/vulkan/TextureVk.cpp

@@ -16,9 +16,11 @@
 
 #include "dawn_native/VulkanBackend.h"
 #include "dawn_native/vulkan/AdapterVk.h"
+#include "dawn_native/vulkan/BufferVk.h"
 #include "dawn_native/vulkan/CommandRecordingContext.h"
 #include "dawn_native/vulkan/DeviceVk.h"
 #include "dawn_native/vulkan/FencedDeleter.h"
+#include "dawn_native/vulkan/UtilsVulkan.h"
 
 namespace dawn_native { namespace vulkan {
 
@@ -453,33 +455,8 @@ namespace dawn_native { namespace vulkan {
             ASSERT(false);
         }
         if (device->IsToggleEnabled(Toggle::NonzeroClearResourcesOnCreationForTesting)) {
-            VkImageSubresourceRange range = {};
+            ClearTexture(ToBackend(GetDevice())->GetPendingRecordingContext(), 0, GetNumMipLevels(),
-            range.aspectMask = GetVkAspectMask();
+                         0, GetArrayLayers(), TextureBase::ClearValue::NonZero);
-            range.baseMipLevel = 0;
-            range.levelCount = GetNumMipLevels();
-            range.baseArrayLayer = 0;
-            range.layerCount = GetArrayLayers();
-            TransitionUsageNow(ToBackend(GetDevice())->GetPendingRecordingContext(),
-                               dawn::TextureUsageBit::CopyDst);
-
-            if (GetFormat().HasDepthOrStencil()) {
-                VkClearDepthStencilValue clear_color[1];
-                clear_color[0].depth = 1.0f;
-                clear_color[0].stencil = 1u;
-                ToBackend(GetDevice())
-                    ->fn.CmdClearDepthStencilImage(
-                        ToBackend(GetDevice())->GetPendingCommandBuffer(), GetHandle(),
-                        VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, clear_color, 1, &range);
-            } else {
-                // TODO(natlee@microsoft.com): use correct union member depending on the texture
-                // format
-                VkClearColorValue clear_color = {{1.0, 1.0, 1.0, 1.0}};
-
-                ToBackend(GetDevice())
-                    ->fn.CmdClearColorImage(ToBackend(GetDevice())->GetPendingCommandBuffer(),
-                                            GetHandle(), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
-                                            &clear_color, 1, &range);
-            }
         }
     }
 
@@ -679,37 +656,81 @@ namespace dawn_native { namespace vulkan {
                                uint32_t baseMipLevel,
                                uint32_t levelCount,
                                uint32_t baseArrayLayer,
-                               uint32_t layerCount) {
+                               uint32_t layerCount,
+                               TextureBase::ClearValue clearValue) {
         VkImageSubresourceRange range = {};
         range.aspectMask = GetVkAspectMask();
         range.baseMipLevel = baseMipLevel;
         range.levelCount = levelCount;
         range.baseArrayLayer = baseArrayLayer;
         range.layerCount = layerCount;
+        uint8_t clearColor = (clearValue == TextureBase::ClearValue::Zero) ? 0 : 1;
 
         TransitionUsageNow(recordingContext, dawn::TextureUsageBit::CopyDst);
         if (GetFormat().HasDepthOrStencil()) {
-            VkClearDepthStencilValue clear_color[1];
+            VkClearDepthStencilValue clearDepthStencilValue[1];
-            clear_color[0].depth = 0.0f;
+            clearDepthStencilValue[0].depth = clearColor;
-            clear_color[0].stencil = 0u;
+            clearDepthStencilValue[0].stencil = clearColor;
             ToBackend(GetDevice())
                 ->fn.CmdClearDepthStencilImage(recordingContext->commandBuffer, GetHandle(),
-                                               VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, clear_color, 1,
+                                               VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
-                                               &range);
+                                               clearDepthStencilValue, 1, &range);
-        } else {
+        } else if (GetFormat().isRenderable) {
-            VkClearColorValue clear_color[1];
+            VkClearColorValue clearColorValue = {{clearColor, clearColor, clearColor, clearColor}};
-            clear_color[0].float32[0] = 0.0f;
-            clear_color[0].float32[1] = 0.0f;
-            clear_color[0].float32[2] = 0.0f;
-            clear_color[0].float32[3] = 0.0f;
             ToBackend(GetDevice())
                 ->fn.CmdClearColorImage(recordingContext->commandBuffer, GetHandle(),
-                                        VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, clear_color, 1,
+                                        VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, &clearColorValue, 1,
                                         &range);
+        } else {
+            // TODO(natlee@microsoft.com): test compressed textures are cleared
+            // create temp buffer with clear color to copy to the texture image
+            dawn_native::vulkan::Device* device =
+                reinterpret_cast<dawn_native::vulkan::Device*>(GetDevice());
+            dawn_native::BufferDescriptor descriptor;
+            descriptor.size = (GetSize().width / GetFormat().blockWidth) *
+                              (GetSize().height / GetFormat().blockHeight) *
+                              GetFormat().blockByteSize;
+            descriptor.nextInChain = nullptr;
+            descriptor.usage = dawn::BufferUsageBit::CopySrc | dawn::BufferUsageBit::MapWrite;
+            std::unique_ptr<Buffer> srcBuffer =
+                std::make_unique<dawn_native::vulkan::Buffer>(device, &descriptor);
+            uint8_t* clearBuffer = nullptr;
+            device->ConsumedError(srcBuffer->MapAtCreation(&clearBuffer));
+            std::fill(reinterpret_cast<uint32_t*>(clearBuffer),
+                      reinterpret_cast<uint32_t*>(clearBuffer + descriptor.size), clearColor);
+
+            // compute the buffer image copy to set the clear region of entire texture
+            dawn_native::BufferCopy bufferCopy;
+            bufferCopy.buffer = srcBuffer.get();
+            bufferCopy.imageHeight = 0;
+            bufferCopy.offset = 0;
+            bufferCopy.rowPitch = 0;
+
+            dawn_native::TextureCopy textureCopy;
+            textureCopy.texture = this;
+            textureCopy.origin = {0, 0, 0};
+            textureCopy.mipLevel = baseMipLevel;
+            textureCopy.arrayLayer = baseArrayLayer;
+
+            Extent3D copySize = {GetSize().width, GetSize().height, 1};
+
+            VkBufferImageCopy region =
+                ComputeBufferImageCopyRegion(bufferCopy, textureCopy, copySize);
+
+            // copy the clear buffer to the texture image
+            srcBuffer->TransitionUsageNow(recordingContext, dawn::BufferUsageBit::CopySrc);
+            ToBackend(GetDevice())
+                ->fn.CmdCopyBufferToImage(recordingContext->commandBuffer, srcBuffer->GetHandle(),
+                                          GetHandle(), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1,
+                                          &region);
         }
-        SetIsSubresourceContentInitialized(baseMipLevel, levelCount, baseArrayLayer, layerCount);
+
+        if (clearValue == TextureBase::ClearValue::Zero) {
+            SetIsSubresourceContentInitialized(baseMipLevel, levelCount, baseArrayLayer,
+                                               layerCount);
             GetDevice()->IncrementLazyClearCountForTesting();
         }
+    }
 
     void Texture::EnsureSubresourceContentInitialized(CommandRecordingContext* recordingContext,
                                                       uint32_t baseMipLevel,
@@ -729,7 +750,8 @@ namespace dawn_native { namespace vulkan {
 
             // If subresource has not been initialized, clear it to black as it could contain dirty
             // bits from recycled memory
-            ClearTexture(recordingContext, baseMipLevel, levelCount, baseArrayLayer, layerCount);
+            ClearTexture(recordingContext, baseMipLevel, levelCount, baseArrayLayer, layerCount,
+                         TextureBase::ClearValue::Zero);
         }
     }
 

src/dawn_native/vulkan/TextureVk.h

@@ -75,7 +75,8 @@ namespace dawn_native { namespace vulkan {
                           uint32_t baseMipLevel,
                           uint32_t levelCount,
                           uint32_t baseArrayLayer,
-                          uint32_t layerCount);
+                          uint32_t layerCount,
+                          TextureBase::ClearValue);
 
         VkImage mHandle = VK_NULL_HANDLE;
         DeviceMemoryAllocation mMemoryAllocation;

src/dawn_native/vulkan/UtilsVulkan.cpp

@@ -15,6 +15,9 @@
 #include "dawn_native/vulkan/UtilsVulkan.h"
 
 #include "common/Assert.h"
+#include "dawn_native/Format.h"
+#include "dawn_native/vulkan/Forward.h"
+#include "dawn_native/vulkan/TextureVk.h"
 
 namespace dawn_native { namespace vulkan {
 
@@ -41,4 +44,55 @@ namespace dawn_native { namespace vulkan {
         }
     }
 
+    // Vulkan SPEC requires the source/destination region specified by each element of
+    // pRegions must be a region that is contained within srcImage/dstImage. Here the size of
+    // the image refers to the virtual size, while Dawn validates texture copy extent with the
+    // physical size, so we need to re-calculate the texture copy extent to ensure it should fit
+    // in the virtual size of the subresource.
+    Extent3D ComputeTextureCopyExtent(const TextureCopy& textureCopy, const Extent3D& copySize) {
+        Extent3D validTextureCopyExtent = copySize;
+        const TextureBase* texture = textureCopy.texture.Get();
+        Extent3D virtualSizeAtLevel = texture->GetMipLevelVirtualSize(textureCopy.mipLevel);
+        if (textureCopy.origin.x + copySize.width > virtualSizeAtLevel.width) {
+            ASSERT(texture->GetFormat().isCompressed);
+            validTextureCopyExtent.width = virtualSizeAtLevel.width - textureCopy.origin.x;
+        }
+        if (textureCopy.origin.y + copySize.height > virtualSizeAtLevel.height) {
+            ASSERT(texture->GetFormat().isCompressed);
+            validTextureCopyExtent.height = virtualSizeAtLevel.height - textureCopy.origin.y;
+        }
+
+        return validTextureCopyExtent;
+    }
+
+    VkBufferImageCopy ComputeBufferImageCopyRegion(const BufferCopy& bufferCopy,
+                                                   const TextureCopy& textureCopy,
+                                                   const Extent3D& copySize) {
+        const Texture* texture = ToBackend(textureCopy.texture.Get());
+
+        VkBufferImageCopy region;
+
+        region.bufferOffset = bufferCopy.offset;
+        // In Vulkan the row length is in texels while it is in bytes for Dawn
+        const Format& format = texture->GetFormat();
+        ASSERT(bufferCopy.rowPitch % format.blockByteSize == 0);
+        region.bufferRowLength = bufferCopy.rowPitch / format.blockByteSize * format.blockWidth;
+        region.bufferImageHeight = bufferCopy.imageHeight;
+
+        region.imageSubresource.aspectMask = texture->GetVkAspectMask();
+        region.imageSubresource.mipLevel = textureCopy.mipLevel;
+        region.imageSubresource.baseArrayLayer = textureCopy.arrayLayer;
+        region.imageSubresource.layerCount = 1;
+
+        region.imageOffset.x = textureCopy.origin.x;
+        region.imageOffset.y = textureCopy.origin.y;
+        region.imageOffset.z = textureCopy.origin.z;
+
+        Extent3D imageExtent = ComputeTextureCopyExtent(textureCopy, copySize);
+        region.imageExtent.width = imageExtent.width;
+        region.imageExtent.height = imageExtent.height;
+        region.imageExtent.depth = copySize.depth;
+
+        return region;
+    }
 }}  // namespace dawn_native::vulkan

src/dawn_native/vulkan/UtilsVulkan.h

@@ -16,12 +16,18 @@
 #define DAWNNATIVE_VULKAN_UTILSVULKAN_H_
 
 #include "common/vulkan_platform.h"
+#include "dawn_native/Commands.h"
 #include "dawn_native/dawn_platform.h"
 
 namespace dawn_native { namespace vulkan {
 
     VkCompareOp ToVulkanCompareOp(dawn::CompareFunction op);
 
+    Extent3D ComputeTextureCopyExtent(const TextureCopy& textureCopy, const Extent3D& copySize);
+    VkBufferImageCopy ComputeBufferImageCopyRegion(const BufferCopy& bufferCopy,
+                                                   const TextureCopy& textureCopy,
+                                                   const Extent3D& copySize);
+
 }}  // namespace dawn_native::vulkan
 
 #endif  // DAWNNATIVE_VULKAN_UTILSVULKAN_H_

src/tests/end2end/NonzeroTextureCreationTests.cpp

@@ -94,6 +94,41 @@ TEST_P(NonzeroTextureCreationTests, ArrayLayerClears) {
     EXPECT_TEXTURE_RGBA8_EQ(expected.data(), texture, 0, 0, kSize, kSize, 0, 2);
 }
 
+// Test that nonrenderable texture formats clear to 1's because toggle is enabled
+TEST_P(NonzeroTextureCreationTests, NonrenderableTextureFormat) {
+    // skip test for other backends since they are not implemented yet
+    DAWN_SKIP_TEST_IF(IsOpenGL() || IsD3D12());
+    dawn::TextureDescriptor descriptor;
+    descriptor.dimension = dawn::TextureDimension::e2D;
+    descriptor.size.width = kSize;
+    descriptor.size.height = kSize;
+    descriptor.size.depth = 1;
+    descriptor.arrayLayerCount = 1;
+    descriptor.sampleCount = 1;
+    descriptor.format = dawn::TextureFormat::RGBA16Snorm;
+    descriptor.mipLevelCount = 1;
+    descriptor.usage = dawn::TextureUsageBit::CopySrc;
+    dawn::Texture texture = device.CreateTexture(&descriptor);
+
+    // Set buffer with dirty data so we know it is cleared by the lazy cleared texture copy
+    uint32_t bufferSize = 8 * kSize * kSize;
+    std::vector<uint8_t> data(bufferSize, 100);
+    dawn::Buffer bufferDst = utils::CreateBufferFromData(
+        device, data.data(), static_cast<uint32_t>(data.size()), dawn::BufferUsageBit::CopySrc);
+
+    dawn::BufferCopyView bufferCopyView = utils::CreateBufferCopyView(bufferDst, 0, 0, 0);
+    dawn::TextureCopyView textureCopyView = utils::CreateTextureCopyView(texture, 0, 0, {0, 0, 0});
+    dawn::Extent3D copySize = {kSize, kSize, 1};
+
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.CopyTextureToBuffer(&textureCopyView, &bufferCopyView, &copySize);
+    dawn::CommandBuffer commands = encoder.Finish();
+    queue.Submit(1, &commands);
+
+    std::vector<uint32_t> expected(bufferSize, 1);
+    EXPECT_BUFFER_U32_RANGE_EQ(expected.data(), bufferDst, 0, 8);
+}
+
 DAWN_INSTANTIATE_TEST(NonzeroTextureCreationTests,
                       ForceWorkarounds(D3D12Backend,
                                        {"nonzero_clear_resources_on_creation_for_testing"},

src/tests/end2end/TextureZeroInitTests.cpp

@@ -86,6 +86,8 @@ class TextureZeroInitTest : public DawnTest {
     constexpr static dawn::TextureFormat kColorFormat = dawn::TextureFormat::RGBA8Unorm;
     constexpr static dawn::TextureFormat kDepthStencilFormat =
         dawn::TextureFormat::Depth24PlusStencil8;
+    constexpr static dawn::TextureFormat kNonrenderableColorFormat =
+        dawn::TextureFormat::RGBA16Snorm;
 };
 
 // This tests that the code path of CopyTextureToBuffer clears correctly to Zero after first usage
@@ -563,6 +565,34 @@ TEST_P(TextureZeroInitTest, ComputePassSampledTextureClear) {
     EXPECT_BUFFER_U32_RANGE_EQ(expectedWithZeros.data(), bufferTex, 0, 4);
 }
 
+// This tests that the code path of CopyTextureToBuffer clears correctly for non-renderable textures
+TEST_P(TextureZeroInitTest, NonRenderableTextureClear) {
+    // skip test for other backends since they are not implemented yet
+    DAWN_SKIP_TEST_IF(IsOpenGL() || IsD3D12());
+
+    dawn::TextureDescriptor descriptor =
+        CreateTextureDescriptor(1, 1, dawn::TextureUsageBit::CopySrc, kNonrenderableColorFormat);
+    dawn::Texture texture = device.CreateTexture(&descriptor);
+
+    // Set buffer with dirty data so we know it is cleared by the lazy cleared texture copy
+    uint32_t bufferSize = 8 * kSize * kSize;
+    std::vector<uint8_t> data(bufferSize, 100);
+    dawn::Buffer bufferDst = utils::CreateBufferFromData(
+        device, data.data(), static_cast<uint32_t>(data.size()), dawn::BufferUsageBit::CopySrc);
+
+    dawn::BufferCopyView bufferCopyView = utils::CreateBufferCopyView(bufferDst, 0, 0, 0);
+    dawn::TextureCopyView textureCopyView = utils::CreateTextureCopyView(texture, 0, 0, {0, 0, 0});
+    dawn::Extent3D copySize = {kSize, kSize, 1};
+
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.CopyTextureToBuffer(&textureCopyView, &bufferCopyView, &copySize);
+    dawn::CommandBuffer commands = encoder.Finish();
+    EXPECT_LAZY_CLEAR(1u, queue.Submit(1, &commands));
+
+    std::vector<uint32_t> expectedWithZeros(bufferSize, 0);
+    EXPECT_BUFFER_U32_RANGE_EQ(expectedWithZeros.data(), bufferDst, 0, 8);
+}
+
 DAWN_INSTANTIATE_TEST(
     TextureZeroInitTest,
     ForceWorkarounds(D3D12Backend, {"nonzero_clear_resources_on_creation_for_testing"}),