[dawn][native] Add tests for reflection of DynamicArrayKind::Undefined.
This could not previously be written because there was no way to use a
dynamic binding array in shaders without a type. arrayLength now allows
that so write the relevant tests.
Bug: 435317394
Change-Id: Ib9a33faf08c1fd372b23447089fe32be10898c2e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/259796
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/native/PipelineLayout.cpp b/src/dawn/native/PipelineLayout.cpp
index 739efb8..23a4ab8 100644
--- a/src/dawn/native/PipelineLayout.cpp
+++ b/src/dawn/native/PipelineLayout.cpp
@@ -430,7 +430,8 @@
if (dynamicArrays[group]->kind == wgpu::DynamicBindingKind::Undefined) {
dynamicArrays[group]->kind = array.kind;
} else {
- DAWN_INVALID_IF(dynamicArrays[group]->kind != array.kind,
+ DAWN_INVALID_IF(array.kind != wgpu::DynamicBindingKind::Undefined &&
+ dynamicArrays[group]->kind != array.kind,
"Dynamic array kind doesn't match for @group(%u) between shader "
"stages (%s vs. %s).",
group, dynamicArrays[group]->kind, array.kind);
diff --git a/src/dawn/tests/unittests/validation/DynamicBindingArrayValidationTests.cpp b/src/dawn/tests/unittests/validation/DynamicBindingArrayValidationTests.cpp
index 6cbff07..e031f4c 100644
--- a/src/dawn/tests/unittests/validation/DynamicBindingArrayValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/DynamicBindingArrayValidationTests.cpp
@@ -902,17 +902,39 @@
)"));
}
+// Test that a shader using only arrayLength on the dynamic binding array is compatible with any
+// DynamicBindingKind.
+TEST_F(DynamicBindingArrayTests, DynamicArrayKindWithoutTypeInfoValidWithAllLayouts) {
+ wgpu::ComputePipelineDescriptor csDesc;
+ csDesc.compute.module = utils::CreateShaderModule(device, R"(
+ enable chromium_experimental_dynamic_binding;
+ @group(0) @binding(0) var a : resource_binding;
+
+ @compute @workgroup_size(1) fn main() {
+ _ = arrayLength(a);
+ }
+ )");
+
+ // Check that it is compatible with DynamicBindingKind::SampledTexture.
+ {
+ wgpu::BindGroupLayout bgl = MakeBindGroupLayout(wgpu::DynamicBindingKind::SampledTexture);
+ csDesc.layout = utils::MakeBasicPipelineLayout(device, &bgl);
+ device.CreateComputePipeline(&csDesc);
+ }
+
+ // TODO(https://crbug.com/435317394): Add tests with additional DynamicBindingKind.
+}
+
// TODO(https://crbug.com/435317394): Add tests for the DynamicArrayKind. It is not possible to do
// it at the moment because we cannot reflect DynamicArrayKind::Undefined (would require referencing
// but not indexing the array) or any value that's not DynamicArrayKind::SampledTexture (no support
// in Dawn or Tint for other cases). Tests to add after that are:
// - The kind in the layout must match the deduced kind for the shader.
// - Case with a resource_binding
-// - A shader only referencing but not accessed with a resource_binding is valid to use with any
-// DynamicArrayKind in the layout.
// - An error is produced at shader module compilation time if it uses the same resource_binding
// with different DynamicArrayKinds.
//
+
// Test that BGL defaulting works with dynamic binding arrays.
TEST_F(DynamicBindingArrayTests, GetBGLSuccess) {
wgpu::ComputePipelineDescriptor csDesc;
@@ -1134,13 +1156,100 @@
}
}
+// Test that defaulting the layout when no type information is given for the dynamic binding array
+// is an error.
+TEST_F(DynamicBindingArrayTests, DefaultedDynamicArrayKindMustNotBeUndefined) {
+ wgpu::ComputePipelineDescriptor csDesc;
+ csDesc.compute.module = utils::CreateShaderModule(device, R"(
+ enable chromium_experimental_dynamic_binding;
+ @group(0) @binding(0) var a : resource_binding;
+
+ @compute @workgroup_size(1) fn main() {
+ _ = arrayLength(a);
+ }
+ )");
+
+ // Control case, explicitly giving a layout works
+ wgpu::BindGroupLayout bgl = MakeBindGroupLayout(wgpu::DynamicBindingKind::SampledTexture);
+ csDesc.layout = utils::MakeBasicPipelineLayout(device, &bgl);
+ device.CreateComputePipeline(&csDesc);
+
+ // Error case, defaulting the layout is not possible.
+ csDesc.layout = nullptr;
+ ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&csDesc));
+}
+
+// Test merging of DynamicBindingKind between VS and FS (FS typed case)
+TEST_F(DynamicBindingArrayTests, MergingUnknowVSDynamicArrayKindInFS) {
+ wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
+ enable chromium_experimental_dynamic_binding;
+
+ @group(0) @binding(0) var untyped : resource_binding;
+ @vertex fn vs() -> @builtin(position) vec4f {
+ _ = arrayLength(untyped);
+ return vec4f();
+ }
+
+ @group(0) @binding(0) var typed : resource_binding;
+ @fragment fn fs() -> @location(0) vec4f {
+ _ = hasBinding<texture_2d<f32>>(typed, 42);
+ return vec4f();
+ }
+ )");
+
+ utils::ComboRenderPipelineDescriptor pDesc;
+ pDesc.layout = nullptr;
+ pDesc.vertex.module = module;
+ pDesc.cFragment.module = module;
+ wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&pDesc);
+
+ // Check that this is a DynamicBindingKind::SampledTexture dynamic binding array.
+ wgpu::TextureDescriptor tDesc = {
+ .usage = wgpu::TextureUsage::TextureBinding,
+ .size = {1, 1},
+ .format = wgpu::TextureFormat::RGBA8Unorm,
+ };
+ wgpu::Texture tex = device.CreateTexture(&tDesc);
+ MakeBindGroup(pipeline.GetBindGroupLayout(0), 1, {{0, tex.CreateView()}});
+}
+
+// Test merging of DynamicBindingKind between VS and FS (VS typed case)
+TEST_F(DynamicBindingArrayTests, MergingUnknowFSDynamicArrayKindInVS) {
+ wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
+ enable chromium_experimental_dynamic_binding;
+
+ @group(0) @binding(0) var typed : resource_binding;
+ @vertex fn vs() -> @builtin(position) vec4f {
+ _ = hasBinding<texture_2d<f32>>(typed, 42);
+ return vec4f();
+ }
+
+ @group(0) @binding(0) var untyped : resource_binding;
+ @fragment fn fs() -> @location(0) vec4f {
+ _ = arrayLength(untyped);
+ return vec4f();
+ }
+ )");
+
+ utils::ComboRenderPipelineDescriptor pDesc;
+ pDesc.layout = nullptr;
+ pDesc.vertex.module = module;
+ pDesc.cFragment.module = module;
+ wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&pDesc);
+
+ // Check that this is a DynamicBindingKind::SampledTexture dynamic binding array.
+ wgpu::TextureDescriptor tDesc = {
+ .usage = wgpu::TextureUsage::TextureBinding,
+ .size = {1, 1},
+ .format = wgpu::TextureFormat::RGBA8Unorm,
+ };
+ wgpu::Texture tex = device.CreateTexture(&tDesc);
+ MakeBindGroup(pipeline.GetBindGroupLayout(0), 1, {{0, tex.CreateView()}});
+}
+
// TODO(https://crbug.com/435317394): Add tests for the DynamicArrayKind defaulting / merging
// between stages. Tests to add are:
// - Using two incompatible kinds between stages produces an error.
-// - Using DynamicArrayKind::Undefined in a stage and not the other makes the BGL default to the
-// non-Unknown DynamicArrayKind. (test both VS-FS and FS-VS directions to be safe).
-// - Test what happens when only DynamicArrayKind::Undefined is used in the defaulted layout. Is
-// that even valid?
// Test that pinning / unpinning is valid for a simple case. This is a control for the test that
// errors are produced when the feature is not enabled.
