Allow unaligned source offset in writeTexture
With some refactoring of the relevant validation code.
Bug: dawn:520
Change-Id: Iedda0f7b1b67c20d3a88f2c4183dcc8eeae2096f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/30742
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index 0df7870..5468387 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -101,36 +101,20 @@
return {};
}
- MaybeError ValidateTextureToBufferCopyRestrictions(const TextureCopyView& src) {
- const Format& format = src.texture->GetFormat();
-
- bool depthSelected = false;
- switch (src.aspect) {
- case wgpu::TextureAspect::All:
- switch (format.aspects) {
- case Aspect::Color:
- case Aspect::Stencil:
- break;
- case Aspect::Depth:
- depthSelected = true;
- break;
- default:
- return DAWN_VALIDATION_ERROR(
- "A single aspect must be selected for multi planar formats in "
- "texture to buffer copies");
- }
- break;
- case wgpu::TextureAspect::DepthOnly:
- ASSERT(format.aspects & Aspect::Depth);
- depthSelected = true;
- break;
- case wgpu::TextureAspect::StencilOnly:
- ASSERT(format.aspects & Aspect::Stencil);
- break;
+ MaybeError ValidateLinearTextureCopyOffset(const TextureDataLayout& layout,
+ const TexelBlockInfo& blockInfo) {
+ if (layout.offset % blockInfo.byteSize != 0) {
+ return DAWN_VALIDATION_ERROR(
+ "offset must be a multiple of the texel block byte size.");
}
+ return {};
+ }
- if (depthSelected) {
- switch (format.format) {
+ MaybeError ValidateTextureDepthStencilToBufferCopyRestrictions(const TextureCopyView& src) {
+ Aspect aspectUsed;
+ DAWN_TRY_ASSIGN(aspectUsed, SingleAspectUsedByTextureCopyView(src));
+ if (aspectUsed == Aspect::Depth) {
+ switch (src.texture->GetFormat().format) {
case wgpu::TextureFormat::Depth24Plus:
case wgpu::TextureFormat::Depth24PlusStencil8:
return DAWN_VALIDATION_ERROR(
@@ -640,7 +624,7 @@
DAWN_TRY(ValidateCanUseAs(destination->texture, wgpu::TextureUsage::CopyDst));
DAWN_TRY(ValidateTextureSampleCountInBufferCopyCommands(destination->texture));
- DAWN_TRY(ValidateBufferToTextureCopyRestrictions(*destination));
+ DAWN_TRY(ValidateLinearToDepthStencilCopyRestrictions(*destination));
// We validate texture copy range before validating linear texture data,
// because in the latter we divide copyExtent.width by blockWidth and
// copyExtent.height by blockHeight while the divisibility conditions are
@@ -650,6 +634,7 @@
const TexelBlockInfo& blockInfo =
destination->texture->GetFormat().GetAspectInfo(destination->aspect).block;
if (GetDevice()->IsValidationEnabled()) {
+ DAWN_TRY(ValidateLinearTextureCopyOffset(source->layout, blockInfo));
DAWN_TRY(ValidateLinearTextureData(source->layout, source->buffer->GetSize(),
blockInfo, *copySize));
@@ -700,11 +685,11 @@
DAWN_TRY(ValidateTextureCopyView(GetDevice(), *source, *copySize));
DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::CopySrc));
DAWN_TRY(ValidateTextureSampleCountInBufferCopyCommands(source->texture));
+ DAWN_TRY(ValidateTextureDepthStencilToBufferCopyRestrictions(*source));
DAWN_TRY(ValidateBufferCopyView(GetDevice(), *destination));
DAWN_TRY(ValidateCanUseAs(destination->buffer, wgpu::BufferUsage::CopyDst));
- DAWN_TRY(ValidateTextureToBufferCopyRestrictions(*source));
// We validate texture copy range before validating linear texture data,
// because in the latter we divide copyExtent.width by blockWidth and
// copyExtent.height by blockHeight while the divisibility conditions are
@@ -714,6 +699,7 @@
const TexelBlockInfo& blockInfo =
source->texture->GetFormat().GetAspectInfo(source->aspect).block;
if (GetDevice()->IsValidationEnabled()) {
+ DAWN_TRY(ValidateLinearTextureCopyOffset(destination->layout, blockInfo));
DAWN_TRY(ValidateLinearTextureData(
destination->layout, destination->buffer->GetSize(), blockInfo, *copySize));
diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp
index 7e978f2..4a19243 100644
--- a/src/dawn_native/CommandValidation.cpp
+++ b/src/dawn_native/CommandValidation.cpp
@@ -446,11 +446,6 @@
uint64_t byteSize,
const TexelBlockInfo& blockInfo,
const Extent3D& copyExtent) {
- // Validation for the texel block alignments:
- if (layout.offset % blockInfo.byteSize != 0) {
- return DAWN_VALIDATION_ERROR("Offset must be a multiple of the texel or block size");
- }
-
ASSERT(copyExtent.width % blockInfo.width == 0);
uint32_t widthInBlocks = copyExtent.width / blockInfo.width;
ASSERT(copyExtent.height % blockInfo.height == 0);
@@ -589,36 +584,35 @@
return {};
}
- MaybeError ValidateBufferToTextureCopyRestrictions(const TextureCopyView& dst) {
- const Format& format = dst.texture->GetFormat();
-
- bool depthSelected = false;
- switch (dst.aspect) {
+ // Always returns a single aspect (color, stencil, or depth).
+ ResultOrError<Aspect> SingleAspectUsedByTextureCopyView(const TextureCopyView& view) {
+ const Format& format = view.texture->GetFormat();
+ switch (view.aspect) {
case wgpu::TextureAspect::All:
- switch (format.aspects) {
- case Aspect::Color:
- case Aspect::Stencil:
- break;
- case Aspect::Depth:
- depthSelected = true;
- break;
- default:
- return DAWN_VALIDATION_ERROR(
- "A single aspect must be selected for multi planar formats in buffer "
- "to texture copies");
+ if (HasOneBit(format.aspects)) {
+ return Aspect{format.aspects};
+ } else {
+ return DAWN_VALIDATION_ERROR(
+ "A single aspect must be selected for multi-planar formats in "
+ "texture <-> linear data copies");
}
break;
case wgpu::TextureAspect::DepthOnly:
ASSERT(format.aspects & Aspect::Depth);
- depthSelected = true;
- break;
+ return Aspect::Depth;
case wgpu::TextureAspect::StencilOnly:
ASSERT(format.aspects & Aspect::Stencil);
- break;
+ return Aspect::Stencil;
}
- if (depthSelected) {
+ }
+
+ MaybeError ValidateLinearToDepthStencilCopyRestrictions(const TextureCopyView& dst) {
+ Aspect aspectUsed;
+ DAWN_TRY_ASSIGN(aspectUsed, SingleAspectUsedByTextureCopyView(dst));
+ if (aspectUsed == Aspect::Depth) {
return DAWN_VALIDATION_ERROR("Cannot copy into the depth aspect of a texture");
}
+
return {};
}
diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h
index 3853b98..faf6f12 100644
--- a/src/dawn_native/CommandValidation.h
+++ b/src/dawn_native/CommandValidation.h
@@ -57,7 +57,8 @@
const Extent3D& copyExtent);
MaybeError ValidateTextureCopyRange(const TextureCopyView& textureCopyView,
const Extent3D& copySize);
- MaybeError ValidateBufferToTextureCopyRestrictions(const TextureCopyView& dst);
+ ResultOrError<Aspect> SingleAspectUsedByTextureCopyView(const TextureCopyView& view);
+ MaybeError ValidateLinearToDepthStencilCopyRestrictions(const TextureCopyView& dst);
MaybeError ValidateBufferCopyView(DeviceBase const* device,
const BufferCopyView& bufferCopyView);
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp
index 519cf93..cd3a135 100644
--- a/src/dawn_native/Queue.cpp
+++ b/src/dawn_native/Queue.cpp
@@ -415,7 +415,7 @@
return DAWN_VALIDATION_ERROR("The sample count of textures must be 1");
}
- DAWN_TRY(ValidateBufferToTextureCopyRestrictions(*destination));
+ DAWN_TRY(ValidateLinearToDepthStencilCopyRestrictions(*destination));
// We validate texture copy range before validating linear texture data,
// because in the latter we divide copyExtent.width by blockWidth and
// copyExtent.height by blockHeight while the divisibility conditions are
diff --git a/src/tests/end2end/QueueTests.cpp b/src/tests/end2end/QueueTests.cpp
index d74ec00..bcda12f 100644
--- a/src/tests/end2end/QueueTests.cpp
+++ b/src/tests/end2end/QueueTests.cpp
@@ -446,9 +446,8 @@
textureSpec.textureSize = {kWidth, kHeight, 1};
textureSpec.level = 0;
- for (unsigned int i : {1, 2, 4, 17, 64, 128, 300}) {
+ for (uint64_t offset : {1, 2, 4, 17, 64, 128, 300}) {
DataSpec dataSpec = MinimumDataSpec({kWidth, kHeight, 1});
- uint64_t offset = i * utils::GetTexelBlockSizeInBytes(kTextureFormat);
dataSpec.size += offset;
dataSpec.offset += offset;
DoTest(textureSpec, dataSpec, {kWidth, kHeight, 1});
diff --git a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
index 277c6f2..6b1c8de 100644
--- a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
+++ b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
@@ -278,18 +278,20 @@
TestWriteTexture(dataSize, 0, 256, 3, destination, 0, {0, 0, 0}, {4, 4, 1}));
}
- // Test WriteTexture with incorrect data offset usage
- TEST_F(QueueWriteTextureValidationTest, IncorrectDataOffset) {
+ // Test WriteTexture with data offset
+ TEST_F(QueueWriteTextureValidationTest, DataOffset) {
uint64_t dataSize =
utils::RequiredBytesInCopy(256, 0, {4, 4, 1}, wgpu::TextureFormat::RGBA8Unorm);
wgpu::Texture destination = Create2DTexture({16, 16, 1}, 5, wgpu::TextureFormat::RGBA8Unorm,
wgpu::TextureUsage::CopyDst);
- // Correct usage
+ // Offset aligned
TestWriteTexture(dataSize, dataSize - 4, 256, 0, destination, 0, {0, 0, 0}, {1, 1, 1});
-
+ // Offset not aligned
+ TestWriteTexture(dataSize, dataSize - 5, 256, 0, destination, 0, {0, 0, 0}, {1, 1, 1});
+ // Offset+size too large
ASSERT_DEVICE_ERROR(
- TestWriteTexture(dataSize, dataSize - 6, 256, 0, destination, 0, {0, 0, 0}, {1, 1, 1}));
+ TestWriteTexture(dataSize, dataSize - 3, 256, 0, destination, 0, {0, 0, 0}, {1, 1, 1}));
}
// Test multisampled textures can be used in WriteTexture.
@@ -551,23 +553,21 @@
static constexpr uint32_t kHeight = 16;
};
- // Tests to verify that data offset must be a multiple of the compressed texture blocks in bytes
+ // Tests to verify that data offset may not be a multiple of the compressed texture block size
TEST_F(WriteTextureTest_CompressedTextureFormats, DataOffset) {
for (wgpu::TextureFormat bcFormat : utils::kBCFormats) {
wgpu::Texture texture = Create2DTexture(bcFormat);
- // Valid usages of data offset.
+ // Valid if aligned.
{
- uint32_t validDataOffset = utils::GetTexelBlockSizeInBytes(bcFormat);
- QueueWriteTextureValidationTest::TestWriteTexture(512, validDataOffset, 256, 4,
- texture, 0, {0, 0, 0}, {4, 4, 1});
+ uint32_t kAlignedOffset = utils::GetTexelBlockSizeInBytes(bcFormat);
+ TestWriteTexture(1024, kAlignedOffset, 256, 4, texture, 0, {0, 0, 0}, {4, 16, 1});
}
- // Failures on invalid data offset.
+ // Still valid if not aligned.
{
- uint32_t kInvalidDataOffset = utils::GetTexelBlockSizeInBytes(bcFormat) / 2;
- ASSERT_DEVICE_ERROR(TestWriteTexture(512, kInvalidDataOffset, 256, 4, texture, 0,
- {0, 0, 0}, {4, 4, 1}));
+ uint32_t kUnalignedOffset = utils::GetTexelBlockSizeInBytes(bcFormat) - 1;
+ TestWriteTexture(1024, kUnalignedOffset, 256, 4, texture, 0, {0, 0, 0}, {4, 16, 1});
}
}
}
