Remove deprecated BindGroupLayoutEntry fields

Bug: dawn:22
Change-Id: Idf0fd0505fb7e5e5389520fa7bc638e137f4c0b6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/51765
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

src/tests/end2end/DeprecatedAPITests.cpp

@@ -85,109 +85,6 @@ TEST_P(DeprecationTests, SetAttachmentDescriptorAttachment) {
     pass.EndPass();
 }
 
-// Test that BindGroupLayoutEntry cannot have a type if buffer, sampler, texture, or storageTexture
-// are defined.
-TEST_P(DeprecationTests, BindGroupLayoutEntryTypeConflict) {
-    wgpu::BindGroupLayoutEntry binding;
-    binding.binding = 0;
-    binding.visibility = wgpu::ShaderStage::Vertex;
-
-    wgpu::BindGroupLayoutDescriptor descriptor;
-    descriptor.entryCount = 1;
-    descriptor.entries = &binding;
-
-    // Succeeds with only a type.
-    binding.type = wgpu::BindingType::UniformBuffer;
-    EXPECT_DEPRECATION_WARNING(device.CreateBindGroupLayout(&descriptor));
-
-    binding.type = wgpu::BindingType::Undefined;
-
-    // Succeeds with only a buffer.type.
-    binding.buffer.type = wgpu::BufferBindingType::Uniform;
-    device.CreateBindGroupLayout(&descriptor);
-    // Fails when both type and a buffer.type are specified.
-    binding.type = wgpu::BindingType::UniformBuffer;
-    ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&descriptor));
-
-    binding.buffer.type = wgpu::BufferBindingType::Undefined;
-    binding.type = wgpu::BindingType::Undefined;
-
-    // Succeeds with only a sampler.type.
-    binding.sampler.type = wgpu::SamplerBindingType::Filtering;
-    device.CreateBindGroupLayout(&descriptor);
-    // Fails when both type and a sampler.type are specified.
-    binding.type = wgpu::BindingType::Sampler;
-    ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&descriptor));
-
-    binding.sampler.type = wgpu::SamplerBindingType::Undefined;
-    binding.type = wgpu::BindingType::Undefined;
-
-    // Succeeds with only a texture.sampleType.
-    binding.texture.sampleType = wgpu::TextureSampleType::Float;
-    device.CreateBindGroupLayout(&descriptor);
-    // Fails when both type and a texture.sampleType are specified.
-    binding.type = wgpu::BindingType::SampledTexture;
-    ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&descriptor));
-
-    binding.texture.sampleType = wgpu::TextureSampleType::Undefined;
-    binding.type = wgpu::BindingType::Undefined;
-
-    // Succeeds with only a storageTexture.access.
-    binding.storageTexture.access = wgpu::StorageTextureAccess::ReadOnly;
-    binding.storageTexture.format = wgpu::TextureFormat::RGBA8Unorm;
-    device.CreateBindGroupLayout(&descriptor);
-    // Fails when both type and a storageTexture.access are specified.
-    binding.type = wgpu::BindingType::ReadonlyStorageTexture;
-    ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&descriptor));
-}
-
-// Test that the deprecated BGLEntry path correctly handles the defaulting of viewDimension.
-// This is a regression test for crbug.com/dawn/620
-TEST_P(DeprecationTests, BindGroupLayoutEntryViewDimensionDefaulting) {
-    wgpu::BindGroupLayoutEntry binding;
-    binding.binding = 0;
-    binding.visibility = wgpu::ShaderStage::Vertex;
-    binding.type = wgpu::BindingType::SampledTexture;
-
-    wgpu::BindGroupLayoutDescriptor bglDesc;
-    bglDesc.entryCount = 1;
-    bglDesc.entries = &binding;
-
-    wgpu::BindGroupLayout bgl;
-
-    // Check that the default viewDimension is 2D.
-    {
-        binding.viewDimension = wgpu::TextureViewDimension::Undefined;
-        EXPECT_DEPRECATION_WARNING(bgl = device.CreateBindGroupLayout(&bglDesc));
-
-        wgpu::TextureDescriptor desc;
-        desc.usage = wgpu::TextureUsage::Sampled;
-        desc.size = {1, 1, 1};
-        desc.format = wgpu::TextureFormat::RGBA8Unorm;
-        desc.dimension = wgpu::TextureDimension::e2D;
-        wgpu::Texture texture = device.CreateTexture(&desc);
-
-        // Success, the default is 2D and we give it a 2D view.
-        utils::MakeBindGroup(device, bgl, {{0, texture.CreateView()}});
-    }
-
-    // Check that setting a non-default viewDimension works.
-    {
-        binding.viewDimension = wgpu::TextureViewDimension::e2DArray;
-        EXPECT_DEPRECATION_WARNING(bgl = device.CreateBindGroupLayout(&bglDesc));
-
-        wgpu::TextureDescriptor desc;
-        desc.usage = wgpu::TextureUsage::Sampled;
-        desc.size = {1, 1, 4};
-        desc.format = wgpu::TextureFormat::RGBA8Unorm;
-        desc.dimension = wgpu::TextureDimension::e2D;
-        wgpu::Texture texture = device.CreateTexture(&desc);
-
-        // Success, the view will be 2DArray and the BGL expects a 2DArray.
-        utils::MakeBindGroup(device, bgl, {{0, texture.CreateView()}});
-    }
-}
-
 DAWN_INSTANTIATE_TEST(DeprecationTests,
                       D3D12Backend(),
                       MetalBackend(),

src/tests/unittests/validation/BindGroupValidationTests.cpp

@@ -736,75 +736,45 @@ TEST_F(BindGroupLayoutValidationTest, BindGroupLayoutVisibilityNoneExpectsBindGr
     ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, bgl, {{0, buffer}}));
 }
 
