From 37140e7c62695d366c0845bddf9c8b991fda07da Mon Sep 17 00:00:00 2001 From: Yunchao He Date: Thu, 15 Jul 2021 18:33:48 +0000 Subject: [PATCH] Fix bugs about TextureViewDesc's default values TextureViewDescriptor's default values for dimension and arrayLayerCount in Dawn are not correct according to WebGPU spec. This change fixes these bugs. Bug: dawn:760 Change-Id: Ic1d069838d6c0f7bb1afa1dceaf73e91bdfdb20a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/58020 Commit-Queue: Yunchao He Reviewed-by: Kai Ninomiya --- src/dawn_native/Texture.cpp | 23 ++++++--- src/tests/end2end/StorageTextureTests.cpp | 51 ++++++++++++------- src/tests/end2end/TextureViewTests.cpp | 4 +- .../RenderPassDescriptorValidationTests.cpp | 4 +- .../validation/TextureViewValidationTests.cpp | 20 +++++++- 5 files changed, 75 insertions(+), 27 deletions(-) diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index d9567d59f1..8d6bc55398 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -362,11 +362,7 @@ namespace dawn_native { break; case wgpu::TextureDimension::e2D: - if (texture->GetArrayLayers() > 1u && desc.arrayLayerCount == 0) { - desc.dimension = wgpu::TextureViewDimension::e2DArray; - } else { - desc.dimension = wgpu::TextureViewDimension::e2D; - } + desc.dimension = wgpu::TextureViewDimension::e2D; break; case wgpu::TextureDimension::e3D: @@ -380,7 +376,22 @@ namespace dawn_native { desc.format = texture->GetFormat().format; } if (desc.arrayLayerCount == 0) { - desc.arrayLayerCount = texture->GetArrayLayers() - desc.baseArrayLayer; + switch (desc.dimension) { + case wgpu::TextureViewDimension::e1D: + case wgpu::TextureViewDimension::e2D: + case wgpu::TextureViewDimension::e3D: + desc.arrayLayerCount = 1; + break; + case wgpu::TextureViewDimension::Cube: + desc.arrayLayerCount = 6; + break; + case wgpu::TextureViewDimension::e2DArray: + case wgpu::TextureViewDimension::CubeArray: + desc.arrayLayerCount = texture->GetArrayLayers() - desc.baseArrayLayer; + break; + default: + UNREACHABLE(); + } } if (desc.mipLevelCount == 0) { desc.mipLevelCount = texture->GetNumMipLevels() - desc.baseMipLevel; diff --git a/src/tests/end2end/StorageTextureTests.cpp b/src/tests/end2end/StorageTextureTests.cpp index c8cb6a0411..47e082c40d 100644 --- a/src/tests/end2end/StorageTextureTests.cpp +++ b/src/tests/end2end/StorageTextureTests.cpp @@ -583,8 +583,10 @@ fn IsEqualTo(pixel : vec4, expected : vec4) -> bool { << fragmentShader; } - void CheckResultInStorageBuffer(wgpu::Texture readonlyStorageTexture, - const std::string& computeShader) { + void CheckResultInStorageBuffer( + wgpu::Texture readonlyStorageTexture, + const std::string& computeShader, + wgpu::TextureViewDimension dimension = wgpu::TextureViewDimension::e2D) { wgpu::ComputePipeline pipeline = CreateComputePipeline(computeShader.c_str()); // Clear the content of the result buffer into 0. @@ -592,9 +594,11 @@ fn IsEqualTo(pixel : vec4, expected : vec4) -> bool { wgpu::Buffer resultBuffer = utils::CreateBufferFromData(device, &kInitialValue, sizeof(kInitialValue), wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopySrc); - wgpu::BindGroup bindGroup = - utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), - {{0, readonlyStorageTexture.CreateView()}, {1, resultBuffer}}); + wgpu::TextureViewDescriptor descriptor; + descriptor.dimension = dimension; + wgpu::BindGroup bindGroup = utils::MakeBindGroup( + device, pipeline.GetBindGroupLayout(0), + {{0, readonlyStorageTexture.CreateView(&descriptor)}, {1, resultBuffer}}); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder computeEncoder = encoder.BeginComputePass(); @@ -635,12 +639,17 @@ fn IsEqualTo(pixel : vec4, expected : vec4) -> bool { queue.Submit(1, &commandBuffer); } - void WriteIntoStorageTextureInComputePass(wgpu::Texture writeonlyStorageTexture, - const char* computeShader) { + void WriteIntoStorageTextureInComputePass( + wgpu::Texture writeonlyStorageTexture, + const char* computeShader, + wgpu::TextureViewDimension dimension = wgpu::TextureViewDimension::e2D) { // Create a compute pipeline that writes the expected pixel values into the storage texture. + wgpu::TextureViewDescriptor descriptor; + descriptor.dimension = dimension; wgpu::ComputePipeline pipeline = CreateComputePipeline(computeShader); - wgpu::BindGroup bindGroup = utils::MakeBindGroup( - device, pipeline.GetBindGroupLayout(0), {{0, writeonlyStorageTexture.CreateView()}}); + wgpu::BindGroup bindGroup = + utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), + {{0, writeonlyStorageTexture.CreateView(&descriptor)}}); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder computePassEncoder = encoder.BeginComputePass(); @@ -652,14 +661,19 @@ fn IsEqualTo(pixel : vec4, expected : vec4) -> bool { queue.Submit(1, &commandBuffer); } - void ReadWriteIntoStorageTextureInComputePass(wgpu::Texture readonlyStorageTexture, - wgpu::Texture writeonlyStorageTexture, - const char* computeShader) { + void ReadWriteIntoStorageTextureInComputePass( + wgpu::Texture readonlyStorageTexture, + wgpu::Texture writeonlyStorageTexture, + const char* computeShader, + wgpu::TextureViewDimension dimension = wgpu::TextureViewDimension::e2D) { // Create a compute pipeline that writes the expected pixel values into the storage texture. + wgpu::TextureViewDescriptor descriptor; + descriptor.dimension = dimension; wgpu::ComputePipeline pipeline = CreateComputePipeline(computeShader); - wgpu::BindGroup bindGroup = utils::MakeBindGroup( - device, pipeline.GetBindGroupLayout(0), - {{0, writeonlyStorageTexture.CreateView()}, {1, readonlyStorageTexture.CreateView()}}); + wgpu::BindGroup bindGroup = + utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), + {{0, writeonlyStorageTexture.CreateView(&descriptor)}, + {1, readonlyStorageTexture.CreateView(&descriptor)}}); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder computePassEncoder = encoder.BeginComputePass(); @@ -991,7 +1005,7 @@ TEST_P(StorageTextureTests, Readonly2DArrayOr3DStorageTexture) { } })"; - CheckResultInStorageBuffer(readonlyStorageTexture, csStream.str()); + CheckResultInStorageBuffer(readonlyStorageTexture, csStream.str(), dimension); } } @@ -1018,7 +1032,8 @@ TEST_P(StorageTextureTests, Writeonly2DArrayOr3DStorageTexture) { // Write the expected pixel values into the write-only storage texture. const std::string computeShader = CommonWriteOnlyTestCode("compute", kTextureFormat, dimension); - WriteIntoStorageTextureInComputePass(writeonlyStorageTexture, computeShader.c_str()); + WriteIntoStorageTextureInComputePass(writeonlyStorageTexture, computeShader.c_str(), + dimension); // Verify the pixel data in the write-only storage texture is expected. CheckOutputStorageTexture(writeonlyStorageTexture, kTextureFormat, kSliceCount); @@ -1053,7 +1068,7 @@ TEST_P(StorageTextureTests, ReadWrite2DArrayOr3DStorageTexture) { // Read values from read-only storage texture and write into the write-only storage texture. const std::string computeShader = CommonReadWriteTestCode(kTextureFormat, dimension); ReadWriteIntoStorageTextureInComputePass(readonlyStorageTexture, writeonlyStorageTexture, - computeShader.c_str()); + computeShader.c_str(), dimension); // Verify the data in the write-only storage texture is expected. CheckOutputStorageTexture(writeonlyStorageTexture, kTextureFormat, kSliceCount); diff --git a/src/tests/end2end/TextureViewTests.cpp b/src/tests/end2end/TextureViewTests.cpp index a4387b0e3e..61116d2fb3 100644 --- a/src/tests/end2end/TextureViewTests.cpp +++ b/src/tests/end2end/TextureViewTests.cpp @@ -361,7 +361,9 @@ TEST_P(TextureViewSamplingTest, Default2DArrayTexture) { constexpr uint32_t kMipLevels = 1; initTexture(kLayers, kMipLevels); - wgpu::TextureView textureView = mTexture.CreateView(); + wgpu::TextureViewDescriptor descriptor; + descriptor.dimension = wgpu::TextureViewDimension::e2DArray; + wgpu::TextureView textureView = mTexture.CreateView(&descriptor); const char* fragmentShader = R"( [[group(0), binding(0)]] var sampler0 : sampler; diff --git a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp index 4505398edd..abbc3796da 100644 --- a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp @@ -559,7 +559,9 @@ namespace { wgpu::Texture resolveTexture = CreateTexture(device, wgpu::TextureDimension::e2D, kColorFormat, kSize, kSize, kArrayLayers2, kLevelCount); - wgpu::TextureView resolveTextureView = resolveTexture.CreateView(); + wgpu::TextureViewDescriptor viewDesc; + viewDesc.dimension = wgpu::TextureViewDimension::e2DArray; + wgpu::TextureView resolveTextureView = resolveTexture.CreateView(&viewDesc); utils::ComboRenderPassDescriptor renderPass = CreateMultisampledRenderPass(); renderPass.cColorAttachments[0].resolveTarget = resolveTextureView; diff --git a/src/tests/unittests/validation/TextureViewValidationTests.cpp b/src/tests/unittests/validation/TextureViewValidationTests.cpp index 31ad3c159e..e60ba915e5 100644 --- a/src/tests/unittests/validation/TextureViewValidationTests.cpp +++ b/src/tests/unittests/validation/TextureViewValidationTests.cpp @@ -288,7 +288,7 @@ namespace { // specifying the values they're supposed to default to. // Variant for a 2D texture with more than 1 array layer. TEST_F(TextureViewValidationTest, TextureViewDescriptorDefaults2DArray) { - constexpr uint32_t kDefaultArrayLayers = 6; + constexpr uint32_t kDefaultArrayLayers = 8; wgpu::Texture texture = Create2DArrayTexture(device, kDefaultArrayLayers); { texture.CreateView(); } @@ -307,7 +307,25 @@ namespace { texture.CreateView(&descriptor); descriptor.dimension = wgpu::TextureViewDimension::e2DArray; texture.CreateView(&descriptor); + // Setting view dimension to 2D, its arrayLayer will default to 1. And view creation + // will success. descriptor.dimension = wgpu::TextureViewDimension::e2D; + texture.CreateView(&descriptor); + // Setting view dimension to Cube, its arrayLayer will default to 6. + descriptor.dimension = wgpu::TextureViewDimension::Cube; + texture.CreateView(&descriptor); + descriptor.baseArrayLayer = 2; + texture.CreateView(&descriptor); + descriptor.baseArrayLayer = 3; + ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor)); + // Setting view dimension to CubeArray, its arrayLayer will default to + // size.depthOrArrayLayers (kDefaultArrayLayers) - baseArrayLayer. + descriptor.dimension = wgpu::TextureViewDimension::CubeArray; + descriptor.baseArrayLayer = 0; + ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor)); + descriptor.baseArrayLayer = 2; + texture.CreateView(&descriptor); + descriptor.baseArrayLayer = 3; ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor)); } {