From eac38cfbc1dee9080a31350c43a538e9f0e8f372 Mon Sep 17 00:00:00 2001 From: Tomek Ponitka Date: Mon, 3 Aug 2020 09:11:52 +0000 Subject: [PATCH] Fixing a deprecated arrayLayer value inside Queue::WriteTexture Chromium always sends arrayLayer instead of origin.depth since the migration hasn't been finished yet. This wasn't caught in Dawn's testing since we are using origin.depth everywhere. Bug: dawn:483 Change-Id: I13b1ccfb016eea01a3291ca439457db09966e9a3 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/26160 Reviewed-by: Austin Eng Reviewed-by: Corentin Wallez Commit-Queue: Tomek Ponitka --- src/dawn_native/CommandEncoder.cpp | 19 -------------- src/dawn_native/Queue.cpp | 6 +++++ src/dawn_native/Texture.cpp | 20 +++++++++++++++ src/dawn_native/Texture.h | 3 +++ src/tests/end2end/DeprecatedAPITests.cpp | 32 ++++++++++++++++++++---- 5 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 71a66fce4a..1d049b5bb7 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -442,25 +442,6 @@ namespace dawn_native { return {}; } - ResultOrError FixTextureCopyView(DeviceBase* device, - const TextureCopyView* view) { - TextureCopyView fixedView = *view; - - if (view->arrayLayer != 0) { - if (view->origin.z != 0) { - return DAWN_VALIDATION_ERROR("arrayLayer and origin.z cannot both be != 0"); - } else { - fixedView.origin.z = fixedView.arrayLayer; - fixedView.arrayLayer = 1; - device->EmitDeprecationWarning( - "wgpu::TextureCopyView::arrayLayer is deprecated in favor of " - "::origin::z"); - } - } - - return fixedView; - } - ResultOrError FixBufferCopyView(DeviceBase* device, const BufferCopyView* view) { BufferCopyView fixedView = *view; diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index 900b92a553..9799842774 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -146,6 +146,12 @@ namespace dawn_native { size_t dataSize, const TextureDataLayout* dataLayout, const Extent3D* writeSize) { + // 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; + DAWN_TRY(ValidateWriteTexture(destination, dataSize, dataLayout, writeSize)); if (writeSize->width == 0 || writeSize->height == 0 || writeSize->depth == 0) { diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index faddc5b6c5..df46321826 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -649,4 +649,24 @@ namespace dawn_native { ASSERT(!IsError()); return mRange; } + + ResultOrError FixTextureCopyView(DeviceBase* device, + const TextureCopyView* view) { + TextureCopyView fixedView = *view; + + if (view->arrayLayer != 0) { + if (view->origin.z != 0) { + return DAWN_VALIDATION_ERROR("arrayLayer and origin.z cannot both be != 0"); + } else { + fixedView.origin.z = fixedView.arrayLayer; + fixedView.arrayLayer = 1; + device->EmitDeprecationWarning( + "wgpu::TextureCopyView::arrayLayer is deprecated in favor of " + "::origin::z"); + } + } + + return fixedView; + } + } // namespace dawn_native diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index 300b370383..6c027e1747 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -177,6 +177,9 @@ namespace dawn_native { SubresourceRange mRange; }; + ResultOrError FixTextureCopyView(DeviceBase* device, + const TextureCopyView* view); + } // namespace dawn_native #endif // DAWNNATIVE_TEXTURE_H_ diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index f0783cf13f..6542c54f70 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/src/tests/end2end/DeprecatedAPITests.cpp @@ -207,9 +207,17 @@ TEST_P(TextureCopyViewArrayLayerDeprecationTests, DeprecationWarning) { EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToTexture(&texNewCopy, &texOldCopy, ©Size)); EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texOldCopy, &bufCopy, ©Size)); EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToTexture(&texOldCopy, &texNewCopy, ©Size)); + wgpu::CommandBuffer command = encoder.Finish(); queue.Submit(1, &command); + + // TODO(dawn:483): Add other backends after implementing WriteTexture in them. + if (IsMetal() || IsVulkan()) { + std::vector data = {1}; + EXPECT_DEPRECATION_WARNING( + queue.WriteTexture(&texOldCopy, data.data(), 4, &bufCopy.layout, ©Size)); + } } // Test that using both TextureCopyView::arrayLayer and origin.z is an error. @@ -233,30 +241,44 @@ TEST_P(TextureCopyViewArrayLayerDeprecationTests, BothArrayLayerAndOriginZIsErro encoder = device.CreateCommandEncoder(); encoder.CopyTextureToTexture(&texErrorCopy, &texNewCopy, ©Size); ASSERT_DEVICE_ERROR(encoder.Finish()); + + // TODO(dawn:483): Add other backends after implementing WriteTexture in them. + if (IsMetal() || IsVulkan()) { + std::vector data = {1}; + ASSERT_DEVICE_ERROR( + queue.WriteTexture(&texErrorCopy, data.data(), 4, &bufCopy.layout, ©Size)); + } } // Test that using TextureCopyView::arrayLayer is correctly taken into account TEST_P(TextureCopyViewArrayLayerDeprecationTests, StateTracking) { - wgpu::TextureCopyView texOOBCopy = MakeErrorTextureCopyView(); + wgpu::TextureCopyView texOOBCopy = MakeOldTextureCopyView(); texOOBCopy.arrayLayer = 2; // Oh no, it is OOB! wgpu::TextureCopyView texNewCopy = MakeNewTextureCopyView(); wgpu::BufferCopyView bufCopy = MakeBufferCopyView(); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - encoder.CopyBufferToTexture(&bufCopy, &texOOBCopy, ©Size); + EXPECT_DEPRECATION_WARNING(encoder.CopyBufferToTexture(&bufCopy, &texOOBCopy, ©Size)); ASSERT_DEVICE_ERROR(encoder.Finish()); encoder = device.CreateCommandEncoder(); - encoder.CopyTextureToTexture(&texNewCopy, &texOOBCopy, ©Size); + EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToTexture(&texNewCopy, &texOOBCopy, ©Size)); ASSERT_DEVICE_ERROR(encoder.Finish()); encoder = device.CreateCommandEncoder(); - encoder.CopyTextureToBuffer(&texOOBCopy, &bufCopy, ©Size); + EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToBuffer(&texOOBCopy, &bufCopy, ©Size)); ASSERT_DEVICE_ERROR(encoder.Finish()); encoder = device.CreateCommandEncoder(); - encoder.CopyTextureToTexture(&texOOBCopy, &texNewCopy, ©Size); + EXPECT_DEPRECATION_WARNING(encoder.CopyTextureToTexture(&texOOBCopy, &texNewCopy, ©Size)); ASSERT_DEVICE_ERROR(encoder.Finish()); + + // TODO(dawn:483): Add other backends after implementing WriteTexture in them. + if (IsMetal() || IsVulkan()) { + std::vector data = {1}; + EXPECT_DEPRECATION_WARNING(ASSERT_DEVICE_ERROR( + queue.WriteTexture(&texOOBCopy, data.data(), 4, &bufCopy.layout, ©Size))); + } } DAWN_INSTANTIATE_TEST(TextureCopyViewArrayLayerDeprecationTests,