+#define BGLEntryType(...) \
+    utils::BindingLayoutEntryInitializationHelper(0, wgpu::ShaderStage::Compute, __VA_ARGS__)
+
 TEST_F(BindGroupLayoutValidationTest, PerStageLimits) {
     struct TestInfo {
         uint32_t maxCount;
-        wgpu::BindingType bindingType;
-        wgpu::BindingType otherBindingType;
+        wgpu::BindGroupLayoutEntry entry;
+        wgpu::BindGroupLayoutEntry otherEntry;
     };
 
-    constexpr TestInfo kTestInfos[] = {
-        {kMaxSampledTexturesPerShaderStage, wgpu::BindingType::SampledTexture,
-         wgpu::BindingType::UniformBuffer},
-        {kMaxSamplersPerShaderStage, wgpu::BindingType::Sampler, wgpu::BindingType::UniformBuffer},
-        {kMaxSamplersPerShaderStage, wgpu::BindingType::ComparisonSampler,
-         wgpu::BindingType::UniformBuffer},
-        {kMaxStorageBuffersPerShaderStage, wgpu::BindingType::StorageBuffer,
-         wgpu::BindingType::UniformBuffer},
-        {kMaxStorageTexturesPerShaderStage, wgpu::BindingType::ReadonlyStorageTexture,
-         wgpu::BindingType::UniformBuffer},
-        {kMaxStorageTexturesPerShaderStage, wgpu::BindingType::WriteonlyStorageTexture,
-         wgpu::BindingType::UniformBuffer},
-        {kMaxUniformBuffersPerShaderStage, wgpu::BindingType::UniformBuffer,
-         wgpu::BindingType::SampledTexture},
+    std::array<TestInfo, 7> kTestInfos = {
+        TestInfo{kMaxSampledTexturesPerShaderStage, BGLEntryType(wgpu::TextureSampleType::Float),
+                 BGLEntryType(wgpu::BufferBindingType::Uniform)},
+        TestInfo{kMaxSamplersPerShaderStage, BGLEntryType(wgpu::SamplerBindingType::Filtering),
+                 BGLEntryType(wgpu::BufferBindingType::Uniform)},
+        TestInfo{kMaxSamplersPerShaderStage, BGLEntryType(wgpu::SamplerBindingType::Comparison),
+                 BGLEntryType(wgpu::BufferBindingType::Uniform)},
+        TestInfo{kMaxStorageBuffersPerShaderStage, BGLEntryType(wgpu::BufferBindingType::Storage),
+                 BGLEntryType(wgpu::BufferBindingType::Uniform)},
+        TestInfo{
+            kMaxStorageTexturesPerShaderStage,
+            BGLEntryType(wgpu::StorageTextureAccess::ReadOnly, wgpu::TextureFormat::RGBA8Unorm),
+            BGLEntryType(wgpu::BufferBindingType::Uniform)},
+        TestInfo{
+            kMaxStorageTexturesPerShaderStage,
+            BGLEntryType(wgpu::StorageTextureAccess::WriteOnly, wgpu::TextureFormat::RGBA8Unorm),
+            BGLEntryType(wgpu::BufferBindingType::Uniform)},
+        TestInfo{kMaxUniformBuffersPerShaderStage, BGLEntryType(wgpu::BufferBindingType::Uniform),
+                 BGLEntryType(wgpu::TextureSampleType::Float)},
     };
 
     for (TestInfo info : kTestInfos) {
         wgpu::BindGroupLayout bgl[2];
         std::vector<utils::BindingLayoutEntryInitializationHelper> maxBindings;
 
-        auto PopulateEntry = [](utils::BindingLayoutEntryInitializationHelper entry) {
-            switch (entry.type) {
-                case wgpu::BindingType::UniformBuffer:
-                    entry.buffer.type = wgpu::BufferBindingType::Uniform;
-                    break;
-                case wgpu::BindingType::StorageBuffer:
-                    entry.buffer.type = wgpu::BufferBindingType::Storage;
-                    break;
-                case wgpu::BindingType::ReadonlyStorageBuffer:
-                    entry.buffer.type = wgpu::BufferBindingType::ReadOnlyStorage;
-                    break;
-
-                case wgpu::BindingType::Sampler:
-                    entry.sampler.type = wgpu::SamplerBindingType::Filtering;
-                    break;
-                case wgpu::BindingType::ComparisonSampler:
-                    entry.sampler.type = wgpu::SamplerBindingType::Comparison;
-                    break;
-
-                case wgpu::BindingType::SampledTexture:
-                    entry.texture.sampleType = wgpu::TextureSampleType::Float;
-                    break;
-
-                case wgpu::BindingType::ReadonlyStorageTexture:
-                    entry.storageTexture.access = wgpu::StorageTextureAccess::ReadOnly;
-                    entry.storageTexture.format = wgpu::TextureFormat::RGBA8Unorm;
-                    break;
-                case wgpu::BindingType::WriteonlyStorageTexture:
-                    entry.storageTexture.access = wgpu::StorageTextureAccess::WriteOnly;
-                    entry.storageTexture.format = wgpu::TextureFormat::RGBA8Unorm;
-                    break;
-                default:
-                    return entry;
-            }
-
-            entry.type = wgpu::BindingType::Undefined;
-
-            return entry;
-        };
-
         for (uint32_t i = 0; i < info.maxCount; ++i) {
-            maxBindings.push_back(PopulateEntry({i, wgpu::ShaderStage::Compute, info.bindingType}));
+            wgpu::BindGroupLayoutEntry entry = info.entry;
+            entry.binding = i;
+            maxBindings.push_back(entry);
         }
 
         // Creating with the maxes works.
@@ -813,24 +783,28 @@ TEST_F(BindGroupLayoutValidationTest, PerStageLimits) {
         // Adding an extra binding of a different type works.
         {
             std::vector<utils::BindingLayoutEntryInitializationHelper> bindings = maxBindings;
-            bindings.push_back(
-                PopulateEntry({info.maxCount, wgpu::ShaderStage::Compute, info.otherBindingType}));
+            wgpu::BindGroupLayoutEntry entry = info.otherEntry;
+            entry.binding = info.maxCount;
+            bindings.push_back(entry);
             MakeBindGroupLayout(bindings.data(), bindings.size());
         }
 
         // Adding an extra binding of the maxed type in a different stage works
         {
             std::vector<utils::BindingLayoutEntryInitializationHelper> bindings = maxBindings;
-            bindings.push_back(
-                PopulateEntry({info.maxCount, wgpu::ShaderStage::Fragment, info.bindingType}));
+            wgpu::BindGroupLayoutEntry entry = info.entry;
+            entry.binding = info.maxCount;
+            entry.visibility = wgpu::ShaderStage::Fragment;
+            bindings.push_back(entry);
             MakeBindGroupLayout(bindings.data(), bindings.size());
         }
 
         // Adding an extra binding of the maxed type and stage exceeds the per stage limit.
         {
             std::vector<utils::BindingLayoutEntryInitializationHelper> bindings = maxBindings;
-            bindings.push_back(
-                PopulateEntry({info.maxCount, wgpu::ShaderStage::Compute, info.bindingType}));
+            wgpu::BindGroupLayoutEntry entry = info.entry;
+            entry.binding = info.maxCount;
+            bindings.push_back(entry);
             ASSERT_DEVICE_ERROR(MakeBindGroupLayout(bindings.data(), bindings.size()));
         }
 
@@ -838,18 +812,19 @@ TEST_F(BindGroupLayoutValidationTest, PerStageLimits) {
         TestCreatePipelineLayout(bgl, 1, true);
 
         // Adding an extra binding of a different type in a different BGL works
-        bgl[1] = utils::MakeBindGroupLayout(
-            device, {PopulateEntry({0, wgpu::ShaderStage::Compute, info.otherBindingType})});
+        bgl[1] = utils::MakeBindGroupLayout(device, {info.otherEntry});
         TestCreatePipelineLayout(bgl, 2, true);
 
-        // Adding an extra binding of the maxed type in a different stage works
-        bgl[1] = utils::MakeBindGroupLayout(
-            device, {PopulateEntry({0, wgpu::ShaderStage::Fragment, info.bindingType})});
-        TestCreatePipelineLayout(bgl, 2, true);
+        {
+            // Adding an extra binding of the maxed type in a different stage works
+            wgpu::BindGroupLayoutEntry entry = info.entry;
+            entry.visibility = wgpu::ShaderStage::Fragment;
+            bgl[1] = utils::MakeBindGroupLayout(device, {entry});
+            TestCreatePipelineLayout(bgl, 2, true);
+        }
 
         // Adding an extra binding of the maxed type in a different BGL exceeds the per stage limit.
-        bgl[1] = utils::MakeBindGroupLayout(
-            device, {PopulateEntry({0, wgpu::ShaderStage::Compute, info.bindingType})});
+        bgl[1] = utils::MakeBindGroupLayout(device, {info.entry});
         TestCreatePipelineLayout(bgl, 2, false);
     }
 }

src/tests/unittests/validation/StorageTextureValidationTests.cpp

@@ -567,25 +567,6 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutViewDimensionMatchesShaderD
     }
 }
 
-// Verify that in a bind group layout binding neither read-only nor write-only storage textures
-// are allowed to have dynamic offsets.
-// TODO(dawn:527): No longer be applicable after changes to BindGroupLayoutEntry are complete.
-TEST_F(StorageTextureValidationTests, StorageTextureCannotHaveDynamicOffsets) {
-    const std::array<wgpu::BindingType, 2> kSupportedStorageTextureBindingTypes = {
-        wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture};
-    for (wgpu::BindingType storageBindingType : kSupportedStorageTextureBindingTypes) {
-        wgpu::BindGroupLayoutEntry bindGroupLayoutBinding;
-        bindGroupLayoutBinding.binding = 0;
-        bindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute;
-        bindGroupLayoutBinding.type = storageBindingType;
-        bindGroupLayoutBinding.storageTextureFormat = wgpu::TextureFormat::R32Float;
-
-        bindGroupLayoutBinding.hasDynamicOffset = true;
-        ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(
-            utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding})));
-    }
-}
-
 // Verify that only a texture view can be used as a read-only or write-only storage texture in a
 // bind group.
 TEST_F(StorageTextureValidationTests, StorageTextureBindingTypeInBindGroup) {

src/tests/unittests/wire/WireArgumentTests.cpp

@@ -302,39 +302,21 @@ TEST_F(WireArgumentTests, StructureOfStructureArrayArgument) {
         {nullptr,
          0,
          WGPUShaderStage_Vertex,
-         WGPUBindingType_Sampler,
-         false,
-         0,
-         WGPUTextureViewDimension_2D,
-         WGPUTextureComponentType_Float,
-         WGPUTextureFormat_RGBA8Unorm,
-         {},
          {},
+         {nullptr, WGPUSamplerBindingType_Filtering},
          {},
          {}},
         {nullptr,
          1,
          WGPUShaderStage_Vertex,
-         WGPUBindingType_SampledTexture,
-         false,
-         0,
-         WGPUTextureViewDimension_2D,
-         WGPUTextureComponentType_Float,
-         WGPUTextureFormat_RGBA8Unorm,
-         {},
          {},
          {},
+         {nullptr, WGPUTextureSampleType_Float, WGPUTextureViewDimension_2D, false},
          {}},
         {nullptr,
          2,
          static_cast<WGPUShaderStage>(WGPUShaderStage_Vertex | WGPUShaderStage_Fragment),
-         WGPUBindingType_UniformBuffer,
-         false,
-         0,
-         WGPUTextureViewDimension_2D,
-         WGPUTextureComponentType_Float,
-         WGPUTextureFormat_RGBA8Unorm,
-         {},
+         {nullptr, WGPUBufferBindingType_Uniform, false, 0},
          {},
          {},
          {}},
@@ -353,7 +335,8 @@ TEST_F(WireArgumentTests, StructureOfStructureArrayArgument) {
                     const auto& a = desc->entries[i];
                     const auto& b = entries[i];
                     if (a.binding != b.binding || a.visibility != b.visibility ||
-                        a.type != b.type) {
+                        a.buffer.type != b.buffer.type || a.sampler.type != b.sampler.type ||
+                        a.texture.sampleType != b.texture.sampleType) {
                         return false;
                     }
                 }