From f02bf9e76d1e070a8532a378e6f8e99b3b2b66ae Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Thu, 7 Apr 2022 20:20:14 +0000 Subject: [PATCH] Make texture view default format robust against invalid aspects Computation of the default depends on the aspect, but the aspect may be invalid. Make the default computation robust such that it picks some valid aspect if a valid one is not provided. This will make computing the default OK, but still fail validation of the aspect later. Bug: chromium:1314049 Change-Id: I2dcfe7962c7e30ac627605b6d0f1c269b3a6af6e Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/86020 Reviewed-by: Loko Kung Commit-Queue: Austin Eng --- src/dawn/native/Device.cpp | 5 ++++- src/dawn/native/Texture.cpp | 7 ++++++- src/dawn/native/Texture.h | 2 +- .../validation/TextureViewValidationTests.cpp | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index f79ead5717..02535fc2b5 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -1630,7 +1630,10 @@ namespace dawn::native { const TextureViewDescriptor* descriptor) { DAWN_TRY(ValidateIsAlive()); DAWN_TRY(ValidateObject(texture)); - TextureViewDescriptor desc = GetTextureViewDescriptorWithDefaults(texture, descriptor); + + TextureViewDescriptor desc; + DAWN_TRY_ASSIGN(desc, GetTextureViewDescriptorWithDefaults(texture, descriptor)); + if (IsValidationEnabled()) { DAWN_TRY_CONTEXT(ValidateTextureViewDescriptor(this, texture, &desc), "validating %s against %s.", &desc, texture); diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp index f4c4fc57cd..cabef76fe0 100644 --- a/src/dawn/native/Texture.cpp +++ b/src/dawn/native/Texture.cpp @@ -443,7 +443,7 @@ namespace dawn::native { return {}; } - TextureViewDescriptor GetTextureViewDescriptorWithDefaults( + ResultOrError GetTextureViewDescriptorWithDefaults( const TextureBase* texture, const TextureViewDescriptor* descriptor) { ASSERT(texture); @@ -473,6 +473,11 @@ namespace dawn::native { if (desc.format == wgpu::TextureFormat::Undefined) { const Format& format = texture->GetFormat(); + + // Check the aspect since |SelectFormatAspects| assumes a valid aspect. + // Creation would have failed validation later since the aspect is invalid. + DAWN_TRY(ValidateTextureAspect(desc.aspect)); + Aspect aspects = SelectFormatAspects(format, desc.aspect); if (HasOneBit(aspects)) { desc.format = format.GetAspectInfo(aspects).format; diff --git a/src/dawn/native/Texture.h b/src/dawn/native/Texture.h index 4024c82ed2..227aeb0fae 100644 --- a/src/dawn/native/Texture.h +++ b/src/dawn/native/Texture.h @@ -34,7 +34,7 @@ namespace dawn::native { MaybeError ValidateTextureViewDescriptor(const DeviceBase* device, const TextureBase* texture, const TextureViewDescriptor* descriptor); - TextureViewDescriptor GetTextureViewDescriptorWithDefaults( + ResultOrError GetTextureViewDescriptorWithDefaults( const TextureBase* texture, const TextureViewDescriptor* descriptor); diff --git a/src/dawn/tests/unittests/validation/TextureViewValidationTests.cpp b/src/dawn/tests/unittests/validation/TextureViewValidationTests.cpp index 36917261e9..f1b16f20d7 100644 --- a/src/dawn/tests/unittests/validation/TextureViewValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/TextureViewValidationTests.cpp @@ -590,6 +590,20 @@ namespace { } } + // Regression test for crbug.com/1314049. Format default depends on the aspect. + // Test that computing the default does not crash if the aspect is invalid. + TEST_F(TextureViewValidationTest, TextureViewDescriptorDefaultsInvalidAspect) { + wgpu::Texture texture = + CreateDepthStencilTexture(device, wgpu::TextureFormat::Depth24PlusStencil8); + + wgpu::TextureViewDescriptor viewDesc = {}; + viewDesc.aspect = static_cast(-1); + + // Validation should catch the invalid aspect. + ASSERT_DEVICE_ERROR(texture.CreateView(&viewDesc), + testing::HasSubstr("Invalid value for WGPUTextureAspect")); + } + // Test creating cube map texture view TEST_F(TextureViewValidationTest, CreateCubeMapTextureView) { constexpr uint32_t kDefaultArrayLayers = 16;