[Static samplers] Pivot to having StaticSamplerBindingLayout
This CL moves static samplers away from being specified via a chained
struct on SamplerLayoutBinding to a chained struct on
BindGroupLayoutEntry, similarly to ExternalTextureBindingLayout. This
organization avoids redundantly specifying the sampler type and makes
the conceptual relationships more clear: a SamplerLayoutBinding is for
a sampler supplied dynamically, whereas the StaticSamplerBindingLayout
is for samplers supplied statically.
As a bonus, this new code organization directly points the way to a
couple next steps that need to happen:
* Incorporation of validation of the number of static samplers
* Populating BindingInfo with the static sampler's sampler object
This CL leaves TODOs for those next steps.
Bug: dawn:2463
Change-Id: I9974b5678051ea38ebab57e801affc474d13938b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/181042
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json
index 64491b3..9edf18c 100644
--- a/src/dawn/dawn.json
+++ b/src/dawn/dawn.json
@@ -375,10 +375,10 @@
{"name": "type", "type": "sampler binding type", "default": "undefined"}
]
},
- "static sampler": {
+ "static sampler binding layout": {
"category": "structure",
"chained": "in",
- "chain roots": ["sampler binding layout"],
+ "chain roots": ["bind group layout entry"],
"tags": ["dawn"],
"members": [
{"name": "sampler", "type": "sampler"}
@@ -3689,7 +3689,7 @@
{"value": 1212, "name": "shared fence MTL shared event descriptor", "tags": ["dawn", "native"]},
{"value": 1213, "name": "shared fence MTL shared event export info", "tags": ["dawn", "native"]},
{"value": 1214, "name": "shared buffer memory D3D12 resource descriptor", "tags": ["dawn", "native"]},
- {"value": 1215, "name": "static sampler", "tags": ["dawn"]}
+ {"value": 1215, "name": "static sampler binding layout", "tags": ["dawn"]}
]
},
"texture": {
diff --git a/src/dawn/native/BindGroupLayoutInternal.cpp b/src/dawn/native/BindGroupLayoutInternal.cpp
index 46fc853..0755617 100644
--- a/src/dawn/native/BindGroupLayoutInternal.cpp
+++ b/src/dawn/native/BindGroupLayoutInternal.cpp
@@ -45,7 +45,6 @@
#include "dawn/native/ObjectContentHasher.h"
#include "dawn/native/ObjectType_autogen.h"
#include "dawn/native/PerStage.h"
-#include "dawn/native/PhysicalDevice.h"
#include "dawn/native/Sampler.h"
#include "dawn/native/ValidationUtils_autogen.h"
@@ -122,21 +121,6 @@
if (entry->sampler.type != wgpu::SamplerBindingType::Undefined) {
bindingMemberCount++;
DAWN_TRY(ValidateSamplerBindingType(entry->sampler.type));
-
- UnpackedPtr<SamplerBindingLayout> samplerLayout;
- DAWN_TRY_ASSIGN(samplerLayout, ValidateAndUnpack(&entry->sampler));
-
- auto staticSampler = samplerLayout.Get<StaticSampler>();
- if (staticSampler) {
- DAWN_INVALID_IF(!device->HasFeature(Feature::StaticSamplers),
- "Static samplers used without the %s feature enabled.",
- device->GetPhysicalDevice()
- ->GetInstance()
- ->GetFeatureInfo(ToAPI(Feature::StaticSamplers))
- ->name);
-
- DAWN_TRY(device->ValidateObject(staticSampler->sampler));
- }
}
if (entry->texture.sampleType != wgpu::TextureSampleType::Undefined) {
@@ -201,6 +185,16 @@
}
}
+ if (auto* staticSamplerBindingLayout = entry.Get<StaticSamplerBindingLayout>()) {
+ bindingMemberCount++;
+
+ DAWN_INVALID_IF(!device->HasFeature(Feature::StaticSamplers),
+ "Static samplers used without the %s feature enabled.",
+ wgpu::FeatureName::StaticSamplers);
+
+ DAWN_TRY(device->ValidateObject(staticSamplerBindingLayout->sampler));
+ }
+
if (auto* externalTextureBindingLayout = entry.Get<ExternalTextureBindingLayout>()) {
bindingMemberCount++;
}
@@ -410,6 +404,8 @@
bindingLayout.viewDimension = wgpu::TextureViewDimension::e2D;
}
bindingInfo.bindingLayout = bindingLayout;
+ } else if (auto* staticSamplerBindingLayout = binding.Get<StaticSamplerBindingLayout>()) {
+ // TODO(crbug.com/dawn/2463): Populate BindingInfo for this entry.
} else {
DAWN_UNREACHABLE();
}
diff --git a/src/dawn/native/BindingInfo.cpp b/src/dawn/native/BindingInfo.cpp
index ae8f3e6..879104e 100644
--- a/src/dawn/native/BindingInfo.cpp
+++ b/src/dawn/native/BindingInfo.cpp
@@ -86,10 +86,10 @@
perStageBindingCountMember = &PerStageBindingCounts::sampledTextureCount;
} else if (entry->storageTexture.access != wgpu::StorageTextureAccess::Undefined) {
perStageBindingCountMember = &PerStageBindingCounts::storageTextureCount;
- } else {
- if (auto* externalTextureBindingLayout = entry.Get<ExternalTextureBindingLayout>()) {
- perStageBindingCountMember = &PerStageBindingCounts::externalTextureCount;
- }
+ } else if (auto* externalTextureBindingLayout = entry.Get<ExternalTextureBindingLayout>()) {
+ perStageBindingCountMember = &PerStageBindingCounts::externalTextureCount;
+ } else if (auto* staticSamplerBindingLayout = entry.Get<StaticSamplerBindingLayout>()) {
+ perStageBindingCountMember = &PerStageBindingCounts::staticSamplerCount;
}
DAWN_ASSERT(perStageBindingCountMember != nullptr);
@@ -115,6 +115,7 @@
bindingCounts->perStage[stage].uniformBufferCount += rhs.perStage[stage].uniformBufferCount;
bindingCounts->perStage[stage].externalTextureCount +=
rhs.perStage[stage].externalTextureCount;
+ bindingCounts->perStage[stage].staticSamplerCount += rhs.perStage[stage].staticSamplerCount;
}
}
@@ -164,12 +165,14 @@
bindingCounts.perStage[stage].externalTextureCount, stage,
limits.v1.maxSampledTexturesPerShaderStage);
+ // TODO(crbug.com/dawn/2463): Account for static samplers here.
DAWN_INVALID_IF(
bindingCounts.perStage[stage].samplerCount > limits.v1.maxSamplersPerShaderStage,
"The number of samplers (%u) in the %s stage exceeds the maximum per-stage limit "
"(%u).",
bindingCounts.perStage[stage].samplerCount, stage, limits.v1.maxSamplersPerShaderStage);
+ // TODO(crbug.com/dawn/2463): Account for static samplers here.
DAWN_INVALID_IF(
bindingCounts.perStage[stage].samplerCount +
(bindingCounts.perStage[stage].externalTextureCount *
diff --git a/src/dawn/native/BindingInfo.h b/src/dawn/native/BindingInfo.h
index 16970f6..1eba544 100644
--- a/src/dawn/native/BindingInfo.h
+++ b/src/dawn/native/BindingInfo.h
@@ -83,6 +83,7 @@
uint32_t storageTextureCount;
uint32_t uniformBufferCount;
uint32_t externalTextureCount;
+ uint32_t staticSamplerCount;
};
struct BindingCounts {
diff --git a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
index 64e01f3..c856614 100644
--- a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -1753,31 +1753,14 @@
});
}
-// Tests that chaining an arbitrary struct onto a sampler binding layout raises
-// an error.
-TEST_F(BindGroupLayoutValidationTest, SamplerBindingLayoutInvalidChainedStruct) {
- wgpu::BindGroupLayoutEntry binding = {};
- binding.binding = 0;
- binding.sampler.type = wgpu::SamplerBindingType::Filtering;
- wgpu::ExternalTextureBindingLayout invalidChainedStruct = {};
- binding.sampler.nextInChain = &invalidChainedStruct;
-
- wgpu::BindGroupLayoutDescriptor desc = {};
- desc.entryCount = 1;
- desc.entries = &binding;
-
- ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&desc));
-}
-
// Tests that creating a bind group layout with a valid static sampler raises an error
// if the required feature is not enabled.
TEST_F(BindGroupLayoutValidationTest, StaticSamplerNotSupportedWithoutFeatureEnabled) {
wgpu::BindGroupLayoutEntry binding = {};
binding.binding = 0;
- binding.sampler.type = wgpu::SamplerBindingType::Filtering;
- wgpu::StaticSampler staticSampler = {};
- staticSampler.sampler = device.CreateSampler();
- binding.sampler.nextInChain = &staticSampler;
+ wgpu::StaticSamplerBindingLayout staticSamplerBinding = {};
+ staticSamplerBinding.sampler = device.CreateSampler();
+ binding.nextInChain = &staticSamplerBinding;
wgpu::BindGroupLayoutDescriptor desc = {};
desc.entryCount = 1;
@@ -1801,10 +1784,9 @@
TEST_F(BindGroupLayoutWithStaticSamplersValidationTest, StaticSamplerSupportedWhenFeatureEnabled) {
wgpu::BindGroupLayoutEntry binding = {};
binding.binding = 0;
- binding.sampler.type = wgpu::SamplerBindingType::Filtering;
- wgpu::StaticSampler staticSampler = {};
- staticSampler.sampler = device.CreateSampler();
- binding.sampler.nextInChain = &staticSampler;
+ wgpu::StaticSamplerBindingLayout staticSamplerBinding = {};
+ staticSamplerBinding.sampler = device.CreateSampler();
+ binding.nextInChain = &staticSamplerBinding;
wgpu::BindGroupLayoutDescriptor desc = {};
desc.entryCount = 1;
@@ -1818,8 +1800,7 @@
TEST_F(BindGroupLayoutWithStaticSamplersValidationTest, StaticSamplerWithInvalidSamplerObject) {
wgpu::BindGroupLayoutEntry binding = {};
binding.binding = 0;
- binding.sampler.type = wgpu::SamplerBindingType::Filtering;
- wgpu::StaticSampler staticSampler = {};
+ wgpu::StaticSamplerBindingLayout staticSamplerBinding = {};
wgpu::SamplerDescriptor samplerDesc;
samplerDesc.minFilter = static_cast<wgpu::FilterMode>(0xFFFFFFFF);
@@ -1827,8 +1808,8 @@
wgpu::Sampler errorSampler;
ASSERT_DEVICE_ERROR(errorSampler = device.CreateSampler(&samplerDesc));
- staticSampler.sampler = errorSampler;
- binding.sampler.nextInChain = &staticSampler;
+ staticSamplerBinding.sampler = errorSampler;
+ binding.nextInChain = &staticSamplerBinding;
wgpu::BindGroupLayoutDescriptor desc = {};
desc.entryCount = 1;
