From 2f1278e68b9073dedcdcfb9574a2477fffbb37b9 Mon Sep 17 00:00:00 2001 From: Brandon Jones Date: Wed, 13 Jan 2021 22:25:58 +0000 Subject: [PATCH] Enabled BindGroupLayout deprecation warning and fixed tests it broke. This should be the last change BindGroupLayout change needed to complete the conversion to the new structure aside from removing the deprecated code paths in the future. Bug: dawn:527 Change-Id: I44f67de80f1b4e1b7b32909d70d74610f7a06d8d Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/37560 Reviewed-by: Kai Ninomiya Reviewed-by: Corentin Wallez Commit-Queue: Brandon Jones --- src/dawn_native/BindGroupLayout.cpp | 5 +- src/tests/end2end/DeprecatedAPITests.cpp | 9 ++-- .../GetBindGroupLayoutValidationTests.cpp | 48 ++++++++++--------- .../StorageTextureValidationTests.cpp | 3 +- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 0ae9b83872..bcff274022 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -158,12 +158,11 @@ namespace dawn_native { "texture, or storageTexture are set"); } } else if (bindingMemberCount == 0) { - // TODO(dawn:527): Raising this warning breaks a ton of validation tests. // Deprecated validation path - /*device->EmitDeprecationWarning( + device->EmitDeprecationWarning( "The format of BindGroupLayoutEntry has changed, and will soon require the " "buffer, sampler, texture, or storageTexture members be set rather than " - "setting type, etc. on the entry directly.");*/ + "setting type, etc. on the entry directly."); DAWN_TRY(ValidateBindingType(entry.type)); DAWN_TRY(ValidateTextureComponentType(entry.textureComponentType)); diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index 5cd4240b6f..f66126dbf7 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/src/tests/end2end/DeprecatedAPITests.cpp @@ -60,9 +60,8 @@ TEST_P(DeprecationTests, BindGroupLayoutEntryTypeConflict) { descriptor.entries = &binding; // Succeeds with only a type. - // Will soon emit a deprecation warning. binding.type = wgpu::BindingType::UniformBuffer; - device.CreateBindGroupLayout(&descriptor); + EXPECT_DEPRECATION_WARNING(device.CreateBindGroupLayout(&descriptor)); binding.type = wgpu::BindingType::Undefined; @@ -117,10 +116,12 @@ TEST_P(DeprecationTests, BindGroupLayoutEntryViewDimensionDefaulting) { bglDesc.entryCount = 1; bglDesc.entries = &binding; + wgpu::BindGroupLayout bgl; + // Check that the default viewDimension is 2D. { binding.viewDimension = wgpu::TextureViewDimension::Undefined; - wgpu::BindGroupLayout bgl = device.CreateBindGroupLayout(&bglDesc); + EXPECT_DEPRECATION_WARNING(bgl = device.CreateBindGroupLayout(&bglDesc)); wgpu::TextureDescriptor desc; desc.usage = wgpu::TextureUsage::Sampled; @@ -136,7 +137,7 @@ TEST_P(DeprecationTests, BindGroupLayoutEntryViewDimensionDefaulting) { // Check that setting a non-default viewDimension works. { binding.viewDimension = wgpu::TextureViewDimension::e2DArray; - wgpu::BindGroupLayout bgl = device.CreateBindGroupLayout(&bglDesc); + EXPECT_DEPRECATION_WARNING(bgl = device.CreateBindGroupLayout(&bglDesc)); wgpu::TextureDescriptor desc; desc.usage = wgpu::TextureUsage::Sampled; diff --git a/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp b/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp index 64b5111cee..2dbd5ee281 100644 --- a/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp +++ b/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp @@ -97,15 +97,15 @@ TEST_F(GetBindGroupLayoutTests, DefaultShaderStageAndDynamicOffsets) { wgpu::BindGroupLayoutEntry binding = {}; binding.binding = 0; - binding.type = wgpu::BindingType::UniformBuffer; - binding.minBufferBindingSize = 4 * sizeof(float); + binding.buffer.type = wgpu::BufferBindingType::Uniform; + binding.buffer.minBindingSize = 4 * sizeof(float); wgpu::BindGroupLayoutDescriptor desc = {}; desc.entryCount = 1; desc.entries = &binding; // Check that visibility and dynamic offsets match - binding.hasDynamicOffset = false; + binding.buffer.hasDynamicOffset = false; binding.visibility = wgpu::ShaderStage::Fragment; EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); @@ -117,7 +117,7 @@ TEST_F(GetBindGroupLayoutTests, DefaultShaderStageAndDynamicOffsets) { EXPECT_NE(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); // Check that any change in hasDynamicOffsets doesn't match. - binding.hasDynamicOffset = true; + binding.buffer.hasDynamicOffset = true; binding.visibility = wgpu::ShaderStage::Fragment; EXPECT_NE(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); } @@ -142,10 +142,10 @@ TEST_F(GetBindGroupLayoutTests, ComputePipeline) { wgpu::BindGroupLayoutEntry binding = {}; binding.binding = 0; - binding.type = wgpu::BindingType::UniformBuffer; + binding.buffer.type = wgpu::BufferBindingType::Uniform; binding.visibility = wgpu::ShaderStage::Compute; - binding.hasDynamicOffset = false; - binding.minBufferBindingSize = 4 * sizeof(float); + binding.buffer.hasDynamicOffset = false; + binding.buffer.minBindingSize = 4 * sizeof(float); wgpu::BindGroupLayoutDescriptor desc = {}; desc.entryCount = 1; @@ -158,8 +158,8 @@ TEST_F(GetBindGroupLayoutTests, ComputePipeline) { TEST_F(GetBindGroupLayoutTests, BindingType) { wgpu::BindGroupLayoutEntry binding = {}; binding.binding = 0; - binding.hasDynamicOffset = false; - binding.minBufferBindingSize = 4 * sizeof(float); + binding.buffer.hasDynamicOffset = false; + binding.buffer.minBindingSize = 4 * sizeof(float); binding.visibility = wgpu::ShaderStage::Fragment; wgpu::BindGroupLayoutDescriptor desc = {}; @@ -169,7 +169,7 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { { // Storage buffer binding is not supported in vertex shader. binding.visibility = wgpu::ShaderStage::Fragment; - binding.type = wgpu::BindingType::StorageBuffer; + binding.buffer.type = wgpu::BufferBindingType::Storage; wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( [[block]] struct S { [[offset(0)]] pos : vec4; @@ -181,7 +181,7 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); } { - binding.type = wgpu::BindingType::UniformBuffer; + binding.buffer.type = wgpu::BufferBindingType::Uniform; wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( [[block]] struct S { [[offset(0)]] pos : vec4; @@ -194,7 +194,7 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { } { - binding.type = wgpu::BindingType::ReadonlyStorageBuffer; + binding.buffer.type = wgpu::BufferBindingType::ReadOnlyStorage; wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( [[block]] struct S { [[offset(0)]] pos : vec4; @@ -206,9 +206,10 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); } - binding.minBufferBindingSize = 0; + binding.buffer.type = wgpu::BufferBindingType::Undefined; + binding.buffer.minBindingSize = 0; { - binding.type = wgpu::BindingType::SampledTexture; + binding.texture.sampleType = wgpu::TextureSampleType::Float; wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( [[set(0), binding(0)]] var myTexture : texture_2d; @@ -218,7 +219,7 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { } { - binding.type = wgpu::BindingType::MultisampledTexture; + binding.texture.multisampled = true; wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( [[set(0), binding(0)]] var myTexture : texture_multisampled_2d; @@ -227,8 +228,9 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); } + binding.texture.sampleType = wgpu::TextureSampleType::Undefined; { - binding.type = wgpu::BindingType::Sampler; + binding.sampler.type = wgpu::SamplerBindingType::Filtering; wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( [[set(0), binding(0)]] var mySampler: sampler; @@ -354,10 +356,10 @@ TEST_F(GetBindGroupLayoutTests, TextureComponentType) { // Test that binding= indices match. TEST_F(GetBindGroupLayoutTests, BindingIndices) { wgpu::BindGroupLayoutEntry binding = {}; - binding.type = wgpu::BindingType::UniformBuffer; binding.visibility = wgpu::ShaderStage::Fragment; - binding.hasDynamicOffset = false; - binding.minBufferBindingSize = 4 * sizeof(float); + binding.buffer.type = wgpu::BufferBindingType::Uniform; + binding.buffer.hasDynamicOffset = false; + binding.buffer.minBindingSize = 4 * sizeof(float); wgpu::BindGroupLayoutDescriptor desc = {}; desc.entryCount = 1; @@ -473,16 +475,16 @@ TEST_F(GetBindGroupLayoutTests, MinBufferSize) { // Create BGLs with minBufferBindingSize 4 and 64. wgpu::BindGroupLayoutEntry binding = {}; binding.binding = 0; - binding.type = wgpu::BindingType::UniformBuffer; + binding.buffer.type = wgpu::BufferBindingType::Uniform; binding.visibility = wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Vertex; wgpu::BindGroupLayoutDescriptor desc = {}; desc.entryCount = 1; desc.entries = &binding; - binding.minBufferBindingSize = 4; + binding.buffer.minBindingSize = 4; wgpu::BindGroupLayout bgl4 = device.CreateBindGroupLayout(&desc); - binding.minBufferBindingSize = 64; + binding.buffer.minBindingSize = 64; wgpu::BindGroupLayout bgl64 = device.CreateBindGroupLayout(&desc); utils::ComboRenderPipelineDescriptor descriptor(device); @@ -536,7 +538,7 @@ TEST_F(GetBindGroupLayoutTests, StageAggregation) { // Create BGLs with minBufferBindingSize 4 and 64. wgpu::BindGroupLayoutEntry binding = {}; binding.binding = 0; - binding.type = wgpu::BindingType::Sampler; + binding.sampler.type = wgpu::SamplerBindingType::Filtering; wgpu::BindGroupLayoutDescriptor desc = {}; desc.entryCount = 1; diff --git a/src/tests/unittests/validation/StorageTextureValidationTests.cpp b/src/tests/unittests/validation/StorageTextureValidationTests.cpp index c3433cf18a..a5701d8f8d 100644 --- a/src/tests/unittests/validation/StorageTextureValidationTests.cpp +++ b/src/tests/unittests/validation/StorageTextureValidationTests.cpp @@ -616,7 +616,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureCannotHaveDynamicOffsets) { bindGroupLayoutBinding.storageTextureFormat = wgpu::TextureFormat::R32Float; bindGroupLayoutBinding.hasDynamicOffset = true; - ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding})); + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING( + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}))); } }