Disallow the copies within the same buffer

This patch adds the validation that forbids the buffer-to-buffer copies
when the source and destination buffer are the same one as in D3D12 the
Source and Destination resource cannot be the same when doing a
CopyBufferRegion.

BUG=dawn:17, dawn:420
TEST=dawn_unittests

Change-Id: Ie3e0c5361919ff369240a65d6e7fbae05b8332b0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21780
Reviewed-by: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
This commit is contained in:
Jiawei Shao 2020-05-19 00:11:11 +00:00 committed by Commit Bot service account
parent 216c10d270
commit 2ae84e9461
3 changed files with 12 additions and 83 deletions

View File

@ -102,20 +102,6 @@ namespace dawn_native {
return {};
}
MaybeError ValidateB2BCopyWithinSameBuffer(uint64_t dataSize,
uint64_t srcOffset,
uint64_t dstOffset) {
uint64_t maxOffset = std::max(srcOffset, dstOffset);
uint64_t minOffset = std::min(srcOffset, dstOffset);
if (minOffset + dataSize > maxOffset) {
return DAWN_VALIDATION_ERROR(
"Copy regions cannot overlap when copy within the same buffer");
}
return {};
}
MaybeError ValidateTexelBufferOffset(const BufferCopyView& bufferCopy,
const Format& format) {
if (bufferCopy.offset % format.blockByteSize != 0) {
@ -624,15 +610,15 @@ namespace dawn_native {
DAWN_TRY(GetDevice()->ValidateObject(source));
DAWN_TRY(GetDevice()->ValidateObject(destination));
if (source == destination) {
return DAWN_VALIDATION_ERROR(
"Source and destination cannot be the same buffer.");
}
DAWN_TRY(ValidateCopySizeFitsInBuffer(source, sourceOffset, size));
DAWN_TRY(ValidateCopySizeFitsInBuffer(destination, destinationOffset, size));
DAWN_TRY(ValidateB2BCopyAlignment(size, sourceOffset, destinationOffset));
if (source == destination) {
DAWN_TRY(
ValidateB2BCopyWithinSameBuffer(size, sourceOffset, destinationOffset));
}
DAWN_TRY(ValidateCanUseAs(source, wgpu::BufferUsage::CopySrc));
DAWN_TRY(ValidateCanUseAs(destination, wgpu::BufferUsage::CopyDst));

View File

@ -71,8 +71,6 @@ class CopyTests : public DawnTest {
}
};
class CopyTests_B2B : public CopyTests {};
class CopyTests_T2B : public CopyTests {
protected:
@ -401,57 +399,6 @@ class CopyTests_T2T : public CopyTests {
}
};
// Test that copying within the same buffer works
TEST_P(CopyTests_B2B, CopyWithinSameBuffer) {
// Copy the first 2 uint32_t values to the 4th and 5th uint32_t values.
{
// Create a buffer with 6 uint32_t values.
wgpu::Buffer buffer = utils::CreateBufferFromData(
device, wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst,
{1u, 2u, 3u, 4u, 5u, 6u});
constexpr uint32_t kSrcOffset = 0u;
constexpr uint32_t kDstOffset = 3u * sizeof(uint32_t);
constexpr uint32_t kSize = 2u * sizeof(uint32_t);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.CopyBufferToBuffer(buffer, kSrcOffset, buffer, kDstOffset, kSize);
wgpu::CommandBuffer commands = encoder.Finish();
queue.Submit(1, &commands);
// Verify the first two uint32_t values are correctly copied to the locations where the 4th
// and 5th uint32_t values are stored.
std::array<uint32_t, 6> kExpected = {{1u, 2u, 3u, 1u, 2u, 6u}};
EXPECT_BUFFER_U32_RANGE_EQ(kExpected.data(), buffer, 0, kExpected.size());
}
// Copy the 4th and 5th uint32_t values to the first two uint32_t values.
{
// Create a buffer with 6 uint32_t values.
wgpu::Buffer buffer = utils::CreateBufferFromData(
device, wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst,
{1u, 2u, 3u, 4u, 5u, 6u});
constexpr uint32_t kSrcOffset = 3u * sizeof(uint32_t);
constexpr uint32_t kDstOffset = 0u;
constexpr uint32_t kSize = 2u * sizeof(uint32_t);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.CopyBufferToBuffer(buffer, kSrcOffset, buffer, kDstOffset, kSize);
wgpu::CommandBuffer commands = encoder.Finish();
queue.Submit(1, &commands);
// Verify the 4th and 5th uint32_t values are correctly copied to the locations where the
// first uint32_t values are stored.
std::array<uint32_t, 6> kExpected = {{4u, 5u, 3u, 4u, 5u, 6u}};
EXPECT_BUFFER_U32_RANGE_EQ(kExpected.data(), buffer, 0, kExpected.size());
}
}
DAWN_INSTANTIATE_TEST(CopyTests_B2B,
D3D12Backend(),
MetalBackend(),
OpenGLBackend(),
VulkanBackend());
// Test that copying an entire texture with 256-byte aligned dimensions works
TEST_P(CopyTests_T2B, FullTextureAligned) {
constexpr uint32_t kWidth = 256;

View File

@ -266,14 +266,13 @@ TEST_F(CopyCommandTest_B2B, BuffersInErrorState) {
}
}
// Test B2B copies within same buffer.
// Test it is not allowed to do B2B copies within same buffer.
TEST_F(CopyCommandTest_B2B, CopyWithinSameBuffer) {
constexpr uint32_t kBufferSize = 16u;
wgpu::Buffer buffer =
CreateBuffer(kBufferSize, wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst);
// When srcOffset < dstOffset, and srcOffset + copySize > dstOffset, it is not allowed because
// the copy regions are overlapping.
// srcOffset < dstOffset, and srcOffset + copySize > dstOffset (overlapping)
{
constexpr uint32_t kSrcOffset = 0u;
constexpr uint32_t kDstOffset = 4u;
@ -285,19 +284,17 @@ TEST_F(CopyCommandTest_B2B, CopyWithinSameBuffer) {
ASSERT_DEVICE_ERROR(encoder.Finish());
}
// When srcOffset < dstOffset, and srcOffset + copySize == dstOffset, it is allowed
// because the copy regions are not overlapping.
// srcOffset < dstOffset, and srcOffset + copySize == dstOffset (not overlapping)
{
constexpr uint32_t kSrcOffset = 0u;
constexpr uint32_t kDstOffset = 8u;
constexpr uint32_t kCopySize = kDstOffset - kSrcOffset;
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.CopyBufferToBuffer(buffer, kSrcOffset, buffer, kDstOffset, kCopySize);
encoder.Finish();
ASSERT_DEVICE_ERROR(encoder.Finish());
}
// When srcOffset > dstOffset, and srcOffset < dstOffset + copySize, it is not allowed because
// the copy regions are overlapping.
// srcOffset > dstOffset, and srcOffset < dstOffset + copySize (overlapping)
{
constexpr uint32_t kSrcOffset = 4u;
constexpr uint32_t kDstOffset = 0u;
@ -309,15 +306,14 @@ TEST_F(CopyCommandTest_B2B, CopyWithinSameBuffer) {
ASSERT_DEVICE_ERROR(encoder.Finish());
}
// When srcOffset > dstOffset, and srcOffset + copySize == dstOffset, it is allowed
// because the copy regions are not overlapping.
// srcOffset > dstOffset, and srcOffset + copySize == dstOffset (not overlapping)
{
constexpr uint32_t kSrcOffset = 8u;
constexpr uint32_t kDstOffset = 0u;
constexpr uint32_t kCopySize = kSrcOffset - kDstOffset;
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.CopyBufferToBuffer(buffer, kSrcOffset, buffer, kDstOffset, kCopySize);
encoder.Finish();
ASSERT_DEVICE_ERROR(encoder.Finish());
}
}