From 5f8a8aadb96b5ac0302836de219943178f240c20 Mon Sep 17 00:00:00 2001 From: Natasha Lee Date: Wed, 14 Aug 2019 23:11:42 +0000 Subject: [PATCH] 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 Reviewed-by: Kai Ninomiya --- src/dawn_native/Texture.h | 2 +- src/dawn_native/vulkan/CommandBufferVk.cpp | 54 +-------- src/dawn_native/vulkan/TextureVk.cpp | 108 +++++++++++------- src/dawn_native/vulkan/TextureVk.h | 3 +- src/dawn_native/vulkan/UtilsVulkan.cpp | 54 +++++++++ src/dawn_native/vulkan/UtilsVulkan.h | 6 + .../end2end/NonzeroTextureCreationTests.cpp | 35 ++++++ src/tests/end2end/TextureZeroInitTests.cpp | 30 +++++ 8 files changed, 194 insertions(+), 98 deletions(-) diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index 066a7d739c..e5c59ee362 100644 --- a/src/dawn_native/Texture.h +++ b/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); diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index 7b1c7fea9d..bc25ea240b 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/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) { diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index ad21992ede..fc2e03151d 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/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 = {}; - range.aspectMask = GetVkAspectMask(); - 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); - } + ClearTexture(ToBackend(GetDevice())->GetPendingRecordingContext(), 0, GetNumMipLevels(), + 0, GetArrayLayers(), TextureBase::ClearValue::NonZero); } } @@ -679,36 +656,80 @@ 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]; - clear_color[0].depth = 0.0f; - clear_color[0].stencil = 0u; + VkClearDepthStencilValue clearDepthStencilValue[1]; + clearDepthStencilValue[0].depth = clearColor; + clearDepthStencilValue[0].stencil = clearColor; ToBackend(GetDevice()) ->fn.CmdClearDepthStencilImage(recordingContext->commandBuffer, GetHandle(), - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, clear_color, 1, - &range); - } else { - VkClearColorValue clear_color[1]; - 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; + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + clearDepthStencilValue, 1, &range); + } else if (GetFormat().isRenderable) { + VkClearColorValue clearColorValue = {{clearColor, clearColor, clearColor, clearColor}}; 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(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 srcBuffer = + std::make_unique(device, &descriptor); + uint8_t* clearBuffer = nullptr; + device->ConsumedError(srcBuffer->MapAtCreation(&clearBuffer)); + std::fill(reinterpret_cast(clearBuffer), + reinterpret_cast(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, + ®ion); + } + + if (clearValue == TextureBase::ClearValue::Zero) { + SetIsSubresourceContentInitialized(baseMipLevel, levelCount, baseArrayLayer, + layerCount); + GetDevice()->IncrementLazyClearCountForTesting(); } - SetIsSubresourceContentInitialized(baseMipLevel, levelCount, baseArrayLayer, layerCount); - GetDevice()->IncrementLazyClearCountForTesting(); } void Texture::EnsureSubresourceContentInitialized(CommandRecordingContext* recordingContext, @@ -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); } } diff --git a/src/dawn_native/vulkan/TextureVk.h b/src/dawn_native/vulkan/TextureVk.h index 52236d3378..75f7a541da 100644 --- a/src/dawn_native/vulkan/TextureVk.h +++ b/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; diff --git a/src/dawn_native/vulkan/UtilsVulkan.cpp b/src/dawn_native/vulkan/UtilsVulkan.cpp index 723a6bb9c8..dd81f3fb5c 100644 --- a/src/dawn_native/vulkan/UtilsVulkan.cpp +++ b/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 diff --git a/src/dawn_native/vulkan/UtilsVulkan.h b/src/dawn_native/vulkan/UtilsVulkan.h index 80fa2da047..8f4b5acee9 100644 --- a/src/dawn_native/vulkan/UtilsVulkan.h +++ b/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_ diff --git a/src/tests/end2end/NonzeroTextureCreationTests.cpp b/src/tests/end2end/NonzeroTextureCreationTests.cpp index 00f26c3c32..e7fe33b648 100644 --- a/src/tests/end2end/NonzeroTextureCreationTests.cpp +++ b/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 data(bufferSize, 100); + dawn::Buffer bufferDst = utils::CreateBufferFromData( + device, data.data(), static_cast(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, ©Size); + dawn::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + std::vector 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"}, diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp index bef10dc7b8..9f10de5c30 100644 --- a/src/tests/end2end/TextureZeroInitTests.cpp +++ b/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 data(bufferSize, 100); + dawn::Buffer bufferDst = utils::CreateBufferFromData( + device, data.data(), static_cast(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, ©Size); + dawn::CommandBuffer commands = encoder.Finish(); + EXPECT_LAZY_CLEAR(1u, queue.Submit(1, &commands)); + + std::vector 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"}),