From 90ad1a34ddf5ff680fe11c5b141f0f686fb090e0 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 11 Jun 2020 11:23:35 +0000 Subject: [PATCH] Add CopyB2B tests including for 0-sized copies This catches a few driver bugs or validation errors when doing empty copies. Bug: dawn:446 Change-Id: Ibef50b0c03b2fa24bc00cf5c7b6d26b8ac6fae8a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19602 Reviewed-by: Austin Eng Reviewed-by: Stephen White Commit-Queue: Corentin Wallez --- src/dawn_native/CommandEncoder.cpp | 17 +++-- src/dawn_native/vulkan/CommandBufferVk.cpp | 1 + src/tests/end2end/CopyTests.cpp | 77 +++++++++++++++++++++- 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index c1201d2bde..5159d9c38b 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -638,13 +638,16 @@ namespace dawn_native { mTopLevelBuffers.insert(destination); } - CopyBufferToBufferCmd* copy = - allocator->Allocate(Command::CopyBufferToBuffer); - copy->source = source; - copy->sourceOffset = sourceOffset; - copy->destination = destination; - copy->destinationOffset = destinationOffset; - copy->size = size; + // Skip noop copies. Some backends validation rules disallow them. + if (size != 0) { + CopyBufferToBufferCmd* copy = + allocator->Allocate(Command::CopyBufferToBuffer); + copy->source = source; + copy->sourceOffset = sourceOffset; + copy->destination = destination; + copy->destinationOffset = destinationOffset; + copy->size = size; + } return {}; }); diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index b306660468..6f55a01c95 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/src/dawn_native/vulkan/CommandBufferVk.cpp @@ -420,6 +420,7 @@ namespace dawn_native { namespace vulkan { switch (type) { case Command::CopyBufferToBuffer: { CopyBufferToBufferCmd* copy = mCommands.NextCommand(); + Buffer* srcBuffer = ToBackend(copy->source.Get()); Buffer* dstBuffer = ToBackend(copy->destination.Get()); diff --git a/src/tests/end2end/CopyTests.cpp b/src/tests/end2end/CopyTests.cpp index 0f9207601d..498157d4d9 100644 --- a/src/tests/end2end/CopyTests.cpp +++ b/src/tests/end2end/CopyTests.cpp @@ -388,6 +388,49 @@ class CopyTests_T2T : public CopyTests { } }; +class CopyTests_B2B : public DawnTest { + protected: + // This is the same signature as CopyBufferToBuffer except that the buffers are replaced by + // only their size. + void DoTest(uint64_t sourceSize, + uint64_t sourceOffset, + uint64_t destinationSize, + uint64_t destinationOffset, + uint64_t copySize) { + ASSERT(sourceSize % 4 == 0); + ASSERT(destinationSize % 4 == 0); + + // Create our two test buffers, destination filled with zeros, source filled with non-zeroes + std::vector zeroes(static_cast(destinationSize / sizeof(uint32_t))); + wgpu::Buffer destination = + utils::CreateBufferFromData(device, zeroes.data(), zeroes.size() * sizeof(uint32_t), + wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::CopySrc); + + std::vector sourceData(static_cast(sourceSize / sizeof(uint32_t))); + for (size_t i = 0; i < sourceData.size(); i++) { + sourceData[i] = i + 1; + } + wgpu::Buffer source = utils::CreateBufferFromData(device, sourceData.data(), + sourceData.size() * sizeof(uint32_t), + wgpu::BufferUsage::CopySrc); + + // Submit the copy + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.CopyBufferToBuffer(source, sourceOffset, destination, destinationOffset, copySize); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + // Check destination is exactly the expected content. + EXPECT_BUFFER_U32_RANGE_EQ(zeroes.data(), destination, 0, + destinationOffset / sizeof(uint32_t)); + EXPECT_BUFFER_U32_RANGE_EQ(sourceData.data() + sourceOffset / sizeof(uint32_t), destination, + destinationOffset, copySize / sizeof(uint32_t)); + uint64_t copyEnd = destinationOffset + copySize; + EXPECT_BUFFER_U32_RANGE_EQ(zeroes.data(), destination, copyEnd, + (destinationSize - copyEnd) / sizeof(uint32_t)); + } +}; + // Test that copying an entire texture with 256-byte aligned dimensions works TEST_P(CopyTests_T2B, FullTextureAligned) { constexpr uint32_t kWidth = 256; @@ -823,6 +866,36 @@ TEST_P(CopyTests_T2T, MultipleMipSrcSingleMipDst) { } } -// TODO(brandon1.jones@intel.com) Add test for ensuring blitCommandEncoder on Metal. - DAWN_INSTANTIATE_TEST(CopyTests_T2T, D3D12Backend(), MetalBackend(), OpenGLBackend(), VulkanBackend()); + +static constexpr uint64_t kSmallBufferSize = 4; +static constexpr uint64_t kLargeBufferSize = 1 << 16; + +// Test copying full buffers +TEST_P(CopyTests_B2B, FullCopy) { + DoTest(kSmallBufferSize, 0, kSmallBufferSize, 0, kSmallBufferSize); + DoTest(kLargeBufferSize, 0, kLargeBufferSize, 0, kLargeBufferSize); +} + +// Test copying small pieces of a buffer at different corner case offsets +TEST_P(CopyTests_B2B, SmallCopyInBigBuffer) { + constexpr uint64_t kEndOffset = kLargeBufferSize - kSmallBufferSize; + DoTest(kLargeBufferSize, 0, kLargeBufferSize, 0, kSmallBufferSize); + DoTest(kLargeBufferSize, kEndOffset, kLargeBufferSize, 0, kSmallBufferSize); + DoTest(kLargeBufferSize, 0, kLargeBufferSize, kEndOffset, kSmallBufferSize); + DoTest(kLargeBufferSize, kEndOffset, kLargeBufferSize, kEndOffset, kSmallBufferSize); +} + +// Test zero-size copies +TEST_P(CopyTests_B2B, ZeroSizedCopy) { + DoTest(kLargeBufferSize, 0, kLargeBufferSize, 0, 0); + DoTest(kLargeBufferSize, 0, kLargeBufferSize, kLargeBufferSize, 0); + DoTest(kLargeBufferSize, kLargeBufferSize, kLargeBufferSize, 0, 0); + DoTest(kLargeBufferSize, kLargeBufferSize, kLargeBufferSize, kLargeBufferSize, 0); +} + +DAWN_INSTANTIATE_TEST(CopyTests_B2B, + D3D12Backend(), + MetalBackend(), + OpenGLBackend(), + VulkanBackend());