From ecabfe8a780852ad6c8f24e35d37777d4746ce4f Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 13 May 2020 17:26:05 +0000 Subject: [PATCH] Remove wgpu::BufferCopyView::rowPitch/imageHeight They were deprecated in favor of bytesPerRow and rowsPerImage. Bug: dawn:22 Change-Id: I5bd3262ee8ba2f891d01f6b8a3f5df86f7596686 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21684 Commit-Queue: Corentin Wallez Reviewed-by: Stephen White Reviewed-by: Austin Eng --- dawn.json | 4 +- src/dawn_native/CommandEncoder.cpp | 78 ++++----------- src/tests/end2end/DeprecatedAPITests.cpp | 121 ----------------------- 3 files changed, 19 insertions(+), 184 deletions(-) diff --git a/dawn.json b/dawn.json index d3d7b90d51..35837701c0 100644 --- a/dawn.json +++ b/dawn.json @@ -210,9 +210,7 @@ "members": [ {"name": "buffer", "type": "buffer"}, {"name": "offset", "type": "uint64_t", "default": 0}, - {"name": "row pitch", "type": "uint32_t", "default": 0}, - {"name": "image height", "type": "uint32_t", "default": 0}, - {"name": "bytes per row", "type": "uint32_t", "default": 0}, + {"name": "bytes per row", "type": "uint32_t"}, {"name": "rows per image", "type": "uint32_t", "default": 0} ] }, diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index dbd206965c..31b635b378 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -499,40 +499,6 @@ namespace dawn_native { return {}; } - // TODO(dawn:22): Remove this once users bytesPerRow/rowsPerImage - ResultOrError FixBufferCopyView(DeviceBase* device, - const BufferCopyView* original) { - BufferCopyView fixed = *original; - - if (fixed.rowPitch != 0) { - if (fixed.bytesPerRow != 0) { - return DAWN_VALIDATION_ERROR( - "Cannot use rowPitch and bytesPerRow at the same time"); - } else { - device->EmitDeprecationWarning( - "BufferCopyView::rowPitch is deprecated, use BufferCopyView::bytesPerRow " - "instead"); - fixed.bytesPerRow = fixed.rowPitch; - fixed.rowPitch = 0; - } - } - - if (fixed.imageHeight != 0) { - if (fixed.rowsPerImage != 0) { - return DAWN_VALIDATION_ERROR( - "Cannot use imageHeight and rowsPerImage at the same time"); - } else { - device->EmitDeprecationWarning( - "BufferCopyView::imageHeight is deprecated, use " - "BufferCopyView::rowsPerImage instead"); - fixed.rowsPerImage = fixed.imageHeight; - fixed.imageHeight = 0; - } - } - - return fixed; - } - } // namespace CommandEncoder::CommandEncoder(DeviceBase* device, const CommandEncoderDescriptor*) @@ -690,24 +656,20 @@ namespace dawn_native { const TextureCopyView* destination, const Extent3D* copySize) { mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { - // TODO(dawn:22): Remove this once users bytesPerRow/rowsPerImage - BufferCopyView fixedSource; - DAWN_TRY_ASSIGN(fixedSource, FixBufferCopyView(GetDevice(), source)); - // Validate objects before doing the defaulting. if (GetDevice()->IsValidationEnabled()) { - DAWN_TRY(GetDevice()->ValidateObject(fixedSource.buffer)); + DAWN_TRY(GetDevice()->ValidateObject(source->buffer)); DAWN_TRY(GetDevice()->ValidateObject(destination->texture)); } // Compute default values for bytesPerRow/rowsPerImage - uint32_t defaultedBytesPerRow = fixedSource.bytesPerRow; + uint32_t defaultedBytesPerRow = source->bytesPerRow; if (defaultedBytesPerRow == 0) { defaultedBytesPerRow = ComputeDefaultBytesPerRow(destination->texture->GetFormat(), copySize->width); } - uint32_t defaultedRowsPerImage = fixedSource.rowsPerImage; + uint32_t defaultedRowsPerImage = source->rowsPerImage; if (defaultedRowsPerImage == 0) { defaultedRowsPerImage = copySize->height; } @@ -731,21 +693,21 @@ namespace dawn_native { &bufferCopySize)); DAWN_TRY(ValidateCopySizeFitsInTexture(*destination, *copySize)); - DAWN_TRY(ValidateCopySizeFitsInBuffer(fixedSource, bufferCopySize)); - DAWN_TRY(ValidateTexelBufferOffset(fixedSource, destination->texture->GetFormat())); + DAWN_TRY(ValidateCopySizeFitsInBuffer(*source, bufferCopySize)); + DAWN_TRY(ValidateTexelBufferOffset(*source, destination->texture->GetFormat())); - DAWN_TRY(ValidateCanUseAs(fixedSource.buffer, wgpu::BufferUsage::CopySrc)); + DAWN_TRY(ValidateCanUseAs(source->buffer, wgpu::BufferUsage::CopySrc)); DAWN_TRY(ValidateCanUseAs(destination->texture, wgpu::TextureUsage::CopyDst)); - mTopLevelBuffers.insert(fixedSource.buffer); + mTopLevelBuffers.insert(source->buffer); mTopLevelTextures.insert(destination->texture); } // Record the copy command. CopyBufferToTextureCmd* copy = allocator->Allocate(Command::CopyBufferToTexture); - copy->source.buffer = fixedSource.buffer; - copy->source.offset = fixedSource.offset; + copy->source.buffer = source->buffer; + copy->source.offset = source->offset; copy->source.bytesPerRow = defaultedBytesPerRow; copy->source.rowsPerImage = defaultedRowsPerImage; copy->destination.texture = destination->texture; @@ -762,24 +724,20 @@ namespace dawn_native { const BufferCopyView* destination, const Extent3D* copySize) { mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { - // TODO(dawn:22): Remove this once users bytesPerRow/rowsPerImage - BufferCopyView fixedDestination; - DAWN_TRY_ASSIGN(fixedDestination, FixBufferCopyView(GetDevice(), destination)); - // Validate objects before doing the defaulting. if (GetDevice()->IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(source->texture)); - DAWN_TRY(GetDevice()->ValidateObject(fixedDestination.buffer)); + DAWN_TRY(GetDevice()->ValidateObject(destination->buffer)); } // Compute default values for bytesPerRow/rowsPerImage - uint32_t defaultedBytesPerRow = fixedDestination.bytesPerRow; + uint32_t defaultedBytesPerRow = destination->bytesPerRow; if (defaultedBytesPerRow == 0) { defaultedBytesPerRow = ComputeDefaultBytesPerRow(source->texture->GetFormat(), copySize->width); } - uint32_t defaultedRowsPerImage = fixedDestination.rowsPerImage; + uint32_t defaultedRowsPerImage = destination->rowsPerImage; if (defaultedRowsPerImage == 0) { defaultedRowsPerImage = copySize->height; } @@ -801,14 +759,14 @@ namespace dawn_native { &bufferCopySize)); DAWN_TRY(ValidateCopySizeFitsInTexture(*source, *copySize)); - DAWN_TRY(ValidateCopySizeFitsInBuffer(fixedDestination, bufferCopySize)); - DAWN_TRY(ValidateTexelBufferOffset(fixedDestination, source->texture->GetFormat())); + DAWN_TRY(ValidateCopySizeFitsInBuffer(*destination, bufferCopySize)); + DAWN_TRY(ValidateTexelBufferOffset(*destination, source->texture->GetFormat())); DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::CopySrc)); - DAWN_TRY(ValidateCanUseAs(fixedDestination.buffer, wgpu::BufferUsage::CopyDst)); + DAWN_TRY(ValidateCanUseAs(destination->buffer, wgpu::BufferUsage::CopyDst)); mTopLevelTextures.insert(source->texture); - mTopLevelBuffers.insert(fixedDestination.buffer); + mTopLevelBuffers.insert(destination->buffer); } // Record the copy command. @@ -819,8 +777,8 @@ namespace dawn_native { copy->copySize = *copySize; copy->source.mipLevel = source->mipLevel; copy->source.arrayLayer = source->arrayLayer; - copy->destination.buffer = fixedDestination.buffer; - copy->destination.offset = fixedDestination.offset; + copy->destination.buffer = destination->buffer; + copy->destination.offset = destination->offset; copy->destination.bytesPerRow = defaultedBytesPerRow; copy->destination.rowsPerImage = defaultedRowsPerImage; diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index a3ded8d770..0c29933b61 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/src/tests/end2end/DeprecatedAPITests.cpp @@ -56,127 +56,6 @@ class DeprecationTests : public DawnTest { } \ } while (0) -// Tests for BufferCopyView.rowPitch/imageHeight -> bytesPerRow/rowsPerImage - -class BufferCopyViewDeprecationTests : public DeprecationTests { - protected: - void TestSetUp() override { - DeprecationTests::TestSetUp(); - - wgpu::BufferDescriptor bufferDesc; - bufferDesc.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst; - bufferDesc.size = kTextureBytesPerRowAlignment * 2; - buffer = device.CreateBuffer(&bufferDesc); - - wgpu::TextureDescriptor textureDesc; - textureDesc.usage = wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst; - textureDesc.size = {2, 2, 1}; - textureDesc.format = wgpu::TextureFormat::RGBA8Unorm; - texture = device.CreateTexture(&textureDesc); - } - - enum CopyType { - B2T, - T2B, - }; - void DoCopy(CopyType type, const wgpu::BufferCopyView& bufferView) { - wgpu::TextureCopyView textureCopyView; - textureCopyView.texture = texture; - textureCopyView.mipLevel = 0; - textureCopyView.arrayLayer = 0; - textureCopyView.origin = {0, 0}; - wgpu::Extent3D copySize = {2, 2, 1}; - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - switch (type) { - case B2T: - encoder.CopyBufferToTexture(&bufferView, &textureCopyView, ©Size); - break; - case T2B: - encoder.CopyTextureToBuffer(&textureCopyView, &bufferView, ©Size); - break; - } - encoder.Finish(); - } - - wgpu::Buffer buffer; - wgpu::Texture texture; -}; - -// Test that using rowPitch produces a deprecation warning. -TEST_P(BufferCopyViewDeprecationTests, RowPitchIsDeprecated) { - wgpu::BufferCopyView view; - view.buffer = buffer; - view.rowPitch = 256; - EXPECT_DEPRECATION_WARNING(DoCopy(B2T, view)); - EXPECT_DEPRECATION_WARNING(DoCopy(T2B, view)); -} - -// Test that using imageHeight produces a deprecation warning. -TEST_P(BufferCopyViewDeprecationTests, ImageHeightIsDeprecated) { - wgpu::BufferCopyView view; - view.buffer = buffer; - view.imageHeight = 2; - view.bytesPerRow = 256; - EXPECT_DEPRECATION_WARNING(DoCopy(B2T, view)); - EXPECT_DEPRECATION_WARNING(DoCopy(T2B, view)); -} - -// Test that using both rowPitch and bytesPerRow produces a validation error. -TEST_P(BufferCopyViewDeprecationTests, BothRowPitchAndBytesPerRowIsInvalid) { - wgpu::BufferCopyView view; - view.buffer = buffer; - view.rowPitch = 256; - view.bytesPerRow = 256; - ASSERT_DEVICE_ERROR(DoCopy(B2T, view)); - ASSERT_DEVICE_ERROR(DoCopy(T2B, view)); -} - -// Test that using both imageHeight and rowsPerImage produces a validation error. -TEST_P(BufferCopyViewDeprecationTests, BothImageHeightAndRowsPerImageIsInvalid) { - wgpu::BufferCopyView view; - view.buffer = buffer; - view.imageHeight = 2; - view.bytesPerRow = 256; - view.rowsPerImage = 2; - ASSERT_DEVICE_ERROR(DoCopy(B2T, view)); - ASSERT_DEVICE_ERROR(DoCopy(T2B, view)); -} - -// Test that rowPitch is correctly taken into account for validation -TEST_P(BufferCopyViewDeprecationTests, RowPitchTakenIntoAccountForValidation) { - wgpu::BufferCopyView view; - view.buffer = buffer; - view.rowPitch = 256; - EXPECT_DEPRECATION_WARNING(DoCopy(B2T, view)); - EXPECT_DEPRECATION_WARNING(DoCopy(T2B, view)); - - view.rowPitch = 128; - ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(DoCopy(B2T, view))); - ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(DoCopy(T2B, view))); -} - -// Test that imageHeight is correctly taken into account for validation -TEST_P(BufferCopyViewDeprecationTests, ImageHeightTakenIntoAccountForValidation) { - wgpu::BufferCopyView view; - view.buffer = buffer; - view.imageHeight = 2; - view.bytesPerRow = 256; - EXPECT_DEPRECATION_WARNING(DoCopy(B2T, view)); - EXPECT_DEPRECATION_WARNING(DoCopy(T2B, view)); - - view.imageHeight = 1; - ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(DoCopy(B2T, view))); - ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(DoCopy(T2B, view))); -} - -DAWN_INSTANTIATE_TEST(BufferCopyViewDeprecationTests, - D3D12Backend(), - MetalBackend(), - NullBackend(), - OpenGLBackend(), - VulkanBackend()); - DAWN_INSTANTIATE_TEST(DeprecationTests, D3D12Backend(), MetalBackend(),