Add BufferCopyView.layout and deprecate old fields

Bug: dawn:22
Change-Id: Idc37faf5edaa01a6fb1a38d1a81022ad652cf428
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24622
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>

dawn.json

@@ -217,6 +217,7 @@
         "category": "structure",
         "extensible": true,
         "members": [
+            {"name": "layout", "type": "texture data layout"},
             {"name": "buffer", "type": "buffer"},
             {"name": "offset", "type": "uint64_t", "default": 0},
             {"name": "bytes per row", "type": "uint32_t"},

src/dawn_native/CommandEncoder.cpp

@@ -406,6 +406,35 @@ namespace dawn_native {
             return fixedView;
         }
 
+        ResultOrError<BufferCopyView> FixBufferCopyView(DeviceBase* device,
+                                                        const BufferCopyView* view) {
+            BufferCopyView fixedView = *view;
+
+            TextureDataLayout& layout = fixedView.layout;
+            if (layout.offset != 0 || layout.bytesPerRow != 0 || layout.rowsPerImage != 0) {
+                // Using non-deprecated path
+                if (fixedView.offset != 0 || fixedView.bytesPerRow != 0 ||
+                    fixedView.rowsPerImage != 0) {
+                    return DAWN_VALIDATION_ERROR(
+                        "WGPUBufferCopyView.offset/bytesPerRow/rowsPerImage is deprecated; use "
+                        "only WGPUBufferCopyView.layout");
+                }
+            } else if (fixedView.offset != 0 || fixedView.bytesPerRow != 0 ||
+                       fixedView.rowsPerImage != 0) {
+                device->EmitDeprecationWarning(
+                    "WGPUBufferCopyView.offset/bytesPerRow/rowsPerImage is deprecated; use "
+                    "WGPUBufferCopyView.layout");
+
+                layout.offset = fixedView.offset;
+                layout.bytesPerRow = fixedView.bytesPerRow;
+                layout.rowsPerImage = fixedView.rowsPerImage;
+                fixedView.offset = 0;
+                fixedView.bytesPerRow = 0;
+                fixedView.rowsPerImage = 0;
+            }
+            return fixedView;
+        }
+
     }  // namespace
 
     CommandEncoder::CommandEncoder(DeviceBase* device, const CommandEncoderDescriptor*)
