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>

src/dawn_native/CommandEncoder.cpp

@@ -693,18 +693,18 @@ namespace dawn_native {
             }
             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()));
+                    source->layout, blockInfo,
-                DAWN_TRY(ValidateLinearTextureData(srcLayout, source->buffer->GetSize(), blockInfo,
+                    destination->texture->GetFormat().HasDepthOrStencil()));
-                                                   *copySize));
+                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 @@ namespace dawn_native {
             }
             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()));
+                    destination->layout, blockInfo,
-                DAWN_TRY(ValidateLinearTextureData(dstLayout, destination->buffer->GetSize(),
+                    source->texture->GetFormat().HasDepthOrStencil()));
-                                                   blockInfo, *copySize));
+                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 =

src/dawn_native/CommandValidation.cpp

@@ -143,45 +143,6 @@ namespace dawn_native {
         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,

src/dawn_native/CommandValidation.h

@@ -36,11 +36,6 @@ namespace dawn_native {
                                                        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);

src/dawn_native/Queue.cpp

@@ -278,13 +278,13 @@ namespace dawn_native {
                                     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 @@ namespace dawn_native {
 
         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 @@ namespace dawn_native {
 
     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 @@ namespace dawn_native {
 
         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 @@ namespace dawn_native {
         const TexelBlockInfo& blockInfo =
             destination->texture->GetFormat().GetAspectInfo(destination->aspect).block;
 
-        TextureDataLayout layout = FixUpDeprecatedTextureDataLayoutOptions(GetDevice(), *dataLayout,
+        DAWN_TRY(ValidateLinearTextureData(dataLayout, dataSize, blockInfo, *writeSize));
-                                                                           blockInfo, *writeSize);
-        DAWN_TRY(ValidateLinearTextureData(layout, dataSize, blockInfo, *writeSize));
 
         DAWN_TRY(destination->texture->ValidateCanUseInSubmitNow());
 

src/dawn_native/Queue.h

@@ -71,7 +71,7 @@ namespace dawn_native {
         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 @@ namespace dawn_native {
                                        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);

src/tests/end2end/CopyTests.cpp

@@ -824,23 +824,10 @@ TEST_P(CopyTests_T2B, BytesPerRowWithOneRowCopy) {
     {
         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 @@ TEST_P(CopyTests_B2T, BytesPerRowWithOneRowCopy) {
     {
         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) {

src/tests/end2end/QueueTests.cpp

@@ -541,24 +541,10 @@ TEST_P(QueueWriteTextureTests, BytesPerRowWithOneRowCopy) {
         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.

src/tests/unittests/validation/CopyCommandsValidationTests.cpp

@@ -580,9 +580,8 @@ TEST_F(CopyCommandTest_B2T, BytesPerRowConstraints) {
                     {0, 1, 4});
 
         // copyHeight = 1 and copyDepth = 1
-        // TODO(crbug.com/dawn/520): Change to ::Failure.
+        TestB2TCopy(utils::Expectation::Failure, source, 0, 0, 1, destination, 0, {0, 0, 0},
-        EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 0, 1,
+                    {64, 1, 1});
-                                               destination, 0, {0, 0, 0}, {64, 1, 1}));
     }
 
     // bytes per row is not 256-byte aligned
@@ -613,9 +612,8 @@ TEST_F(CopyCommandTest_B2T, BytesPerRowConstraints) {
                     {65, 1, 0});
 
         // copyHeight = 1 and copyDepth = 1
-        // TODO(crbug.com/dawn/520): Change to ::Failure.
+        TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 1, destination, 0, {0, 0, 0},
-        EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 256, 1,
+                    {65, 1, 1});
-                                               destination, 0, {0, 0, 0}, {65, 1, 1}));
     }
 }
 
@@ -626,11 +624,10 @@ TEST_F(CopyCommandTest_B2T, RowsPerImageConstraints) {
         Create2DTexture(16, 16, 1, 5, wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureUsage::CopyDst);
 
     // rowsPerImage is zero
-    // TODO(crbug.com/dawn/520): Change to ::Failure.
+    TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 0, destination, 0, {0, 0, 0},
-    EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0,
+                {1, 1, 1});
-                                           destination, 0, {0, 0, 0}, {1, 1, 1}));
+    TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 0, destination, 0, {0, 0, 0},
-    EXPECT_DEPRECATION_WARNING(TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0,
+                {4, 4, 1});
-                                           destination, 0, {0, 0, 0}, {4, 4, 1}));
 
     // rowsPerImage is undefined
     TestB2TCopy(utils::Expectation::Success, source, 0, 256, wgpu::kCopyStrideUndefined,
@@ -1198,9 +1195,8 @@ TEST_F(CopyCommandTest_T2B, BytesPerRowConstraints) {
                     {0, 1, 4});
 
         // copyHeight = 1 and copyDepth = 1
-        // TODO(crbug.com/dawn/520): Change to ::Failure.
+        TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 0, 1,
-        EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
+                    {64, 1, 1});
-                                               destination, 0, 0, 1, {64, 1, 1}));
     }
 
     // bytes per row is not 256-byte aligned
@@ -1231,9 +1227,8 @@ TEST_F(CopyCommandTest_T2B, BytesPerRowConstraints) {
                     {65, 1, 0});
 
         // copyHeight = 1 and copyDepth = 1
-        // TODO(crbug.com/dawn/520): Change to ::Failure.
+        TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 1,
-        EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
+                    {65, 1, 1});
-                                               destination, 0, 256, 1, {65, 1, 1}));
     }
 }
 
@@ -1244,11 +1239,10 @@ TEST_F(CopyCommandTest_T2B, RowsPerImageConstraints) {
     wgpu::Buffer destination = CreateBuffer(bufferSize, wgpu::BufferUsage::CopyDst);
 
     // rowsPerImage is zero (Valid)
-    // TODO(crbug.com/dawn/520): Change to ::Failure.
+    TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0,
-    EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
+                {1, 1, 1});
-                                           destination, 0, 256, 0, {1, 1, 1}));
+    TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, 256, 0,
-    EXPECT_DEPRECATION_WARNING(TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0},
+                {4, 4, 1});
-                                           destination, 0, 256, 0, {4, 4, 1}));
 
     // rowsPerImage is undefined
     TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, 256,

src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp

@@ -254,8 +254,7 @@ namespace {
                                                  0, {0, 0, 0}, {0, 1, 2}));
 
             // copyHeight = 1 and copyDepth = 1
-            // TODO(crbug.com/dawn/520): Change to ASSERT_DEVICE_ERROR.
+            ASSERT_DEVICE_ERROR(
-            EXPECT_DEPRECATION_WARNING(
                 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 @@ namespace {
                 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
+            ASSERT_DEVICE_ERROR(
-            // validated if height > 1 || depth > 1.
-            EXPECT_DEPRECATION_WARNING(
                 TestWriteTexture(128, 0, 11, 1, destination, 0, {0, 0, 0}, {3, 1, 1}));
         }
 
@@ -312,7 +309,7 @@ namespace {
         // 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}));
     }