Drop deprecated rowsPerImage/bytesPerRow behavior
Removes the last remaining code paths supporting the old rowsPerImage
and bytesPerRow defaulting behavior, and updates all related tests to
strictly expect the spec-complaint behavior.
Change-Id: I022db0b142939d82e33d5989460488881e5a1ab3
Bug: dawn:520
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/56803
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index 1c1cd72..58c4a6d 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -693,18 +693,18 @@
}
const TexelBlockInfo& blockInfo =
destination->texture->GetFormat().GetAspectInfo(destination->aspect).block;
- TextureDataLayout srcLayout = FixUpDeprecatedTextureDataLayoutOptions(
- GetDevice(), source->layout, blockInfo, *copySize);
if (GetDevice()->IsValidationEnabled()) {
DAWN_TRY(ValidateLinearTextureCopyOffset(
- srcLayout, blockInfo, destination->texture->GetFormat().HasDepthOrStencil()));
- DAWN_TRY(ValidateLinearTextureData(srcLayout, source->buffer->GetSize(), blockInfo,
- *copySize));
+ source->layout, blockInfo,
+ destination->texture->GetFormat().HasDepthOrStencil()));
+ DAWN_TRY(ValidateLinearTextureData(source->layout, source->buffer->GetSize(),
+ blockInfo, *copySize));
mTopLevelBuffers.insert(source->buffer);
mTopLevelTextures.insert(destination->texture);
}
+ TextureDataLayout srcLayout = source->layout;
ApplyDefaultTextureDataLayoutOptions(&srcLayout, blockInfo, *copySize);
CopyBufferToTextureCmd* copy =
@@ -745,18 +745,18 @@
}
const TexelBlockInfo& blockInfo =
source->texture->GetFormat().GetAspectInfo(source->aspect).block;
- TextureDataLayout dstLayout = FixUpDeprecatedTextureDataLayoutOptions(
- GetDevice(), destination->layout, blockInfo, *copySize);
if (GetDevice()->IsValidationEnabled()) {
DAWN_TRY(ValidateLinearTextureCopyOffset(
- dstLayout, blockInfo, source->texture->GetFormat().HasDepthOrStencil()));
- DAWN_TRY(ValidateLinearTextureData(dstLayout, destination->buffer->GetSize(),
- blockInfo, *copySize));
+ destination->layout, blockInfo,
+ source->texture->GetFormat().HasDepthOrStencil()));
+ DAWN_TRY(ValidateLinearTextureData(
+ destination->layout, destination->buffer->GetSize(), blockInfo, *copySize));
mTopLevelTextures.insert(source->texture);
mTopLevelBuffers.insert(destination->buffer);
}
+ TextureDataLayout dstLayout = destination->layout;
ApplyDefaultTextureDataLayoutOptions(&dstLayout, blockInfo, *copySize);
CopyTextureToBufferCmd* copy =
diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp
index a5b0543..e46d438 100644
--- a/src/dawn_native/CommandValidation.cpp
+++ b/src/dawn_native/CommandValidation.cpp
@@ -143,45 +143,6 @@
return {};
}
- TextureDataLayout FixUpDeprecatedTextureDataLayoutOptions(
- DeviceBase* device,
- const TextureDataLayout& originalLayout,
- const TexelBlockInfo& blockInfo,
- const Extent3D& copyExtent) {
- // TODO(crbug.com/dawn/520): Remove deprecated functionality.
- TextureDataLayout layout = originalLayout;
-
- if (copyExtent.height != 0 && layout.rowsPerImage == 0) {
- if (copyExtent.depthOrArrayLayers > 1) {
- device->EmitDeprecationWarning(
- "rowsPerImage soon must be non-zero if copy depth > 1 (it will no longer "
- "default to the copy height).");
- ASSERT(copyExtent.height % blockInfo.height == 0);
- uint32_t heightInBlocks = copyExtent.height / blockInfo.height;
- layout.rowsPerImage = heightInBlocks;
- } else if (copyExtent.depthOrArrayLayers == 1) {
- device->EmitDeprecationWarning(
- "rowsPerImage soon must be non-zero or unspecified if copy depth == 1 (it will "
- "no longer default to the copy height).");
- layout.rowsPerImage = wgpu::kCopyStrideUndefined;
- }
- }
-
- // Only bother to fix-up for height == 1 && depth == 1.
- // The other cases that used to be allowed were zero-size copies.
- ASSERT(copyExtent.width % blockInfo.width == 0);
- uint32_t widthInBlocks = copyExtent.width / blockInfo.width;
- uint32_t bytesInLastRow = widthInBlocks * blockInfo.byteSize;
- if (copyExtent.height == 1 && copyExtent.depthOrArrayLayers == 1 &&
- bytesInLastRow > layout.bytesPerRow) {
- device->EmitDeprecationWarning(
- "Soon, even if copy height == 1, bytesPerRow must be >= the byte size of each row "
- "or left unspecified.");
- layout.bytesPerRow = wgpu::kCopyStrideUndefined;
- }
- return layout;
- }
-
// Replace wgpu::kCopyStrideUndefined with real values, so backends don't have to think about
// it.
void ApplyDefaultTextureDataLayoutOptions(TextureDataLayout* layout,
diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h
index 01c7a65..a822d05 100644
--- a/src/dawn_native/CommandValidation.h
+++ b/src/dawn_native/CommandValidation.h
@@ -36,11 +36,6 @@
uint32_t bytesPerRow,
uint32_t rowsPerImage);
- TextureDataLayout FixUpDeprecatedTextureDataLayoutOptions(
- DeviceBase* device,
- const TextureDataLayout& originalLayout,
- const TexelBlockInfo& blockInfo,
- const Extent3D& copyExtent);
void ApplyDefaultTextureDataLayoutOptions(TextureDataLayout* layout,
const TexelBlockInfo& blockInfo,
const Extent3D& copyExtent);
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp
index 05e76a0..9aac22b 100644
--- a/src/dawn_native/Queue.cpp
+++ b/src/dawn_native/Queue.cpp
@@ -278,13 +278,13 @@
const TextureDataLayout* dataLayout,
const Extent3D* writeSize) {
GetDevice()->ConsumedError(
- WriteTextureInternal(destination, data, dataSize, dataLayout, writeSize));
+ WriteTextureInternal(destination, data, dataSize, *dataLayout, writeSize));
}
MaybeError QueueBase::WriteTextureInternal(const ImageCopyTexture* destination,
const void* data,
size_t dataSize,
- const TextureDataLayout* dataLayout,
+ const TextureDataLayout& dataLayout,
const Extent3D* writeSize) {
DAWN_TRY(ValidateWriteTexture(destination, dataSize, dataLayout, writeSize));
@@ -294,7 +294,7 @@
const TexelBlockInfo& blockInfo =
destination->texture->GetFormat().GetAspectInfo(destination->aspect).block;
- TextureDataLayout layout = *dataLayout;
+ TextureDataLayout layout = dataLayout;
ApplyDefaultTextureDataLayoutOptions(&layout, blockInfo, *writeSize);
return WriteTextureImpl(*destination, data, layout, *writeSize);
}
@@ -460,7 +460,7 @@
MaybeError QueueBase::ValidateWriteTexture(const ImageCopyTexture* destination,
size_t dataSize,
- const TextureDataLayout* dataLayout,
+ const TextureDataLayout& dataLayout,
const Extent3D* writeSize) const {
DAWN_TRY(GetDevice()->ValidateIsAlive());
DAWN_TRY(GetDevice()->ValidateObject(this));
@@ -468,7 +468,7 @@
DAWN_TRY(ValidateImageCopyTexture(GetDevice(), *destination, *writeSize));
- if (dataLayout->offset > dataSize) {
+ if (dataLayout.offset > dataSize) {
return DAWN_VALIDATION_ERROR("Queue::WriteTexture out of range");
}
@@ -490,9 +490,7 @@
const TexelBlockInfo& blockInfo =
destination->texture->GetFormat().GetAspectInfo(destination->aspect).block;
- TextureDataLayout layout = FixUpDeprecatedTextureDataLayoutOptions(GetDevice(), *dataLayout,
- blockInfo, *writeSize);
- DAWN_TRY(ValidateLinearTextureData(layout, dataSize, blockInfo, *writeSize));
+ DAWN_TRY(ValidateLinearTextureData(dataLayout, dataSize, blockInfo, *writeSize));
DAWN_TRY(destination->texture->ValidateCanUseInSubmitNow());
diff --git a/src/dawn_native/Queue.h b/src/dawn_native/Queue.h
index b1795c1..f0178d2 100644
--- a/src/dawn_native/Queue.h
+++ b/src/dawn_native/Queue.h
@@ -71,7 +71,7 @@
MaybeError WriteTextureInternal(const ImageCopyTexture* destination,
const void* data,
size_t dataSize,
- const TextureDataLayout* dataLayout,
+ const TextureDataLayout& dataLayout,
const Extent3D* writeSize);
MaybeError CopyTextureForBrowserInternal(const ImageCopyTexture* source,
const ImageCopyTexture* destination,
@@ -97,7 +97,7 @@
size_t size) const;
MaybeError ValidateWriteTexture(const ImageCopyTexture* destination,
size_t dataSize,
- const TextureDataLayout* dataLayout,
+ const TextureDataLayout& dataLayout,
const Extent3D* writeSize) const;
void SubmitInternal(uint32_t commandCount, CommandBufferBase* const* commands);
diff --git a/src/tests/end2end/CopyTests.cpp b/src/tests/end2end/CopyTests.cpp
index ea73a7c..0cb348c 100644
--- a/src/tests/end2end/CopyTests.cpp
+++ b/src/tests/end2end/CopyTests.cpp
@@ -824,23 +824,10 @@
{
BufferSpec bufferSpec = MinimumBufferSpec(5, 1);
- // bytesPerRow = 0
- // TODO(crbug.com/dawn/520): This behavior is deprecated; remove this case.
- bufferSpec.bytesPerRow = 0;
- EXPECT_DEPRECATION_WARNING(DoTest(textureSpec, bufferSpec, {5, 1, 1}));
-
// bytesPerRow undefined
bufferSpec.bytesPerRow = wgpu::kCopyStrideUndefined;
DoTest(textureSpec, bufferSpec, {5, 1, 1});
}
-
- // bytesPerRow < bytesInACompleteRow
- // TODO(crbug.com/dawn/520): This behavior is deprecated; remove this case.
- {
- BufferSpec bufferSpec = MinimumBufferSpec(259, 1);
- bufferSpec.bytesPerRow = 256;
- EXPECT_DEPRECATION_WARNING(DoTest(textureSpec, bufferSpec, {259, 1, 1}));
- }
}
TEST_P(CopyTests_T2B, StrideSpecialCases) {
@@ -1540,23 +1527,10 @@
{
BufferSpec bufferSpec = MinimumBufferSpec(5, 1);
- // bytesPerRow = 0
- // TODO(crbug.com/dawn/520): This behavior is deprecated; remove this case.
- bufferSpec.bytesPerRow = 0;
- EXPECT_DEPRECATION_WARNING(DoTest(textureSpec, bufferSpec, {5, 1, 1}));
-
// bytesPerRow undefined
bufferSpec.bytesPerRow = wgpu::kCopyStrideUndefined;
DoTest(textureSpec, bufferSpec, {5, 1, 1});
}
-
- // bytesPerRow < bytesInACompleteRow
- // TODO(crbug.com/dawn/520): This behavior is deprecated; remove this case.
- {
- BufferSpec bufferSpec = MinimumBufferSpec(259, 1);
- bufferSpec.bytesPerRow = 256;
- EXPECT_DEPRECATION_WARNING(DoTest(textureSpec, bufferSpec, {259, 1, 1}));
- }
}
TEST_P(CopyTests_B2T, StrideSpecialCases) {
diff --git a/src/tests/end2end/QueueTests.cpp b/src/tests/end2end/QueueTests.cpp
index e151611..41281c8 100644
--- a/src/tests/end2end/QueueTests.cpp
+++ b/src/tests/end2end/QueueTests.cpp
@@ -541,24 +541,10 @@
constexpr wgpu::Extent3D copyExtent = {5, 1, 1};
DataSpec dataSpec = MinimumDataSpec(copyExtent);
- // bytesPerRow = 0
- // TODO(crbug.com/dawn/520): This behavior is deprecated; remove this case.
- dataSpec.bytesPerRow = 0;
- EXPECT_DEPRECATION_WARNING(DoTest(textureSpec, dataSpec, copyExtent));
-
// bytesPerRow undefined
dataSpec.bytesPerRow = wgpu::kCopyStrideUndefined;
DoTest(textureSpec, dataSpec, copyExtent);
}
-
- // bytesPerRow < bytesInACompleteRow
- // TODO(crbug.com/dawn/520): This behavior is deprecated; remove this case.
- {
- constexpr wgpu::Extent3D copyExtent = {259, 1, 1};
- DataSpec dataSpec = MinimumDataSpec(copyExtent);
- dataSpec.bytesPerRow = 256;
- EXPECT_DEPRECATION_WARNING(DoTest(textureSpec, dataSpec, copyExtent));
- }
}
// Test with bytesPerRow greater than needed in a write to a texture array.
diff --git a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
index a96c14f..bd560bd 100644
--- a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
+++ b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
@@ -580,9 +580,8 @@
{0, 1, 4});
// copyHeight = 1 and copyDepth = 1
- // TODO(crbug.com/dawn/520): Change to ::Failure.
- EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 0, 1,
- destination, 0, {0, 0, 0}, {64, 1, 1}));
+ TestB2TCopy(utils::Expectation::Failure, source, 0, 0, 1, destination, 0, {0, 0, 0},
+ {64, 1, 1});
}
// bytes per row is not 256-byte aligned
@@ -613,9 +612,8 @@
{65, 1, 0});
// copyHeight = 1 and copyDepth = 1
- // TODO(crbug.com/dawn/520): Change to ::Failure.
- EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 256, 1,
- destination, 0, {0, 0, 0}, {65, 1, 1}));
+ TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 1, destination, 0, {0, 0, 0},
+ {65, 1, 1});
}
}
@@ -626,11 +624,10 @@
Create2DTexture(16, 16, 1, 5, wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureUsage::CopyDst);
// rowsPerImage is zero
- // TODO(crbug.com/dawn/520): Change to ::Failure.
- EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0,
- destination, 0, {0, 0, 0}, {1, 1, 1}));
- EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0,
- destination, 0, {0, 0, 0}, {4, 4, 1}));
+ TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 0, destination, 0, {0, 0, 0},
+ {1, 1, 1});
+ TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 0, destination, 0, {0, 0, 0},
+ {4, 4, 1});
// rowsPerImage is undefined
TestB2TCopy(utils::Expectation::Success, source, 0, 256, wgpu::kCopyStrideUndefined,
@@ -1198,9 +1195,8 @@
{0, 1, 4});
// copyHeight = 1 and copyDepth = 1
- // TODO(crbug.com/dawn/520): Change to ::Failure.
- EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
- destination, 0, 0, 1, {64, 1, 1}));
+ TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 0, 1,
+ {64, 1, 1});
}
// bytes per row is not 256-byte aligned
@@ -1231,9 +1227,8 @@
{65, 1, 0});
// copyHeight = 1 and copyDepth = 1
- // TODO(crbug.com/dawn/520): Change to ::Failure.
- EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
- destination, 0, 256, 1, {65, 1, 1}));
+ TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 1,
+ {65, 1, 1});
}
}
@@ -1244,11 +1239,10 @@
wgpu::Buffer destination = CreateBuffer(bufferSize, wgpu::BufferUsage::CopyDst);
// rowsPerImage is zero (Valid)
- // TODO(crbug.com/dawn/520): Change to ::Failure.
- EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
- destination, 0, 256, 0, {1, 1, 1}));
- EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
- destination, 0, 256, 0, {4, 4, 1}));
+ TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0,
+ {1, 1, 1});
+ TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0,
+ {4, 4, 1});
// rowsPerImage is undefined
TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, 256,
diff --git a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
index 6d8a2e7..db52141 100644
--- a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
+++ b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
@@ -254,8 +254,7 @@
0, {0, 0, 0}, {0, 1, 2}));
// copyHeight = 1 and copyDepth = 1
- // TODO(crbug.com/dawn/520): Change to ASSERT_DEVICE_ERROR.
- EXPECT_DEPRECATION_WARNING(
+ ASSERT_DEVICE_ERROR(
TestWriteTexture(128, 0, 0, 1, destination, 0, {0, 0, 0}, {3, 1, 1}));
TestWriteTexture(128, 0, wgpu::kCopyStrideUndefined, 1, destination, 0, {0, 0, 0},
{3, 1, 1});
@@ -278,9 +277,7 @@
TestWriteTexture(128, 0, 11, 1, destination, 0, {0, 0, 0}, {3, 1, 0}));
// copyHeight = 1 and copyDepth = 1
- // TODO(crbug.com/dawn/520): Change to ASSERT_DEVICE_ERROR. bytesPerRow used to be only
- // validated if height > 1 || depth > 1.
- EXPECT_DEPRECATION_WARNING(
+ ASSERT_DEVICE_ERROR(
TestWriteTexture(128, 0, 11, 1, destination, 0, {0, 0, 0}, {3, 1, 1}));
}
@@ -312,7 +309,7 @@
// rowsPerImage is less than copy height (Invalid)
ASSERT_DEVICE_ERROR(
TestWriteTexture(dataSize, 0, 256, 3, destination, 0, {0, 0, 0}, {4, 4, 1}));
- EXPECT_DEPRECATION_WARNING(
+ ASSERT_DEVICE_ERROR(
TestWriteTexture(dataSize, 0, 256, 0, destination, 0, {0, 0, 0}, {4, 4, 1}));
}