@@ -570,12 +599,17 @@ namespace dawn_native {
                                              const TextureCopyView* destination,
                                              const Extent3D* copySize) {
         mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
-            // TODO(dawn:22): Remove once migration from GPUTextureCopyView.arrayLayer to
+            // TODO(crbug.com/dawn/22): Remove once migration from GPUTextureCopyView.arrayLayer to
             // GPUTextureCopyView.origin.z is done.
             TextureCopyView fixedDest;
             DAWN_TRY_ASSIGN(fixedDest, FixTextureCopyView(GetDevice(), destination));
             destination = &fixedDest;
 
+            // TODO(crbug.com/dawn/22): Remove once migration to .layout is done.
+            BufferCopyView fixedSource;
+            DAWN_TRY_ASSIGN(fixedSource, FixBufferCopyView(GetDevice(), source));
+            source = &fixedSource;
+
             if (GetDevice()->IsValidationEnabled()) {
                 DAWN_TRY(ValidateBufferCopyView(GetDevice(), *source));
                 DAWN_TRY(ValidateCanUseAs(source->buffer, wgpu::BufferUsage::CopySrc));
@@ -584,13 +618,7 @@ namespace dawn_native {
                 DAWN_TRY(ValidateCanUseAs(destination->texture, wgpu::TextureUsage::CopyDst));
                 DAWN_TRY(ValidateTextureSampleCountInCopyCommands(destination->texture));
 
-                TextureDataLayout sourceAsTextureDataLayout;
+                DAWN_TRY(ValidateLinearTextureData(source->layout, source->buffer->GetSize(),
-                sourceAsTextureDataLayout.offset = source->offset;
-                sourceAsTextureDataLayout.bytesPerRow = source->bytesPerRow;
-                sourceAsTextureDataLayout.rowsPerImage = source->rowsPerImage;
-
-                DAWN_TRY(ValidateLinearTextureData(sourceAsTextureDataLayout,
-                                                   source->buffer->GetSize(),
                                                    destination->texture->GetFormat(), *copySize));
                 DAWN_TRY(ValidateTextureCopyRange(*destination, *copySize));
 
@@ -599,7 +627,7 @@ namespace dawn_native {
             }
 
             // Compute default value for rowsPerImage
-            uint32_t defaultedRowsPerImage = source->rowsPerImage;
+            uint32_t defaultedRowsPerImage = source->layout.rowsPerImage;
             if (defaultedRowsPerImage == 0) {
                 defaultedRowsPerImage = copySize->height;
             }
@@ -608,8 +636,8 @@ namespace dawn_native {
             CopyBufferToTextureCmd* copy =
                 allocator->Allocate<CopyBufferToTextureCmd>(Command::CopyBufferToTexture);
             copy->source.buffer = source->buffer;
-            copy->source.offset = source->offset;
+            copy->source.offset = source->layout.offset;
-            copy->source.bytesPerRow = source->bytesPerRow;
+            copy->source.bytesPerRow = source->layout.bytesPerRow;
             copy->source.rowsPerImage = defaultedRowsPerImage;
             copy->destination.texture = destination->texture;
             copy->destination.origin = destination->origin;
@@ -624,12 +652,17 @@ namespace dawn_native {
                                              const BufferCopyView* destination,
                                              const Extent3D* copySize) {
         mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
-            // TODO(dawn:22): Remove once migration from GPUTextureCopyView.arrayLayer to
+            // TODO(crbug.com/dawn/22): Remove once migration from GPUTextureCopyView.arrayLayer to
             // GPUTextureCopyView.origin.z is done.
             TextureCopyView fixedSrc;
             DAWN_TRY_ASSIGN(fixedSrc, FixTextureCopyView(GetDevice(), source));
             source = &fixedSrc;
 
+            // TODO(crbug.com/dawn/22): Remove once migration to .layout is done.
+            BufferCopyView fixedDst;
+            DAWN_TRY_ASSIGN(fixedDst, FixBufferCopyView(GetDevice(), destination));
+            destination = &fixedDst;
+
             if (GetDevice()->IsValidationEnabled()) {
                 DAWN_TRY(ValidateTextureCopyView(GetDevice(), *source));
                 DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::CopySrc));
@@ -638,12 +671,7 @@ namespace dawn_native {
                 DAWN_TRY(ValidateBufferCopyView(GetDevice(), *destination));
                 DAWN_TRY(ValidateCanUseAs(destination->buffer, wgpu::BufferUsage::CopyDst));
 
-                TextureDataLayout dstAsTextureDataLayout;
+                DAWN_TRY(ValidateLinearTextureData(destination->layout,
-                dstAsTextureDataLayout.offset = destination->offset;
-                dstAsTextureDataLayout.bytesPerRow = destination->bytesPerRow;
-                dstAsTextureDataLayout.rowsPerImage = destination->rowsPerImage;
-
-                DAWN_TRY(ValidateLinearTextureData(dstAsTextureDataLayout,
                                                    destination->buffer->GetSize(),
                                                    source->texture->GetFormat(), *copySize));
                 DAWN_TRY(ValidateTextureCopyRange(*source, *copySize));
@@ -653,7 +681,7 @@ namespace dawn_native {
             }
 
             // Compute default value for rowsPerImage
-            uint32_t defaultedRowsPerImage = destination->rowsPerImage;
+            uint32_t defaultedRowsPerImage = destination->layout.rowsPerImage;
             if (defaultedRowsPerImage == 0) {
                 defaultedRowsPerImage = copySize->height;
             }
@@ -665,8 +693,8 @@ namespace dawn_native {
             copy->source.origin = source->origin;
             copy->source.mipLevel = source->mipLevel;
             copy->destination.buffer = destination->buffer;
-            copy->destination.offset = destination->offset;
+            copy->destination.offset = destination->layout.offset;
-            copy->destination.bytesPerRow = destination->bytesPerRow;
+            copy->destination.bytesPerRow = destination->layout.bytesPerRow;
             copy->destination.rowsPerImage = defaultedRowsPerImage;
             copy->copySize = *copySize;
 
@@ -678,7 +706,7 @@ namespace dawn_native {
                                               const TextureCopyView* destination,
                                               const Extent3D* copySize) {
         mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
-            // TODO(dawn:22): Remove once migration from GPUTextureCopyView.arrayLayer to
+            // TODO(crbug.com/dawn/22): Remove once migration from GPUTextureCopyView.arrayLayer to
             // GPUTextureCopyView.origin.z is done.
             TextureCopyView fixedSrc;
             DAWN_TRY_ASSIGN(fixedSrc, FixTextureCopyView(GetDevice(), source));

src/dawn_native/CommandValidation.cpp

@@ -458,10 +458,15 @@ namespace dawn_native {
 
     MaybeError ValidateBufferCopyView(DeviceBase const* device,
                                       const BufferCopyView& bufferCopyView) {
+        // Should have already been fixed up to not use deprecated fields.
+        ASSERT(bufferCopyView.offset == 0 && bufferCopyView.bytesPerRow == 0 &&
+               bufferCopyView.rowsPerImage == 0);
+
         DAWN_TRY(device->ValidateObject(bufferCopyView.buffer));
-        if (bufferCopyView.bytesPerRow % kTextureBytesPerRowAlignment != 0) {
+        if (bufferCopyView.layout.bytesPerRow % kTextureBytesPerRowAlignment != 0) {
             return DAWN_VALIDATION_ERROR("bytesPerRow must be a multiple of 256");
         }
+
         return {};
     }
 

src/dawn_native/CommandValidation.h

@@ -48,7 +48,7 @@ namespace dawn_native {
                                         const Extent3D& copySize);
 
     MaybeError ValidateBufferCopyView(DeviceBase const* device,
-                                      const BufferCopyView& bufferCopyView);
+                                      const BufferCopyView& bufferCopyViewt);
     MaybeError ValidateTextureCopyView(DeviceBase const* device,
                                        const TextureCopyView& textureCopyView);
 

src/tests/end2end/D3D12ResourceWrappingTests.cpp

@@ -266,17 +266,8 @@ class D3D12SharedHandleUsageTests : public D3D12ResourceTestBase {
   protected:
     // Submits a 1x1x1 copy from source to destination
     void SimpleCopyTextureToTexture(wgpu::Texture source, wgpu::Texture destination) {
-        wgpu::TextureCopyView copySrc;
+        wgpu::TextureCopyView copySrc = utils::CreateTextureCopyView(source, 0, {0, 0, 0});
-        copySrc.texture = source;
+        wgpu::TextureCopyView copyDst = utils::CreateTextureCopyView(destination, 0, {0, 0, 0});
-        copySrc.mipLevel = 0;
-        copySrc.arrayLayer = 0;
-        copySrc.origin = {0, 0, 0};
-
-        wgpu::TextureCopyView copyDst;
-        copyDst.texture = destination;
-        copyDst.mipLevel = 0;
-        copyDst.arrayLayer = 0;
-        copyDst.origin = {0, 0, 0};
 
         wgpu::Extent3D copySize = {1, 1, 1};
 

src/tests/end2end/DeprecatedAPITests.cpp

@@ -173,9 +173,9 @@ class TextureCopyViewArrayLayerDeprecationTests : public DeprecationTests {
         desc.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst;
         desc.size = 4;
 
-        wgpu::BufferCopyView copy;
+        wgpu::BufferCopyView copy = {};
         copy.buffer = device.CreateBuffer(&desc);
-        copy.bytesPerRow = kTextureBytesPerRowAlignment;
+        copy.layout.bytesPerRow = kTextureBytesPerRowAlignment;
         return copy;
     }
 
@@ -251,3 +251,115 @@ DAWN_INSTANTIATE_TEST(TextureCopyViewArrayLayerDeprecationTests,
                       NullBackend(),
                       OpenGLBackend(),
                       VulkanBackend());
+
+class BufferCopyViewDeprecationTests : public DeprecationTests {
+  protected:
+    wgpu::TextureCopyView MakeTextureCopyView() {
+        wgpu::TextureDescriptor desc = {};
+        desc.usage = wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst;
+        desc.dimension = wgpu::TextureDimension::e2D;
+        desc.size = {1, 1, 2};
+        desc.format = wgpu::TextureFormat::RGBA8Unorm;
+
+        wgpu::TextureCopyView copy;
+        copy.texture = device.CreateTexture(&desc);
+        copy.arrayLayer = 0;
+        copy.origin = {0, 0, 1};
+        return copy;
+    }
+
+    wgpu::Extent3D copySize = {1, 1, 1};
+};
+
+// Test that using BufferCopyView::{offset,bytesPerRow,rowsPerImage} emits a warning.
+TEST_P(BufferCopyViewDeprecationTests, DeprecationWarning) {
+    wgpu::BufferDescriptor desc;
+    desc.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst;
+    desc.size = 8;
+    wgpu::Buffer buffer = device.CreateBuffer(&desc);
+
+    wgpu::TextureCopyView texCopy = MakeTextureCopyView();
+
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::BufferCopyView bufCopy = {};
+        bufCopy.buffer = buffer;
+        bufCopy.offset = 4;
+        EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texCopy, &copySize));
+        EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texCopy, &bufCopy, &copySize));
+        // Since bytesPerRow is 0
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::BufferCopyView bufCopy = {};
+        bufCopy.buffer = buffer;
+        bufCopy.bytesPerRow = kTextureBytesPerRowAlignment;
+        EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texCopy, &copySize));
+        EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texCopy, &bufCopy, &copySize));
+        wgpu::CommandBuffer command = encoder.Finish();
+        queue.Submit(1, &command);
+    }
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::BufferCopyView bufCopy = {};
+        bufCopy.buffer = buffer;
+        bufCopy.rowsPerImage = 1;
+        EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texCopy, &copySize));
+        EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texCopy, &bufCopy, &copySize));
+        // Since bytesPerRow is 0
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+}
+
+// Test that using both any old field and any new field is an error
+TEST_P(BufferCopyViewDeprecationTests, BothOldAndNew) {
+    wgpu::BufferDescriptor desc;
+    desc.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst;
+    desc.size = 8;
+    wgpu::Buffer buffer = device.CreateBuffer(&desc);
+
+    wgpu::TextureCopyView texCopy = MakeTextureCopyView();
+
+    auto testOne = [=](const wgpu::BufferCopyView& bufCopy) {
+        {
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            encoder.CopyBufferToTexture(&bufCopy, &texCopy, &copySize);
+            ASSERT_DEVICE_ERROR(encoder.Finish());
+        }
+        {
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            encoder.CopyTextureToBuffer(&texCopy, &bufCopy, &copySize);
+            ASSERT_DEVICE_ERROR(encoder.Finish());
+        }
+    };
+
+    {
+        wgpu::BufferCopyView bufCopy = {};
+        bufCopy.buffer = buffer;
+        bufCopy.layout.bytesPerRow = kTextureBytesPerRowAlignment;
+        {
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            encoder.CopyBufferToTexture(&bufCopy, &texCopy, &copySize);
+            encoder.CopyTextureToBuffer(&texCopy, &bufCopy, &copySize);
+            wgpu::CommandBuffer command = encoder.Finish();
+            queue.Submit(1, &command);
+        }
+
+        bufCopy.offset = 4;
+        testOne(bufCopy);
+        bufCopy.offset = 0;
+        bufCopy.bytesPerRow = kTextureBytesPerRowAlignment;
+        testOne(bufCopy);
+        bufCopy.bytesPerRow = 0;
+        bufCopy.rowsPerImage = 1;
+        testOne(bufCopy);
+    }
+}
+
+DAWN_INSTANTIATE_TEST(BufferCopyViewDeprecationTests,
+                      D3D12Backend(),
+                      MetalBackend(),
+                      NullBackend(),
+                      OpenGLBackend(),
+                      VulkanBackend());

src/tests/white_box/VulkanImageWrappingTestsDmaBuf.cpp

@@ -313,17 +313,8 @@ namespace dawn_native { namespace vulkan {
                                         wgpu::Queue dawnQueue,
                                         wgpu::Texture source,
                                         wgpu::Texture destination) {
-            wgpu::TextureCopyView copySrc;
+            wgpu::TextureCopyView copySrc = utils::CreateTextureCopyView(source, 0, {0, 0, 0});
-            copySrc.texture = source;
+            wgpu::TextureCopyView copyDst = utils::CreateTextureCopyView(destination, 0, {0, 0, 0});
-            copySrc.mipLevel = 0;
-            copySrc.arrayLayer = 0;
-            copySrc.origin = {0, 0, 0};
-
-            wgpu::TextureCopyView copyDst;
-            copyDst.texture = destination;
-            copyDst.mipLevel = 0;
-            copyDst.arrayLayer = 0;
-            copyDst.origin = {0, 0, 0};
 
             wgpu::Extent3D copySize = {1, 1, 1};
 
@@ -526,17 +517,9 @@ namespace dawn_native { namespace vulkan {
         wgpu::Buffer copyDstBuffer = device.CreateBuffer(&bufferDesc);
 
         // Copy |deviceWrappedTexture| into |copyDstBuffer|
-        wgpu::TextureCopyView copySrc;
+        wgpu::TextureCopyView copySrc =
-        copySrc.texture = deviceWrappedTexture;
+            utils::CreateTextureCopyView(deviceWrappedTexture, 0, {0, 0, 0});
-        copySrc.mipLevel = 0;
+        wgpu::BufferCopyView copyDst = utils::CreateBufferCopyView(copyDstBuffer, 0, 256, 0);
-        copySrc.arrayLayer = 0;
-        copySrc.origin = {0, 0, 0};
-
-        wgpu::BufferCopyView copyDst;
-        copyDst.buffer = copyDstBuffer;
-        copyDst.offset = 0;
-        copyDst.bytesPerRow = 256;
-        copyDst.rowsPerImage = 0;
 
         wgpu::Extent3D copySize = {1, 1, 1};
 
@@ -584,17 +567,9 @@ namespace dawn_native { namespace vulkan {
             utils::CreateBufferFromData(secondDevice, wgpu::BufferUsage::CopySrc, {0x04030201});
 
         // Copy |copySrcBuffer| into |secondDeviceWrappedTexture|
-        wgpu::BufferCopyView copySrc;
+        wgpu::BufferCopyView copySrc = utils::CreateBufferCopyView(copySrcBuffer, 0, 256, 0);
-        copySrc.buffer = copySrcBuffer;
+        wgpu::TextureCopyView copyDst =
-        copySrc.offset = 0;
+            utils::CreateTextureCopyView(secondDeviceWrappedTexture, 0, {0, 0, 0});
-        copySrc.bytesPerRow = 256;
-        copySrc.rowsPerImage = 0;
-
-        wgpu::TextureCopyView copyDst;
-        copyDst.texture = secondDeviceWrappedTexture;
-        copyDst.mipLevel = 0;
-        copyDst.arrayLayer = 0;
-        copyDst.origin = {0, 0, 0};
 
         wgpu::Extent3D copySize = {1, 1, 1};
 

src/tests/white_box/VulkanImageWrappingTestsOpaqueFD.cpp

@@ -669,17 +669,9 @@ namespace dawn_native { namespace vulkan {
         wgpu::Buffer copyDstBuffer = device.CreateBuffer(&bufferDesc);
 
         // Copy |deviceWrappedTexture| into |copyDstBuffer|
-        wgpu::TextureCopyView copySrc;
+        wgpu::TextureCopyView copySrc =
-        copySrc.texture = deviceWrappedTexture;
+            utils::CreateTextureCopyView(deviceWrappedTexture, 0, {0, 0, 0});
-        copySrc.mipLevel = 0;
+        wgpu::BufferCopyView copyDst = utils::CreateBufferCopyView(copyDstBuffer, 0, 256, 0);
-        copySrc.arrayLayer = 0;
-        copySrc.origin = {0, 0, 0};
-
-        wgpu::BufferCopyView copyDst;
-        copyDst.buffer = copyDstBuffer;
-        copyDst.offset = 0;
-        copyDst.bytesPerRow = 256;
-        copyDst.rowsPerImage = 0;
 
         wgpu::Extent3D copySize = {1, 1, 1};
 
@@ -731,17 +723,9 @@ namespace dawn_native { namespace vulkan {
             utils::CreateBufferFromData(secondDevice, wgpu::BufferUsage::CopySrc, {0x04030201});
 
         // Copy |copySrcBuffer| into |secondDeviceWrappedTexture|
-        wgpu::BufferCopyView copySrc;
+        wgpu::BufferCopyView copySrc = utils::CreateBufferCopyView(copySrcBuffer, 0, 256, 0);
-        copySrc.buffer = copySrcBuffer;
+        wgpu::TextureCopyView copyDst =
-        copySrc.offset = 0;
+            utils::CreateTextureCopyView(secondDeviceWrappedTexture, 0, {0, 0, 0});
-        copySrc.bytesPerRow = 256;
-        copySrc.rowsPerImage = 0;
-
-        wgpu::TextureCopyView copyDst;
-        copyDst.texture = secondDeviceWrappedTexture;
-        copyDst.mipLevel = 0;
-        copyDst.arrayLayer = 0;
-        copyDst.origin = {0, 0, 0};
 
         wgpu::Extent3D copySize = {1, 1, 1};
 

src/utils/WGPUHelpers.cpp

@@ -267,11 +267,11 @@ namespace utils {
                                               uint64_t offset,
                                               uint32_t bytesPerRow,
                                               uint32_t rowsPerImage) {
-        wgpu::BufferCopyView bufferCopyView;
+        wgpu::BufferCopyView bufferCopyView = {};
         bufferCopyView.buffer = buffer;
-        bufferCopyView.offset = offset;
+        bufferCopyView.layout.offset = offset;
-        bufferCopyView.bytesPerRow = bytesPerRow;
+        bufferCopyView.layout.bytesPerRow = bytesPerRow;
-        bufferCopyView.rowsPerImage = rowsPerImage;
+        bufferCopyView.layout.rowsPerImage = rowsPerImage;
 
         return bufferCopyView;
     }