diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 2450edc919..b1a63488eb 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -15,6 +15,7 @@ #include "dawn_native/CommandEncoder.h" #include "common/BitSetIterator.h" +#include "common/Math.h" #include "dawn_native/BindGroup.h" #include "dawn_native/Buffer.h" #include "dawn_native/CommandBuffer.h" @@ -713,12 +714,20 @@ namespace dawn_native { defaultedRowsPerImage = copySize->height; } + // In the case of one row copy bytesPerRow might not contain enough bytes + uint32_t bytesPerRow = source->layout.bytesPerRow; + if (copySize->height <= 1 && copySize->depth <= 1) { + bytesPerRow = + Align(copySize->width * destination->texture->GetFormat().blockByteSize, + kTextureBytesPerRowAlignment); + } + // Record the copy command. CopyBufferToTextureCmd* copy = allocator->Allocate(Command::CopyBufferToTexture); copy->source.buffer = source->buffer; copy->source.offset = source->layout.offset; - copy->source.bytesPerRow = source->layout.bytesPerRow; + copy->source.bytesPerRow = bytesPerRow; copy->source.rowsPerImage = defaultedRowsPerImage; copy->destination.texture = destination->texture; copy->destination.origin = destination->origin; @@ -773,6 +782,13 @@ namespace dawn_native { defaultedRowsPerImage = copySize->height; } + // In the case of one row copy bytesPerRow might not contain enough bytes + uint32_t bytesPerRow = destination->layout.bytesPerRow; + if (copySize->height <= 1 && copySize->depth <= 1) { + bytesPerRow = Align(copySize->width * source->texture->GetFormat().blockByteSize, + kTextureBytesPerRowAlignment); + } + // Record the copy command. CopyTextureToBufferCmd* copy = allocator->Allocate(Command::CopyTextureToBuffer); @@ -782,7 +798,7 @@ namespace dawn_native { copy->source.aspect = source->aspect; copy->destination.buffer = destination->buffer; copy->destination.offset = destination->layout.offset; - copy->destination.bytesPerRow = destination->layout.bytesPerRow; + copy->destination.bytesPerRow = bytesPerRow; copy->destination.rowsPerImage = defaultedRowsPerImage; copy->copySize = *copySize; diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 095cf61a83..034585a527 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -440,8 +440,9 @@ namespace dawn_native { } // Validation for other members in layout: - if (layout.bytesPerRow < - copyExtent.width / blockInfo.blockWidth * blockInfo.blockByteSize) { + if ((copyExtent.height > 1 || copyExtent.depth > 1) && + layout.bytesPerRow < + copyExtent.width / blockInfo.blockWidth * blockInfo.blockByteSize) { return DAWN_VALIDATION_ERROR( "bytesPerRow must not be less than the number of bytes per row"); } diff --git a/src/tests/end2end/CopyTests.cpp b/src/tests/end2end/CopyTests.cpp index 28728bdfaa..434e248453 100644 --- a/src/tests/end2end/CopyTests.cpp +++ b/src/tests/end2end/CopyTests.cpp @@ -667,6 +667,32 @@ TEST_P(CopyTests_T2B, RowPitchUnaligned) { } } +// Test that copying with bytesPerRow = 0 and bytesPerRow < bytesInACompleteRow works +// when we're copying one row only +TEST_P(CopyTests_T2B, BytesPerRowWithOneRowCopy) { + constexpr uint32_t kWidth = 259; + constexpr uint32_t kHeight = 127; + + TextureSpec textureSpec; + textureSpec.copyOrigin = {0, 0, 0}; + textureSpec.textureSize = {kWidth, kHeight, 1}; + textureSpec.level = 0; + + // bytesPerRow = 0 + { + BufferSpec bufferSpec = MinimumBufferSpec(5, 1); + bufferSpec.bytesPerRow = 0; + DoTest(textureSpec, bufferSpec, {5, 1, 1}); + } + + // bytesPerRow < bytesInACompleteRow + { + BufferSpec bufferSpec = MinimumBufferSpec(259, 1); + bufferSpec.bytesPerRow = 256; + DoTest(textureSpec, bufferSpec, {259, 1, 1}); + } +} + // Test that copying whole texture 2D array layers in one texture-to-buffer-copy works. TEST_P(CopyTests_T2B, Texture2DArrayRegion) { constexpr uint32_t kWidth = 256; @@ -1076,6 +1102,32 @@ TEST_P(CopyTests_B2T, RowPitchUnaligned) { } } +// Test that copying with bytesPerRow = 0 and bytesPerRow < bytesInACompleteRow works +// when we're copying one row only +TEST_P(CopyTests_B2T, BytesPerRowWithOneRowCopy) { + constexpr uint32_t kWidth = 259; + constexpr uint32_t kHeight = 127; + + TextureSpec textureSpec; + textureSpec.copyOrigin = {0, 0, 0}; + textureSpec.textureSize = {kWidth, kHeight, 1}; + textureSpec.level = 0; + + // bytesPerRow = 0 + { + BufferSpec bufferSpec = MinimumBufferSpec(5, 1); + bufferSpec.bytesPerRow = 0; + DoTest(textureSpec, bufferSpec, {5, 1, 1}); + } + + // bytesPerRow < bytesInACompleteRow + { + BufferSpec bufferSpec = MinimumBufferSpec(259, 1); + bufferSpec.bytesPerRow = 256; + DoTest(textureSpec, bufferSpec, {259, 1, 1}); + } +} + // Test that copying whole texture 2D array layers in one texture-to-buffer-copy works. TEST_P(CopyTests_B2T, Texture2DArrayRegion) { constexpr uint32_t kWidth = 256; diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index 6542c54f70..7d65e83131 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/src/tests/end2end/DeprecatedAPITests.cpp @@ -323,18 +323,18 @@ TEST_P(BufferCopyViewDeprecationTests, DeprecationWarning) { bufCopy.offset = 4; EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texCopy, ©Size)); EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texCopy, &bufCopy, ©Size)); - // Since bytesPerRow is 0 - ASSERT_DEVICE_ERROR(encoder.Finish()); + wgpu::CommandBuffer command = encoder.Finish(); + queue.Submit(1, &command); } { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::BufferCopyView bufCopy = {}; bufCopy.buffer = buffer; - bufCopy.bytesPerRow = kTextureBytesPerRowAlignment; + bufCopy.bytesPerRow = 128; EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texCopy, ©Size)); EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texCopy, &bufCopy, ©Size)); - wgpu::CommandBuffer command = encoder.Finish(); - queue.Submit(1, &command); + // Since bytesPerRow is not 256-aligned. + ASSERT_DEVICE_ERROR(encoder.Finish()); } { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -343,8 +343,8 @@ TEST_P(BufferCopyViewDeprecationTests, DeprecationWarning) { bufCopy.rowsPerImage = 1; EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texCopy, ©Size)); EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texCopy, &bufCopy, ©Size)); - // Since bytesPerRow is 0 - ASSERT_DEVICE_ERROR(encoder.Finish()); + wgpu::CommandBuffer command = encoder.Finish(); + queue.Submit(1, &command); } } diff --git a/src/tests/end2end/QueueTests.cpp b/src/tests/end2end/QueueTests.cpp index 4e5febdc0b..edf9ccf980 100644 --- a/src/tests/end2end/QueueTests.cpp +++ b/src/tests/end2end/QueueTests.cpp @@ -462,6 +462,36 @@ TEST_P(QueueWriteTextureTests, VaryingBytesPerRow) { } } +// Test that writing with bytesPerRow = 0 and bytesPerRow < bytesInACompleteRow works +// when we're copying one row only +TEST_P(QueueWriteTextureTests, BytesPerRowWithOneRowCopy) { + constexpr uint32_t kWidth = 259; + constexpr uint32_t kHeight = 127; + + TextureSpec textureSpec; + textureSpec.copyOrigin = {0, 0, 0}; + textureSpec.textureSize = {kWidth, kHeight, 1}; + textureSpec.level = 0; + + // bytesPerRow = 0 + { + constexpr wgpu::Extent3D copyExtent = {5, 1, 1}; + + DataSpec dataSpec = MinimumDataSpec(copyExtent); + dataSpec.bytesPerRow = 0; + DoTest(textureSpec, dataSpec, copyExtent); + } + + // bytesPerRow < bytesInACompleteRow + { + constexpr wgpu::Extent3D copyExtent = {259, 1, 1}; + + DataSpec dataSpec = MinimumDataSpec(copyExtent); + dataSpec.bytesPerRow = 256; + DoTest(textureSpec, dataSpec, copyExtent); + } +} + // Test with bytesPerRow greater than needed in a write to a texture array. TEST_P(QueueWriteTextureTests, VaryingArrayBytesPerRow) { constexpr uint32_t kWidth = 257; diff --git a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp index 1edb39650b..3d4f65c871 100644 --- a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp +++ b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp @@ -507,16 +507,45 @@ TEST_F(CopyCommandTest_B2T, IncorrectBytesPerRow) { wgpu::TextureUsage::CopyDst); // bytes per row is 0 - TestB2TCopy(utils::Expectation::Failure, source, 0, 0, 0, destination, 0, {0, 0, 0}, - {64, 4, 1}); + { + // copyHeight > 1 + TestB2TCopy(utils::Expectation::Failure, source, 0, 0, 0, destination, 0, {0, 0, 0}, + {64, 4, 1}); + + // copyDepth > 1 + TestB2TCopy(utils::Expectation::Failure, source, 0, 0, 1, destination, 0, {0, 0, 0}, + {64, 1, 4}); + + // copyHeight = 1 and copyDepth = 1 + TestB2TCopy(utils::Expectation::Success, source, 0, 0, 0, destination, 0, {0, 0, 0}, + {64, 1, 1}); + } // bytes per row is not 256-byte aligned - TestB2TCopy(utils::Expectation::Failure, source, 0, 128, 0, destination, 0, {0, 0, 0}, - {4, 4, 1}); + { + // copyHeight > 1 + TestB2TCopy(utils::Expectation::Failure, source, 0, 128, 0, destination, 0, {0, 0, 0}, + {4, 4, 1}); + + // copyHeight = 1 and copyDepth = 1 + TestB2TCopy(utils::Expectation::Failure, source, 0, 128, 0, destination, 0, {0, 0, 0}, + {4, 1, 1}); + } // bytes per row is less than width * bytesPerPixel - TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 0, destination, 0, {0, 0, 0}, - {65, 1, 1}); + { + // copyHeight > 1 + TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 0, destination, 0, {0, 0, 0}, + {65, 2, 1}); + + // copyDepth > 1 + TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 1, destination, 0, {0, 0, 0}, + {65, 1, 2}); + + // copyHeight = 1 and copyDepth = 1 + TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0, destination, 0, {0, 0, 0}, + {65, 1, 1}); + } } TEST_F(CopyCommandTest_B2T, ImageHeightConstraint) { @@ -907,20 +936,49 @@ TEST_F(CopyCommandTest_T2B, IncorrectUsage) { TEST_F(CopyCommandTest_T2B, IncorrectBytesPerRow) { uint64_t bufferSize = BufferSizeForTextureCopy(128, 16, 1); wgpu::Texture source = Create2DTexture(128, 16, 5, 1, wgpu::TextureFormat::RGBA8Unorm, - wgpu::TextureUsage::CopyDst); - wgpu::Buffer destination = CreateBuffer(bufferSize, wgpu::BufferUsage::CopySrc); + wgpu::TextureUsage::CopySrc); + wgpu::Buffer destination = CreateBuffer(bufferSize, wgpu::BufferUsage::CopyDst); // bytes per row is 0 - TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0, - {64, 4, 1}); + { + // copyHeight > 1 + TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 0, 0, + {64, 4, 1}); + + // copyDepth > 1 + TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 0, 1, + {64, 1, 4}); + + // copyHeight = 1 and copyDepth = 1 + TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, 0, 0, + {64, 1, 1}); + } // bytes per row is not 256-byte aligned - TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 257, 0, - {4, 4, 1}); + { + // copyHeight > 1 + TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 128, 0, + {4, 4, 1}); + + // copyHeight = 1 and copyDepth = 1 + TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 128, 0, + {4, 1, 1}); + } // bytes per row is less than width * bytesPerPixel - TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0, - {65, 1, 1}); + { + // copyHeight > 1 + TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0, + {65, 2, 1}); + + // copyDepth > 1 + TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 1, + {65, 1, 2}); + + // copyHeight = 1 and copyDepth = 1 + TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, 256, 0, + {65, 1, 1}); + } } TEST_F(CopyCommandTest_T2B, ImageHeightConstraint) { @@ -1033,9 +1091,9 @@ TEST_F(CopyCommandTest_T2B, TextureCopyBufferSizeLastRowComputation) { for (wgpu::TextureFormat format : kFormats) { wgpu::Texture source = - Create2DTexture(kWidth, kHeight, 1, 1, format, wgpu::TextureUsage::CopyDst); + Create2DTexture(kWidth, kHeight, 1, 1, format, wgpu::TextureUsage::CopySrc); - wgpu::Buffer destination = CreateBuffer(kInvalidBufferSize, wgpu::BufferUsage::CopySrc); + wgpu::Buffer destination = CreateBuffer(kInvalidBufferSize, wgpu::BufferUsage::CopyDst); TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, kBytesPerRow, 0, {kWidth, kHeight, 1}); } diff --git a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp index a9dca68d30..d0aad6170e 100644 --- a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp +++ b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp @@ -221,11 +221,33 @@ namespace { wgpu::Texture destination = Create2DTexture({3, 7, 1}, 1, wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureUsage::CopyDst); - // bytesPerRow = 0 is invalid - ASSERT_DEVICE_ERROR(TestWriteTexture(128, 0, 0, 0, destination, 0, {0, 0, 0}, {3, 7, 1})); + // bytesPerRow = 0 + { + // copyHeight > 1 + ASSERT_DEVICE_ERROR( + TestWriteTexture(128, 0, 0, 0, destination, 0, {0, 0, 0}, {3, 7, 1})); + + // copyDepth > 1 + ASSERT_DEVICE_ERROR( + TestWriteTexture(128, 0, 0, 1, destination, 0, {0, 0, 0}, {3, 1, 2})); + + // copyHeight = 1 and copyDepth = 1 + TestWriteTexture(128, 0, 0, 0, destination, 0, {0, 0, 0}, {3, 1, 1}); + } // bytesPerRow = 11 is invalid since a row takes 12 bytes. - ASSERT_DEVICE_ERROR(TestWriteTexture(128, 0, 11, 0, destination, 0, {0, 0, 0}, {3, 7, 1})); + { + // copyHeight > 1 + ASSERT_DEVICE_ERROR( + TestWriteTexture(128, 0, 11, 0, destination, 0, {0, 0, 0}, {3, 7, 1})); + + // copyDepth > 1 + ASSERT_DEVICE_ERROR( + TestWriteTexture(128, 0, 11, 1, destination, 0, {0, 0, 0}, {3, 1, 2})); + + // copyHeight = 1 and copyDepth = 1 + TestWriteTexture(128, 0, 11, 0, destination, 0, {0, 0, 0}, {3, 1, 1}); + } // bytesPerRow = 12 is valid since a row takes 12 bytes. TestWriteTexture(128, 0, 12, 0, destination, 0, {0, 0, 0}, {3, 7, 1});