From 84ae2bfe3bcc3323ac60ca4eed64219aab2cd158 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 13 May 2020 17:05:55 +0000 Subject: [PATCH] Remove deprecated "Binding" types and members This removes the following types and members as well as fixup code and depraction tests for them: - wgpu::BindGroupLayoutBinding - wgpu::BindGroupLayoutDescriptor::bindingCount - wgpu::BindGroupLayoutDescriptor::bindings - wgpu::BindGroupBinding - wgpu::BindGroupDescriptor::bindingCount - wgpu::BindGroupDescriptor::bindings Bug: dawn:22 Change-Id: Ifc0e25107f3dcfbb850624cb362909f38c90bec2 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21680 Reviewed-by: Stephen White Reviewed-by: Austin Eng Commit-Queue: Corentin Wallez --- dawn.json | 8 +- generator/templates/webgpu.h | 4 - generator/templates/webgpu_cpp.h | 4 - src/dawn_native/Device.cpp | 38 +---- src/tests/end2end/DeprecatedAPITests.cpp | 155 ------------------ .../wire/WireMultipleDeviceTests.cpp | 2 +- 6 files changed, 7 insertions(+), 204 deletions(-) diff --git a/dawn.json b/dawn.json index b3dba41b6f..0a809c2e30 100644 --- a/dawn.json +++ b/dawn.json @@ -77,9 +77,7 @@ "members": [ {"name": "label", "type": "char", "annotation": "const*", "length": "strlen", "optional": true}, {"name": "layout", "type": "bind group layout"}, - {"name": "binding count", "type": "uint32_t", "default": 0}, - {"name": "bindings", "type": "bind group entry", "annotation": "const*", "length": "binding count"}, - {"name": "entry count", "type": "uint32_t", "default": 0}, + {"name": "entry count", "type": "uint32_t"}, {"name": "entries", "type": "bind group entry", "annotation": "const*", "length": "entry count"} ] }, @@ -106,9 +104,7 @@ "extensible": true, "members": [ {"name": "label", "type": "char", "annotation": "const*", "length": "strlen", "optional": true}, - {"name": "binding count", "type": "uint32_t", "default": 0}, - {"name": "bindings", "type": "bind group layout entry", "annotation": "const*", "length": "binding count"}, - {"name": "entry count", "type": "uint32_t", "default": 0}, + {"name": "entry count", "type": "uint32_t"}, {"name": "entries", "type": "bind group layout entry", "annotation": "const*", "length": "entry count"} ] }, diff --git a/generator/templates/webgpu.h b/generator/templates/webgpu.h index c3b2f50767..4e1c7c8ef3 100644 --- a/generator/templates/webgpu.h +++ b/generator/templates/webgpu.h @@ -114,10 +114,6 @@ typedef struct WGPUChainedStruct { {% endfor %} -// TODO(dawn:22): Remove this once users use the "Entry" version. -typedef WGPUBindGroupEntry WGPUBindGroupBinding; -typedef WGPUBindGroupLayoutEntry WGPUBindGroupLayoutBinding; - #ifdef __cplusplus extern "C" { #endif diff --git a/generator/templates/webgpu_cpp.h b/generator/templates/webgpu_cpp.h index b53f83e295..3dde1e803e 100644 --- a/generator/templates/webgpu_cpp.h +++ b/generator/templates/webgpu_cpp.h @@ -216,10 +216,6 @@ namespace wgpu { {% endfor %} - // TODO(dawn:22): Remove this once users use the "Entry" version. - using BindGroupBinding = BindGroupEntry; - using BindGroupLayoutBinding = BindGroupLayoutEntry; - } // namespace wgpu #endif // WEBGPU_CPP_H_ diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index c75eed7c04..efa8d2d86e 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -773,25 +773,10 @@ namespace dawn_native { MaybeError DeviceBase::CreateBindGroupInternal(BindGroupBase** result, const BindGroupDescriptor* descriptor) { DAWN_TRY(ValidateIsAlive()); - - // TODO(dawn:22): Remove this once users use entries/entryCount - BindGroupDescriptor fixedDescriptor = *descriptor; - if (fixedDescriptor.bindingCount != 0) { - if (fixedDescriptor.entryCount != 0) { - return DAWN_VALIDATION_ERROR("Cannot use bindings and entries at the same time"); - } else { - EmitDeprecationWarning( - "BindGroupEntry::bindings/bindingCount is deprecated, use entries/entryCount " - "instead"); - fixedDescriptor.entryCount = fixedDescriptor.bindingCount; - fixedDescriptor.entries = fixedDescriptor.bindings; - } - } - if (IsValidationEnabled()) { - DAWN_TRY(ValidateBindGroupDescriptor(this, &fixedDescriptor)); + DAWN_TRY(ValidateBindGroupDescriptor(this, descriptor)); } - DAWN_TRY_ASSIGN(*result, CreateBindGroupImpl(&fixedDescriptor)); + DAWN_TRY_ASSIGN(*result, CreateBindGroupImpl(descriptor)); return {}; } @@ -799,25 +784,10 @@ namespace dawn_native { BindGroupLayoutBase** result, const BindGroupLayoutDescriptor* descriptor) { DAWN_TRY(ValidateIsAlive()); - - // TODO(dawn:22): Remove this once users use entries/entryCount - BindGroupLayoutDescriptor fixedDescriptor = *descriptor; - if (fixedDescriptor.bindingCount != 0) { - if (fixedDescriptor.entryCount != 0) { - return DAWN_VALIDATION_ERROR("Cannot use bindings and entries at the same time"); - } else { - EmitDeprecationWarning( - "BindGroupLayoutEntry::bindings/bindingCount is deprecated, use " - "entries/entryCount instead"); - fixedDescriptor.entryCount = fixedDescriptor.bindingCount; - fixedDescriptor.entries = fixedDescriptor.bindings; - } - } - if (IsValidationEnabled()) { - DAWN_TRY(ValidateBindGroupLayoutDescriptor(this, &fixedDescriptor)); + DAWN_TRY(ValidateBindGroupLayoutDescriptor(this, descriptor)); } - DAWN_TRY_ASSIGN(*result, GetOrCreateBindGroupLayout(&fixedDescriptor)); + DAWN_TRY_ASSIGN(*result, GetOrCreateBindGroupLayout(descriptor)); return {}; } diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index 63770fd5de..e95c7e1a50 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/src/tests/end2end/DeprecatedAPITests.cpp @@ -82,8 +82,6 @@ TEST_P(DeprecationTests, BGLEntryTextureDimensionIsDeprecated) { entryDesc.textureDimension = wgpu::TextureViewDimension::e2D; wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 0; - bglDesc.bindings = nullptr; bglDesc.entryCount = 1; bglDesc.entries = &entryDesc; EXPECT_DEPRECATION_WARNING(device.CreateBindGroupLayout(&bglDesc)); @@ -97,8 +95,6 @@ TEST_P(DeprecationTests, BGLEntryTextureDimensionAndViewUndefinedEmitsNoWarning) entryDesc.type = wgpu::BindingType::Sampler; wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 0; - bglDesc.bindings = nullptr; bglDesc.entryCount = 1; bglDesc.entries = &entryDesc; device.CreateBindGroupLayout(&bglDesc); @@ -113,8 +109,6 @@ TEST_P(DeprecationTests, BGLEntryTextureAndViewDimensionIsInvalid) { entryDesc.viewDimension = wgpu::TextureViewDimension::e2D; wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 0; - bglDesc.bindings = nullptr; bglDesc.entryCount = 1; bglDesc.entries = &entryDesc; ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&bglDesc)); @@ -130,8 +124,6 @@ TEST_P(DeprecationTests, BGLEntryTextureDimensionStateTracking) { entryDesc.textureDimension = wgpu::TextureViewDimension::Cube; wgpu::BindGroupLayoutDescriptor bglDesc = {}; - bglDesc.bindingCount = 0; - bglDesc.bindings = nullptr; bglDesc.entryCount = 1; bglDesc.entries = &entryDesc; wgpu::BindGroupLayout layout; @@ -160,153 +152,6 @@ TEST_P(DeprecationTests, BGLEntryTextureDimensionStateTracking) { ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, arrayView}})); } -// Test for BindGroupLayout::bindings/bindingCount -> entries/entryCount - -// Test that creating a BGL with bindings emits a deprecation warning. -TEST_P(DeprecationTests, BGLDescBindingIsDeprecated) { - wgpu::BindGroupLayoutEntry entryDesc; - entryDesc.binding = 0; - entryDesc.visibility = wgpu::ShaderStage::None; - entryDesc.type = wgpu::BindingType::Sampler; - - wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 1; - bglDesc.bindings = &entryDesc; - bglDesc.entryCount = 0; - bglDesc.entries = nullptr; - EXPECT_DEPRECATION_WARNING(device.CreateBindGroupLayout(&bglDesc)); -} - -// Test that creating a BGL with both entries and bindings is an error -TEST_P(DeprecationTests, BGLDescBindingAndEntriesIsInvalid) { - wgpu::BindGroupLayoutEntry entryDesc; - entryDesc.binding = 0; - entryDesc.visibility = wgpu::ShaderStage::None; - entryDesc.type = wgpu::BindingType::Sampler; - - wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 1; - bglDesc.bindings = &entryDesc; - bglDesc.entryCount = 1; - bglDesc.entries = &entryDesc; - ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&bglDesc)); -} - -// Test that creating a BGL with both entries and bindings to 0 doesn't emit warnings -TEST_P(DeprecationTests, BGLDescBindingAndEntriesBothZeroEmitsNoWarning) { - wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 0; - bglDesc.bindings = nullptr; - bglDesc.entryCount = 0; - bglDesc.entries = nullptr; - device.CreateBindGroupLayout(&bglDesc); -} - -// Test that creating a BGL with bindings still does correct state tracking -TEST_P(DeprecationTests, BGLDescBindingStateTracking) { - wgpu::BindGroupLayoutEntry entryDesc; - entryDesc.binding = 0; - entryDesc.type = wgpu::BindingType::Sampler; - entryDesc.visibility = wgpu::ShaderStage::None; - - wgpu::BindGroupLayoutDescriptor bglDesc; - bglDesc.bindingCount = 1; - bglDesc.bindings = &entryDesc; - bglDesc.entryCount = 0; - bglDesc.entries = nullptr; - wgpu::BindGroupLayout layout; - EXPECT_DEPRECATION_WARNING(layout = device.CreateBindGroupLayout(&bglDesc)); - - // Test a case where if |bindings| wasn't taken into account, no validation error would happen - // because the layout would be empty - wgpu::BindGroupDescriptor badBgDesc; - badBgDesc.layout = layout; - badBgDesc.bindingCount = 0; - badBgDesc.bindings = nullptr; - badBgDesc.entryCount = 0; - badBgDesc.entries = nullptr; - ASSERT_DEVICE_ERROR(device.CreateBindGroup(&badBgDesc)); -} - -// Test for BindGroup::bindings/bindingCount -> entries/entryCount - -// Test that creating a BG with bindings emits a deprecation warning. -TEST_P(DeprecationTests, BGDescBindingIsDeprecated) { - wgpu::SamplerDescriptor samplerDesc = {}; - wgpu::Sampler sampler = device.CreateSampler(&samplerDesc); - - wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::Sampler}}); - - wgpu::BindGroupEntry entryDesc; - entryDesc.binding = 0; - entryDesc.sampler = sampler; - - wgpu::BindGroupDescriptor bgDesc; - bgDesc.layout = layout; - bgDesc.bindingCount = 1; - bgDesc.bindings = &entryDesc; - bgDesc.entryCount = 0; - bgDesc.entries = nullptr; - EXPECT_DEPRECATION_WARNING(device.CreateBindGroup(&bgDesc)); -} - -// Test that creating a BG with both entries and bindings is an error -TEST_P(DeprecationTests, BGDescBindingAndEntriesIsInvalid) { - wgpu::SamplerDescriptor samplerDesc = {}; - wgpu::Sampler sampler = device.CreateSampler(&samplerDesc); - - wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::Sampler}}); - - wgpu::BindGroupEntry entryDesc = {}; - entryDesc.binding = 0; - entryDesc.sampler = sampler; - - wgpu::BindGroupDescriptor bgDesc; - bgDesc.layout = layout; - bgDesc.bindingCount = 1; - bgDesc.bindings = &entryDesc; - bgDesc.entryCount = 1; - bgDesc.entries = &entryDesc; - ASSERT_DEVICE_ERROR(device.CreateBindGroup(&bgDesc)); -} - -// Test that creating a BG with both entries and bindings to 0 doesn't emit warnings -TEST_P(DeprecationTests, BGDescBindingAndEntriesBothZeroEmitsNoWarning) { - wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout(device, {}); - - wgpu::BindGroupDescriptor bgDesc; - bgDesc.layout = layout; - bgDesc.bindingCount = 0; - bgDesc.bindings = nullptr; - bgDesc.entryCount = 0; - bgDesc.entries = nullptr; - device.CreateBindGroup(&bgDesc); -} - -// Test that creating a BG with bindings still does correct state tracking -TEST_P(DeprecationTests, BGDescBindingStateTracking) { - wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout(device, {}); - - // Test a case where if |bindings| wasn't taken into account, no validation error would happen - // because it would match the empty layout. - wgpu::SamplerDescriptor samplerDesc = {}; - wgpu::Sampler sampler = device.CreateSampler(&samplerDesc); - - wgpu::BindGroupEntry entryDesc = {}; - entryDesc.binding = 0; - entryDesc.sampler = sampler; - - wgpu::BindGroupDescriptor bgDesc; - bgDesc.layout = layout; - bgDesc.bindingCount = 1; - bgDesc.bindings = &entryDesc; - bgDesc.entryCount = 0; - bgDesc.entries = nullptr; - EXPECT_DEPRECATION_WARNING(ASSERT_DEVICE_ERROR(device.CreateBindGroup(&bgDesc))); -} - // Tests for ShaderModuleDescriptor.code/codeSize -> ShaderModuleSPIRVDescriptor static const char kEmptyShader[] = R"(#version 450 diff --git a/src/tests/unittests/wire/WireMultipleDeviceTests.cpp b/src/tests/unittests/wire/WireMultipleDeviceTests.cpp index 0be91b45d6..3ba0da0ab4 100644 --- a/src/tests/unittests/wire/WireMultipleDeviceTests.cpp +++ b/src/tests/unittests/wire/WireMultipleDeviceTests.cpp @@ -174,7 +174,7 @@ TEST_F(WireMultipleDeviceTests, DifferentDeviceObjectCreationIsError) { wireA.FlushClient(); - std::array entries = {}; + std::array entries = {}; // Create a buffer on wire A. WGPUBufferDescriptor bufferDesc = {};