[dawn][native] Support BG with BGLs that use arraySize.
This expands the single BGL entry at the API surface into one
BindingInfo per element in the BGL's internal format. This lets BG
validation and code to store element unchanged. A test is added that one
must specify all the array elements when creating a BG with a BGL that
uses arraySize.
BGL validation is updated to require that elements created for the
arrays never go past maxBindingsPerBindGroup, and a test is added.
Bug: 393558555
Change-Id: I7e91040d7659d4db26bc1b46a38aef08e707be0c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/236554
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/BindGroupLayoutInternal.cpp b/src/dawn/native/BindGroupLayoutInternal.cpp
index 1501aac..146238b 100644
--- a/src/dawn/native/BindGroupLayoutInternal.cpp
+++ b/src/dawn/native/BindGroupLayoutInternal.cpp
@@ -402,7 +402,7 @@
};
}
-BindingInfo ConvertToBindingInfo(const UnpackedPtr<BindGroupLayoutEntry>& binding) {
+BindingInfo ConvertToBindingInfoNoArray(const UnpackedPtr<BindGroupLayoutEntry>& binding) {
BindingInfo bindingInfo;
bindingInfo.binding = BindingNumber(binding->binding);
bindingInfo.visibility = binding->visibility;
@@ -431,7 +431,8 @@
// This function handles the conversion of the API format for each binding info to Dawn's internal
// representation of them. This is also where the ExternalTextures are replaced and expanded in the
-// various bindings that are used internally in Dawn.
+// various bindings that are used internally in Dawn. Arrays are also expanded to individual
+// bindings here.
struct ExpandedBindingInfo {
ityp::vector<BindingIndex, BindingInfo> entries;
ExternalTextureBindingExpansionMap externalTextureBindingExpansions;
@@ -445,10 +446,16 @@
for (uint32_t i = 0; i < descriptor->entryCount; i++) {
UnpackedPtr<BindGroupLayoutEntry> entry = Unpack(&descriptor->entries[i]);
+ BindingIndex arraySize{1};
+ if (const auto* arraySizeInfo = entry.Get<BindGroupLayoutEntryArraySize>()) {
+ arraySize = BindingIndex(arraySizeInfo->arraySize);
+ }
+
// External textures are expanded from a texture_external into two sampled texture bindings
// and one uniform buffer binding. The original binding number is used for the first sampled
// texture.
if (entry.Get<ExternalTextureBindingLayout>()) {
+ DAWN_ASSERT(arraySize == BindingIndex{1});
dawn::native::ExternalTextureBindingExpansion bindingExpansion;
BindingInfo plane0Entry = CreateSampledTextureBindingForExternalTexture(
@@ -471,7 +478,16 @@
continue;
}
- result.entries.push_back(ConvertToBindingInfo(entry));
+ // Add one BindingInfo per element of the array with increasing indexInArray for backends to
+ // know which element it is when they need it, but also with increasing BindingNumber as the
+ // array takes consecutive binding numbers on the API side.
+ BindingInfo info = ConvertToBindingInfoNoArray(entry);
+ info.arraySize = arraySize;
+ for (BindingIndex indexInArray : Range(arraySize)) {
+ info.indexInArray = indexInArray;
+ result.entries.push_back(info);
+ info.binding++;
+ }
}
return result;
}
@@ -649,6 +665,8 @@
const BindingInfo& info = mBindingInfo[index];
recorder.Record(info.visibility);
+ recorder.Record(info.arraySize);
+ recorder.Record(info.indexInArray);
MatchVariant(
info.bindingLayout,
diff --git a/src/dawn/native/BindingInfo.cpp b/src/dawn/native/BindingInfo.cpp
index f0dc17f..65e6111 100644
--- a/src/dawn/native/BindingInfo.cpp
+++ b/src/dawn/native/BindingInfo.cpp
@@ -394,7 +394,7 @@
bool BindingInfo::operator==(const BindingInfo& other) const {
return binding == other.binding && visibility == other.visibility &&
- bindingLayout == other.bindingLayout;
+ arraySize == other.arraySize && bindingLayout == other.bindingLayout;
}
} // namespace dawn::native
diff --git a/src/dawn/native/BindingInfo.h b/src/dawn/native/BindingInfo.h
index 3f91c6b..1810654 100644
--- a/src/dawn/native/BindingInfo.h
+++ b/src/dawn/native/BindingInfo.h
@@ -146,6 +146,11 @@
BindingNumber binding;
wgpu::ShaderStage visibility;
+ // The size of the array this binding is part of. Each BindingInfy represents a single entry.
+ BindingIndex arraySize{1};
+ // The index of this entry in the array. Must be 0 if this entry is not in an array.
+ BindingIndex indexInArray{0};
+
std::variant<BufferBindingInfo,
SamplerBindingInfo,
TextureBindingInfo,
diff --git a/src/dawn/native/webgpu_absl_format.cpp b/src/dawn/native/webgpu_absl_format.cpp
index 0c7f597..559f776 100644
--- a/src/dawn/native/webgpu_absl_format.cpp
+++ b/src/dawn/native/webgpu_absl_format.cpp
@@ -116,34 +116,34 @@
const BindingInfo& value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s) {
- static const auto* const fmt =
- new absl::ParsedFormat<'u', 's', 's', 's'>("{ binding: %u, visibility: %s, %s: %s }");
+ s->Append(absl::StrFormat("{ binding: %u, visibility: %s, ", value.binding, value.visibility));
+ if (value.arraySize != BindingIndex(1)) {
+ s->Append(absl::StrFormat("arraySize: %u, indexInArray: %u, ", value.arraySize,
+ value.indexInArray));
+ }
+
MatchVariant(
value.bindingLayout,
[&](const BufferBindingInfo& layout) {
- s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding), value.visibility,
- BindingInfoType::Buffer, layout));
+ s->Append(absl::StrFormat("%s: %s ", BindingInfoType::Buffer, layout));
},
[&](const SamplerBindingInfo& layout) {
- s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding), value.visibility,
- BindingInfoType::Sampler, layout));
+ s->Append(absl::StrFormat("%s: %s ", BindingInfoType::Sampler, layout));
},
[&](const StaticSamplerBindingInfo& layout) {
- s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding), value.visibility,
- BindingInfoType::StaticSampler, layout));
+ s->Append(absl::StrFormat("%s: %s ", BindingInfoType::StaticSampler, layout));
},
[&](const TextureBindingInfo& layout) {
- s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding), value.visibility,
- BindingInfoType::Texture, layout));
+ s->Append(absl::StrFormat("%s: %s ", BindingInfoType::Texture, layout));
},
[&](const StorageTextureBindingInfo& layout) {
- s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding), value.visibility,
- BindingInfoType::StorageTexture, layout));
+ s->Append(absl::StrFormat("%s: %s ", BindingInfoType::StorageTexture, layout));
},
[&](const InputAttachmentBindingInfo& layout) {
- s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding), value.visibility,
- BindingInfoType::InputAttachment, layout));
+ s->Append(absl::StrFormat("%s: %s ", BindingInfoType::InputAttachment, layout));
});
+
+ s->Append(absl::StrFormat("}"));
return {true};
}
@@ -168,7 +168,7 @@
const TextureBindingInfo& value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s) {
- s->Append(absl::StrFormat("{sampleType: %s, viewDimension: %u, multisampled: %u}",
+ s->Append(absl::StrFormat("{sampleType: %s, viewDimension: %s, multisampled: %u}",
value.sampleType, value.viewDimension, value.multisampled));
return {true};
}
diff --git a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
index 75a4e76..6adc3a6 100644
--- a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -1201,6 +1201,46 @@
}
}
+// Test making that all the bindings for an BGL with arraySize must be specified.
+TEST_F(BindGroupValidationTest, AllArrayElementsMustBeSpecified) {
+ // Create the BGL with three entries for the array.
+ wgpu::BindGroupLayoutEntryArraySize arraySize;
+ arraySize.arraySize = 3;
+
+ wgpu::BindGroupLayoutEntry entry;
+ entry.binding = 1;
+ entry.visibility = wgpu::ShaderStage::Fragment;
+ entry.texture.sampleType = wgpu::TextureSampleType::Float;
+ entry.nextInChain = &arraySize;
+
+ wgpu::BindGroupLayoutDescriptor bglDesc;
+ bglDesc.entryCount = 1;
+ bglDesc.entries = &entry;
+ wgpu::BindGroupLayout bgl = device.CreateBindGroupLayout(&bglDesc);
+
+ // The texture used for the test.
+ wgpu::TextureDescriptor tDesc;
+ tDesc.size = {1, 1};
+ tDesc.format = wgpu::TextureFormat::RGBA8Unorm;
+ tDesc.usage = wgpu::TextureUsage::TextureBinding;
+ wgpu::Texture t = device.CreateTexture(&tDesc);
+
+ // Success case, making a BindGroup with all three entries
+ utils::MakeBindGroup(device, bgl,
+ {
+ {1, t.CreateView()},
+ {2, t.CreateView()},
+ {3, t.CreateView()},
+ });
+
+ // Error case, one element is missing
+ ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, bgl,
+ {
+ {1, t.CreateView()},
+ {3, t.CreateView()},
+ }));
+}
+
// Test what happens when the layout is an error.
TEST_F(BindGroupValidationTest, ErrorLayout) {
wgpu::BindGroupLayout goodLayout = utils::MakeBindGroupLayout(
