diff --git a/dawn.json b/dawn.json index 14bfbe8e8d..41ed56b114 100644 --- a/dawn.json +++ b/dawn.json @@ -170,14 +170,6 @@ "members": [ {"name": "binding", "type": "uint32_t"}, {"name": "visibility", "type": "shader stage"}, - - {"name": "type", "type": "binding type", "default": "undefined"}, - {"name": "has dynamic offset", "type": "bool", "default": "false"}, - {"name": "min buffer binding size", "type": "uint64_t", "default": "0"}, - {"name": "view dimension", "type": "texture view dimension", "default": "undefined"}, - {"name": "texture component type", "type": "texture component type", "default": "float"}, - {"name": "storage texture format", "type": "texture format", "default": "undefined"}, - {"name": "buffer", "type": "buffer binding layout"}, {"name": "sampler", "type": "sampler binding layout"}, {"name": "texture", "type": "texture binding layout"}, @@ -193,21 +185,6 @@ {"name": "entries", "type": "bind group layout entry", "annotation": "const*", "length": "entry count"} ] }, - "binding type": { - "category": "enum", - "values": [ - {"value": 0, "name": "undefined", "jsrepr": "undefined", "valid": false}, - {"value": 1, "name": "uniform buffer"}, - {"value": 2, "name": "storage buffer"}, - {"value": 3, "name": "readonly storage buffer"}, - {"value": 4, "name": "sampler"}, - {"value": 5, "name": "comparison sampler"}, - {"value": 6, "name": "sampled texture"}, - {"value": 7, "name": "multisampled texture"}, - {"value": 8, "name": "readonly storage texture"}, - {"value": 9, "name": "writeonly storage texture"} - ] - }, "blend component": { "category": "structure", "extensible": false, diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 4417610ec3..e07656a8d0 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -147,86 +147,10 @@ namespace dawn_native { } } - if (bindingMemberCount > 1) { + if (bindingMemberCount != 1) { return DAWN_VALIDATION_ERROR( - "Only one of buffer, sampler, texture, or storageTexture may be set for each " - "BindGroupLayoutEntry"); - } else if (bindingMemberCount == 1) { - if (entry.type != wgpu::BindingType::Undefined) { - return DAWN_VALIDATION_ERROR( - "BindGroupLayoutEntry type must be undefined if any of buffer, sampler, " - "texture, or storageTexture are set"); - } - } else if (bindingMemberCount == 0) { - // Deprecated validation path - 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."); - - DAWN_TRY(ValidateBindingType(entry.type)); - DAWN_TRY(ValidateTextureComponentType(entry.textureComponentType)); - - wgpu::TextureViewDimension viewDimension = wgpu::TextureViewDimension::e2D; - if (entry.viewDimension != wgpu::TextureViewDimension::Undefined) { - DAWN_TRY(ValidateTextureViewDimension(entry.viewDimension)); - viewDimension = entry.viewDimension; - } - - bool canBeDynamic = false; - - switch (entry.type) { - case wgpu::BindingType::StorageBuffer: - allowedStages &= ~wgpu::ShaderStage::Vertex; - DAWN_FALLTHROUGH; - case wgpu::BindingType::UniformBuffer: - case wgpu::BindingType::ReadonlyStorageBuffer: - canBeDynamic = true; - break; - - case wgpu::BindingType::SampledTexture: - break; - - case wgpu::BindingType::MultisampledTexture: - if (viewDimension != wgpu::TextureViewDimension::e2D) { - return DAWN_VALIDATION_ERROR("Multisampled binding must be 2D."); - } - if (entry.textureComponentType == - wgpu::TextureComponentType::DepthComparison) { - return DAWN_VALIDATION_ERROR( - "Multisampled binding must not be DepthComparison."); - } - break; - - case wgpu::BindingType::WriteonlyStorageTexture: - allowedStages &= ~wgpu::ShaderStage::Vertex; - DAWN_FALLTHROUGH; - case wgpu::BindingType::ReadonlyStorageTexture: - DAWN_TRY(ValidateStorageTextureFormat(device, entry.storageTextureFormat)); - DAWN_TRY(ValidateStorageTextureViewDimension(viewDimension)); - break; - - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: - break; - - case wgpu::BindingType::Undefined: - UNREACHABLE(); - } - - if (entry.hasDynamicOffset && !canBeDynamic) { - return DAWN_VALIDATION_ERROR("Binding type cannot be dynamic."); - } - - // Dynamic storage buffers aren't bounds checked properly in D3D12. Disallow them as - // unsafe until the bounds checks are implemented. - if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) && entry.hasDynamicOffset && - (entry.type == wgpu::BindingType::StorageBuffer || - entry.type == wgpu::BindingType::ReadonlyStorageBuffer)) { - return DAWN_VALIDATION_ERROR( - "Dynamic storage buffers are disallowed because they aren't secure yet. " - "See https://crbug.com/dawn/429"); - } + "Exactly one of buffer, sampler, texture, or storageTexture must be set for " + "each BindGroupLayoutEntry"); } if (!IsSubset(entry.visibility, allowedStages)) { @@ -269,117 +193,17 @@ namespace dawn_native { } } - // TODO(dawn:527): Once the deprecated BindGroupLayoutEntry path has been removed, this can - // turn into a simple `binding.buffer.type != wgpu::BufferBindingType::Undefined` check. bool IsBufferBinding(const BindGroupLayoutEntry& binding) { - if (binding.buffer.type != wgpu::BufferBindingType::Undefined) { - return true; - } else if (binding.sampler.type != wgpu::SamplerBindingType::Undefined) { - return false; - } else if (binding.texture.sampleType != wgpu::TextureSampleType::Undefined) { - return false; - } else if (binding.storageTexture.access != wgpu::StorageTextureAccess::Undefined) { - return false; - } - - // Deprecated path - switch (binding.type) { - case wgpu::BindingType::UniformBuffer: - case wgpu::BindingType::StorageBuffer: - case wgpu::BindingType::ReadonlyStorageBuffer: - return true; - case wgpu::BindingType::SampledTexture: - case wgpu::BindingType::MultisampledTexture: - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: - case wgpu::BindingType::ReadonlyStorageTexture: - case wgpu::BindingType::WriteonlyStorageTexture: - case wgpu::BindingType::Undefined: - return false; - } + return binding.buffer.type != wgpu::BufferBindingType::Undefined; } bool BindingHasDynamicOffset(const BindGroupLayoutEntry& binding) { if (binding.buffer.type != wgpu::BufferBindingType::Undefined) { return binding.buffer.hasDynamicOffset; - } else if (binding.sampler.type != wgpu::SamplerBindingType::Undefined) { - return false; - } else if (binding.texture.sampleType != wgpu::TextureSampleType::Undefined) { - return false; - } else if (binding.storageTexture.access != wgpu::StorageTextureAccess::Undefined) { - return false; - } - - return binding.hasDynamicOffset; - } - - bool SortBindingsCompare(const BindGroupLayoutEntry& a, const BindGroupLayoutEntry& b) { - const bool aIsBuffer = IsBufferBinding(a); - const bool bIsBuffer = IsBufferBinding(b); - if (aIsBuffer != bIsBuffer) { - // Always place buffers first. - return aIsBuffer; - } else { - if (aIsBuffer) { - bool aHasDynamicOffset = BindingHasDynamicOffset(a); - bool bHasDynamicOffset = BindingHasDynamicOffset(b); - ASSERT(bIsBuffer); - if (aHasDynamicOffset != bHasDynamicOffset) { - // Buffers with dynamic offsets should come before those without. - // This makes it easy to iterate over the dynamic buffer bindings - // [0, dynamicBufferCount) during validation. - return aHasDynamicOffset; - } - if (aHasDynamicOffset) { - ASSERT(bHasDynamicOffset); - ASSERT(a.binding != b.binding); - // Above, we ensured that dynamic buffers are first. Now, ensure that - // dynamic buffer bindings are in increasing order. This is because dynamic - // buffer offsets are applied in increasing order of binding number. - return a.binding < b.binding; - } - } - // Otherwise, sort by type. - if (a.type != b.type) { - return a.type < b.type; - } - } - if (a.visibility != b.visibility) { - return a.visibility < b.visibility; - } - if (a.viewDimension != b.viewDimension) { - return a.viewDimension < b.viewDimension; - } - if (a.textureComponentType != b.textureComponentType) { - return a.textureComponentType < b.textureComponentType; - } - if (a.storageTextureFormat != b.storageTextureFormat) { - return a.storageTextureFormat < b.storageTextureFormat; - } - if (a.minBufferBindingSize != b.minBufferBindingSize) { - return a.minBufferBindingSize < b.minBufferBindingSize; } return false; } - // This is a utility function to help ASSERT that the BGL-binding comparator places buffers - // first. - bool CheckBufferBindingsFirst(ityp::span bindings) { - BindingIndex lastBufferIndex{0}; - BindingIndex firstNonBufferIndex = std::numeric_limits::max(); - for (BindingIndex i{0}; i < bindings.size(); ++i) { - if (bindings[i].bindingType == BindingInfoType::Buffer) { - lastBufferIndex = std::max(i, lastBufferIndex); - } else { - firstNonBufferIndex = std::min(i, firstNonBufferIndex); - } - } - - // If there are no buffers, then |lastBufferIndex| is initialized to 0 and - // |firstNonBufferIndex| gets set to 0. - return firstNonBufferIndex >= lastBufferIndex; - } - BindingInfo CreateBindGroupLayoutInfo(const BindGroupLayoutEntry& binding) { BindingInfo bindingInfo; bindingInfo.binding = BindingNumber(binding.binding); @@ -405,92 +229,108 @@ namespace dawn_native { if (binding.storageTexture.viewDimension == wgpu::TextureViewDimension::Undefined) { bindingInfo.storageTexture.viewDimension = wgpu::TextureViewDimension::e2D; } - } else { - // Deprecated entry layout. - switch (binding.type) { - case wgpu::BindingType::UniformBuffer: - bindingInfo.bindingType = BindingInfoType::Buffer; - bindingInfo.buffer.type = wgpu::BufferBindingType::Uniform; - bindingInfo.buffer.hasDynamicOffset = binding.hasDynamicOffset; - bindingInfo.buffer.minBindingSize = binding.minBufferBindingSize; - break; - case wgpu::BindingType::StorageBuffer: - bindingInfo.bindingType = BindingInfoType::Buffer; - bindingInfo.buffer.type = wgpu::BufferBindingType::Storage; - bindingInfo.buffer.hasDynamicOffset = binding.hasDynamicOffset; - bindingInfo.buffer.minBindingSize = binding.minBufferBindingSize; - break; - case wgpu::BindingType::ReadonlyStorageBuffer: - bindingInfo.bindingType = BindingInfoType::Buffer; - bindingInfo.buffer.type = wgpu::BufferBindingType::ReadOnlyStorage; - bindingInfo.buffer.hasDynamicOffset = binding.hasDynamicOffset; - bindingInfo.buffer.minBindingSize = binding.minBufferBindingSize; - break; - - case wgpu::BindingType::Sampler: - bindingInfo.bindingType = BindingInfoType::Sampler; - bindingInfo.sampler.type = wgpu::SamplerBindingType::Filtering; - break; - case wgpu::BindingType::ComparisonSampler: - bindingInfo.bindingType = BindingInfoType::Sampler; - bindingInfo.sampler.type = wgpu::SamplerBindingType::Comparison; - break; - - case wgpu::BindingType::MultisampledTexture: - bindingInfo.texture.multisampled = true; - DAWN_FALLTHROUGH; - case wgpu::BindingType::SampledTexture: - bindingInfo.bindingType = BindingInfoType::Texture; - bindingInfo.texture.viewDimension = binding.viewDimension; - if (binding.viewDimension == wgpu::TextureViewDimension::Undefined) { - bindingInfo.texture.viewDimension = wgpu::TextureViewDimension::e2D; - } - - switch (binding.textureComponentType) { - case wgpu::TextureComponentType::Float: - bindingInfo.texture.sampleType = wgpu::TextureSampleType::Float; - break; - case wgpu::TextureComponentType::Uint: - bindingInfo.texture.sampleType = wgpu::TextureSampleType::Uint; - break; - case wgpu::TextureComponentType::Sint: - bindingInfo.texture.sampleType = wgpu::TextureSampleType::Sint; - break; - case wgpu::TextureComponentType::DepthComparison: - bindingInfo.texture.sampleType = wgpu::TextureSampleType::Depth; - break; - } - break; - - case wgpu::BindingType::ReadonlyStorageTexture: - bindingInfo.bindingType = BindingInfoType::StorageTexture; - bindingInfo.storageTexture.access = wgpu::StorageTextureAccess::ReadOnly; - bindingInfo.storageTexture.format = binding.storageTextureFormat; - bindingInfo.storageTexture.viewDimension = binding.viewDimension; - if (binding.viewDimension == wgpu::TextureViewDimension::Undefined) { - bindingInfo.storageTexture.viewDimension = - wgpu::TextureViewDimension::e2D; - } - break; - case wgpu::BindingType::WriteonlyStorageTexture: - bindingInfo.bindingType = BindingInfoType::StorageTexture; - bindingInfo.storageTexture.access = wgpu::StorageTextureAccess::WriteOnly; - bindingInfo.storageTexture.format = binding.storageTextureFormat; - bindingInfo.storageTexture.viewDimension = binding.viewDimension; - if (binding.viewDimension == wgpu::TextureViewDimension::Undefined) { - bindingInfo.storageTexture.viewDimension = - wgpu::TextureViewDimension::e2D; - } - break; - - case wgpu::BindingType::Undefined: - UNREACHABLE(); - } } return bindingInfo; } + bool SortBindingsCompare(const BindGroupLayoutEntry& a, const BindGroupLayoutEntry& b) { + const bool aIsBuffer = IsBufferBinding(a); + const bool bIsBuffer = IsBufferBinding(b); + if (aIsBuffer != bIsBuffer) { + // Always place buffers first. + return aIsBuffer; + } + + if (aIsBuffer) { + bool aHasDynamicOffset = BindingHasDynamicOffset(a); + bool bHasDynamicOffset = BindingHasDynamicOffset(b); + ASSERT(bIsBuffer); + if (aHasDynamicOffset != bHasDynamicOffset) { + // Buffers with dynamic offsets should come before those without. + // This makes it easy to iterate over the dynamic buffer bindings + // [0, dynamicBufferCount) during validation. + return aHasDynamicOffset; + } + if (aHasDynamicOffset) { + ASSERT(bHasDynamicOffset); + ASSERT(a.binding != b.binding); + // Above, we ensured that dynamic buffers are first. Now, ensure that + // dynamic buffer bindings are in increasing order. This is because dynamic + // buffer offsets are applied in increasing order of binding number. + return a.binding < b.binding; + } + } + + // This applies some defaults and gives us a single value to check for the binding type. + BindingInfo aInfo = CreateBindGroupLayoutInfo(a); + BindingInfo bInfo = CreateBindGroupLayoutInfo(b); + + // Sort by type. + if (aInfo.bindingType != bInfo.bindingType) { + return aInfo.bindingType < bInfo.bindingType; + } + + if (a.visibility != b.visibility) { + return a.visibility < b.visibility; + } + + switch (aInfo.bindingType) { + case BindingInfoType::Buffer: + if (aInfo.buffer.minBindingSize != bInfo.buffer.minBindingSize) { + return aInfo.buffer.minBindingSize < bInfo.buffer.minBindingSize; + } + break; + case BindingInfoType::Sampler: + if (aInfo.sampler.type != bInfo.sampler.type) { + return aInfo.sampler.type < bInfo.sampler.type; + } + break; + case BindingInfoType::Texture: + if (aInfo.texture.multisampled != bInfo.texture.multisampled) { + return aInfo.texture.multisampled < bInfo.texture.multisampled; + } + if (aInfo.texture.viewDimension != bInfo.texture.viewDimension) { + return aInfo.texture.viewDimension < bInfo.texture.viewDimension; + } + if (aInfo.texture.sampleType != bInfo.texture.sampleType) { + return aInfo.texture.sampleType < bInfo.texture.sampleType; + } + break; + case BindingInfoType::StorageTexture: + if (aInfo.storageTexture.access != bInfo.storageTexture.access) { + return aInfo.storageTexture.access < bInfo.storageTexture.access; + } + if (aInfo.storageTexture.viewDimension != bInfo.storageTexture.viewDimension) { + return aInfo.storageTexture.viewDimension < + bInfo.storageTexture.viewDimension; + } + if (aInfo.storageTexture.format != bInfo.storageTexture.format) { + return aInfo.storageTexture.format < bInfo.storageTexture.format; + } + break; + } + return false; + } + + // This is a utility function to help ASSERT that the BGL-binding comparator places buffers + // first. + bool CheckBufferBindingsFirst(ityp::span bindings) { + BindingIndex lastBufferIndex{0}; + BindingIndex firstNonBufferIndex = std::numeric_limits::max(); + for (BindingIndex i{0}; i < bindings.size(); ++i) { + if (bindings[i].bindingType == BindingInfoType::Buffer) { + lastBufferIndex = std::max(i, lastBufferIndex); + } else { + firstNonBufferIndex = std::min(i, firstNonBufferIndex); + } + } + + // If there are no buffers, then |lastBufferIndex| is initialized to 0 and + // |firstNonBufferIndex| gets set to 0. + return firstNonBufferIndex >= lastBufferIndex; + } + } // namespace // BindGroupLayoutBase diff --git a/src/dawn_native/BindingInfo.cpp b/src/dawn_native/BindingInfo.cpp index 6c7f9268f1..3b13a33f65 100644 --- a/src/dawn_native/BindingInfo.cpp +++ b/src/dawn_native/BindingInfo.cpp @@ -56,50 +56,6 @@ namespace dawn_native { perStageBindingCountMember = &PerStageBindingCounts::sampledTextureCount; } else if (entry.storageTexture.access != wgpu::StorageTextureAccess::Undefined) { perStageBindingCountMember = &PerStageBindingCounts::storageTextureCount; - } else { - // Deprecated path. - switch (entry.type) { - case wgpu::BindingType::UniformBuffer: - ++bindingCounts->bufferCount; - if (entry.hasDynamicOffset) { - ++bindingCounts->dynamicUniformBufferCount; - } - if (entry.minBufferBindingSize == 0) { - ++bindingCounts->unverifiedBufferCount; - } - perStageBindingCountMember = &PerStageBindingCounts::uniformBufferCount; - break; - - case wgpu::BindingType::StorageBuffer: - case wgpu::BindingType::ReadonlyStorageBuffer: - ++bindingCounts->bufferCount; - if (entry.hasDynamicOffset) { - ++bindingCounts->dynamicStorageBufferCount; - } - if (entry.minBufferBindingSize == 0) { - ++bindingCounts->unverifiedBufferCount; - } - perStageBindingCountMember = &PerStageBindingCounts::storageBufferCount; - break; - - case wgpu::BindingType::SampledTexture: - case wgpu::BindingType::MultisampledTexture: - perStageBindingCountMember = &PerStageBindingCounts::sampledTextureCount; - break; - - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: - perStageBindingCountMember = &PerStageBindingCounts::samplerCount; - break; - - case wgpu::BindingType::ReadonlyStorageTexture: - case wgpu::BindingType::WriteonlyStorageTexture: - perStageBindingCountMember = &PerStageBindingCounts::storageTextureCount; - break; - - case wgpu::BindingType::Undefined: - UNREACHABLE(); - } } ASSERT(perStageBindingCountMember != nullptr); diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index 573d0957cf..8205e3bacd 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/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(), diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index 6a8b6de9f9..601f4210fb 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/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 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 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 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 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 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); } } diff --git a/src/tests/unittests/validation/StorageTextureValidationTests.cpp b/src/tests/unittests/validation/StorageTextureValidationTests.cpp index 68270151e3..6660ca6a1f 100644 --- a/src/tests/unittests/validation/StorageTextureValidationTests.cpp +++ b/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 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) { diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp index a72e8ef7a4..4d21fa772d 100644 --- a/src/tests/unittests/wire/WireArgumentTests.cpp +++ b/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_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; } } diff --git a/src/utils/ComboRenderPipelineDescriptor.h b/src/utils/ComboRenderPipelineDescriptor.h index ade8375878..036ce1c711 100644 --- a/src/utils/ComboRenderPipelineDescriptor.h +++ b/src/utils/ComboRenderPipelineDescriptor.h @@ -23,6 +23,7 @@ namespace utils { + // Primarily used by tests to easily set up the vertex buffer state portion of a RenderPipeline. class ComboVertexStateDescriptor { public: ComboVertexStateDescriptor(); @@ -37,7 +38,6 @@ namespace utils { std::array cAttributes; }; - // For creating the new style of render pipeline descriptors class ComboRenderPipelineDescriptor : public wgpu::RenderPipelineDescriptor { public: ComboRenderPipelineDescriptor(); diff --git a/src/utils/WGPUHelpers.cpp b/src/utils/WGPUHelpers.cpp index c560c39113..7f29e62b35 100644 --- a/src/utils/WGPUHelpers.cpp +++ b/src/utils/WGPUHelpers.cpp @@ -296,25 +296,6 @@ namespace utils { storageTexture.viewDimension = textureViewDimension; } - BindingLayoutEntryInitializationHelper::BindingLayoutEntryInitializationHelper( - uint32_t entryBinding, - wgpu::ShaderStage entryVisibility, - wgpu::BindingType entryType, - bool bufferHasDynamicOffset, - uint64_t bufferMinBindingSize, - wgpu::TextureViewDimension textureViewDimension, - wgpu::TextureComponentType textureComponent, - wgpu::TextureFormat storageFormat) { - binding = entryBinding; - visibility = entryVisibility; - type = entryType; - hasDynamicOffset = bufferHasDynamicOffset; - minBufferBindingSize = bufferMinBindingSize; - viewDimension = textureViewDimension; - textureComponentType = textureComponent; - storageTextureFormat = storageFormat; - } - BindingLayoutEntryInitializationHelper::BindingLayoutEntryInitializationHelper( const wgpu::BindGroupLayoutEntry& entry) : wgpu::BindGroupLayoutEntry(entry) { diff --git a/src/utils/WGPUHelpers.h b/src/utils/WGPUHelpers.h index 26c01fea0f..1ae082ea2b 100644 --- a/src/utils/WGPUHelpers.h +++ b/src/utils/WGPUHelpers.h @@ -127,16 +127,6 @@ namespace utils { wgpu::TextureFormat format, wgpu::TextureViewDimension viewDimension = wgpu::TextureViewDimension::e2D); - // Backwards compat support for the deprecated path - BindingLayoutEntryInitializationHelper( - uint32_t entryBinding, - wgpu::ShaderStage entryVisibility, - wgpu::BindingType entryType, - bool bufferHasDynamicOffset = false, - uint64_t bufferMinBindingSize = 0, - wgpu::TextureViewDimension textureViewDimension = wgpu::TextureViewDimension::Undefined, - wgpu::TextureComponentType textureComponent = wgpu::TextureComponentType::Float, - wgpu::TextureFormat storageFormat = wgpu::TextureFormat::Undefined); BindingLayoutEntryInitializationHelper(const wgpu::BindGroupLayoutEntry& entry); };