Skip to content

Commit c166ac2

Browse files
KangzDawn LUCI CQ
authored andcommitted
[dawn][native] Disallow GetBGL(maxBindGroups - 1) with resource tables.
The resource tables use one bind group slot so it doesn't make sense to query the BGL at position maxBindGroups - 1 since it conceptually doesn't exist. Bug: 463925499 Change-Id: Ic2a16d43c36bdeef1e6fad8fdef6ecd57fa3827d Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/280438 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: dan sinclair <dsinclair@chromium.org>
1 parent 343ef28 commit c166ac2

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

src/dawn/native/Pipeline.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,18 @@ MaybeError PipelineBase::ValidateGetBindGroupLayout(BindGroupIndex groupIndex) {
342342
DAWN_TRY(GetDevice()->ValidateIsAlive());
343343
DAWN_TRY(GetDevice()->ValidateObject(this));
344344
DAWN_TRY(GetDevice()->ValidateObject(mLayout.Get()));
345-
DAWN_INVALID_IF(groupIndex >= kMaxBindGroupsTyped,
346-
"Bind group layout index (%u) exceeds the maximum number of bind groups (%u).",
347-
groupIndex, kMaxBindGroups);
345+
346+
if (mLayout->UsesResourceTable()) {
347+
DAWN_INVALID_IF(groupIndex >= kMaxBindGroupsTyped - BindGroupIndex(1),
348+
"Bind group layout index (%u) exceeds or equals the maximum number of bind "
349+
"groups (%u) - 1 (one slot reserved for the resource table).",
350+
groupIndex, kMaxBindGroups);
351+
} else {
352+
DAWN_INVALID_IF(groupIndex >= kMaxBindGroupsTyped,
353+
"Bind group layout index (%u) exceeds or equals the maximum number of bind "
354+
"groups (%u).",
355+
groupIndex, kMaxBindGroups);
356+
}
348357
return {};
349358
}
350359

src/dawn/tests/unittests/validation/ResourceTableValidationTests.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,41 @@ TEST_F(ResourceTableValidationTest,
350350
}
351351
}
352352

353+
// Test that GetBindGroupLayout is valid for one less BGL if resource tables are used.
354+
TEST_F(ResourceTableValidationTest, GetBindGroupLayoutValidForOneLessIndex) {
355+
// Default behavior case: GetBGL is valid until kMaxBindGroups - 1 when no resource table is
356+
// used.
357+
{
358+
wgpu::ComputePipelineDescriptor csDesc;
359+
csDesc.layout = nullptr;
360+
csDesc.compute.module = utils::CreateShaderModule(device, R"(
361+
enable chromium_experimental_resource_table;
362+
@compute @workgroup_size(1) fn main() {
363+
}
364+
)");
365+
wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&csDesc);
366+
367+
pipeline.GetBindGroupLayout(kMaxBindGroups - 1);
368+
ASSERT_DEVICE_ERROR(pipeline.GetBindGroupLayout(kMaxBindGroups));
369+
}
370+
371+
// Resource table case: GetBGL is valid until kMaxBindGroups - 2.
372+
{
373+
wgpu::ComputePipelineDescriptor csDesc;
374+
csDesc.layout = nullptr;
375+
csDesc.compute.module = utils::CreateShaderModule(device, R"(
376+
enable chromium_experimental_resource_table;
377+
@compute @workgroup_size(1) fn main() {
378+
_ = hasResource<texture_2d<f32>>(0);
379+
}
380+
)");
381+
wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&csDesc);
382+
383+
pipeline.GetBindGroupLayout(kMaxBindGroups - 2);
384+
ASSERT_DEVICE_ERROR(pipeline.GetBindGroupLayout(kMaxBindGroups - 1));
385+
}
386+
}
387+
353388
// Tests calling CommandEncoder::SetResourceTable
354389
TEST_F(ResourceTableValidationTest, CommandEncoder_SetResourceTable) {
355390
// Failure case: invalid encoder state

0 commit comments

Comments
 (0)