From a24acf2fbc8a6c2535596582923b4b95ea9a1382 Mon Sep 17 00:00:00 2001 From: Shaobo Date: Wed, 2 Mar 2022 02:44:01 +0000 Subject: [PATCH] Validate destination texture states for CopyTextureForBrowser Current implmentation of ValidateCopyTextureForBrowser has a bug to verify destination texture states. This CL fix the simple bug and add noop copy in unittest to catch this issue. Bug: dawn:1306 Change-Id: I193105ced89db5092873d604c7dbf43d7ea4fba0 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/82101 Commit-Queue: Shaobo Yan Reviewed-by: Austin Eng Reviewed-by: Jiawei Shao Commit-Queue: Jiawei Shao --- .../native/CopyTextureForBrowserHelper.cpp | 5 ++- .../validation/CopyTextureForBrowserTests.cpp | 38 +++++++++++++++---- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/dawn/native/CopyTextureForBrowserHelper.cpp b/src/dawn/native/CopyTextureForBrowserHelper.cpp index 84b78c38a9..bfaa2685cb 100644 --- a/src/dawn/native/CopyTextureForBrowserHelper.cpp +++ b/src/dawn/native/CopyTextureForBrowserHelper.cpp @@ -329,8 +329,9 @@ namespace dawn::native { DAWN_INVALID_IF(source->texture->GetTextureState() == TextureBase::TextureState::Destroyed, "Source texture %s is destroyed.", source->texture); - DAWN_INVALID_IF(source->texture->GetTextureState() == TextureBase::TextureState::Destroyed, - "Destination texture %s is destroyed.", destination->texture); + DAWN_INVALID_IF( + destination->texture->GetTextureState() == TextureBase::TextureState::Destroyed, + "Destination texture %s is destroyed.", destination->texture); DAWN_TRY_CONTEXT(ValidateImageCopyTexture(device, *source, *copySize), "validating the ImageCopyTexture for the source"); diff --git a/src/dawn/tests/unittests/validation/CopyTextureForBrowserTests.cpp b/src/dawn/tests/unittests/validation/CopyTextureForBrowserTests.cpp index 881c602346..489a802fa6 100644 --- a/src/dawn/tests/unittests/validation/CopyTextureForBrowserTests.cpp +++ b/src/dawn/tests/unittests/validation/CopyTextureForBrowserTests.cpp @@ -159,33 +159,57 @@ TEST_F(CopyTextureForBrowserTest, IncorrectUsage) { // Test source or destination texture is destroyed. TEST_F(CopyTextureForBrowserTest, DestroyedTexture) { - wgpu::Texture source = - Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, - wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding); - wgpu::Texture destination = - Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, - wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::RenderAttachment); - wgpu::CopyTextureForBrowserOptions options = {}; // Valid src and dst textures. { + wgpu::Texture source = + Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding); + wgpu::Texture destination = + Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::RenderAttachment); TestCopyTextureForBrowser(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, options); + + // Check noop copy + TestCopyTextureForBrowser(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, + {0, 0, 0}, {0, 0, 0}, wgpu::TextureAspect::All, options); } // Destroyed src texture. { + wgpu::Texture source = + Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding); + wgpu::Texture destination = + Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::RenderAttachment); source.Destroy(); TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, options); + + // Check noop copy + TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, + {0, 0, 0}, {0, 0, 0}, wgpu::TextureAspect::All, options); } // Destroyed dst texture. { + wgpu::Texture source = + Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding); + wgpu::Texture destination = + Create2DTexture(16, 16, 5, 4, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::RenderAttachment); + destination.Destroy(); TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, options); + + // Check noop copy + TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, + {0, 0, 0}, {0, 0, 0}, wgpu::TextureAspect::All, options); } }