Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds max_dynamic_buffers_per_pipeline tests #2595

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adds max_dynamic_buffers_per_pipeline tests #2595

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 119 additions & 7 deletions src/webgpu/api/validation/createBindGroupLayout.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ function* pickExtraBindingTypesForPerStage(entry: BGLEntry, extraTypeSame: boole
}
}

const kMaxResourcesCases = kUnitCaseParamsBuilder
const kMaxResourcesPerStageCases = kUnitCaseParamsBuilder
.combine('maxedEntry', allBindingEntries(false))
.beginSubcases()
.combine('maxedVisibility', kShaderStages)
Expand All @@ -298,19 +298,19 @@ const kMaxResourcesCases = kUnitCaseParamsBuilder
.combine('extraVisibility', kShaderStages)
.filter(p => (bindingTypeInfo(p.extraEntry).validStages & p.extraVisibility) !== 0);

// Should never fail unless kLimitInfo.maxBindingsPerBindGroup.default is exceeded, because the validation for
// resources-of-type-per-stage is in pipeline layout creation.
// One bind group layout can have a maximum number of each type of binding *per stage* (which is
// different for each type). Test that the max works, then add one more entry of same-or-different
// type and same-or-different visibility.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All test description text should be in the .desc(), let's clean this up and move any useful info into there.

g.test('max_resources_per_stage,in_bind_group_layout')
.desc(
`
Test that the maximum number of bindings of a given type per-stage cannot be exceeded in a
single bind group layout.
- Test each binding type.
- Test that creation of a bind group layout using the maximum number of bindings works.
- Test that creation of a bind group layout using the maximum number of bindings + 1 fails.
- TODO(#230): Update to enforce per-stage and per-pipeline-layout limits on BGLs as well.`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test hasn't actually changed. Am I reading right that the test was already up-to-date despite, just outdated comment and TODO?

- Test that creation of a bind group layout using the maximum number of bindings + 1 fails.`
)
.params(kMaxResourcesCases)
.params(kMaxResourcesPerStageCases)
.fn(t => {
const { maxedEntry, extraEntry, maxedVisibility, extraVisibility } = t.params;
const maxedTypeInfo = bindingTypeInfo(maxedEntry);
Expand Down Expand Up @@ -361,7 +361,7 @@ g.test('max_resources_per_stage,in_pipeline_layout')
- Test that creation of a pipeline using the maximum number of bindings + 1 fails.
`
)
.params(kMaxResourcesCases)
.params(kMaxResourcesPerStageCases)
.fn(t => {
const { maxedEntry, extraEntry, maxedVisibility, extraVisibility } = t.params;
const maxedTypeInfo = bindingTypeInfo(maxedEntry);
Expand Down Expand Up @@ -402,6 +402,118 @@ g.test('max_resources_per_stage,in_pipeline_layout')
}, newBindingCountsTowardSamePerStageLimit);
});

const kMaxResourcesPerPipelineCases = kUnitCaseParamsBuilder
.combine('maxedType', kBufferBindingTypes)
.beginSubcases()
.combine('maxedVisibility', kShaderStages)
.filter(p => (bufferBindingTypeInfo({ type: p.maxedType }).validStages & p.maxedVisibility) !== 0)
.combine('extraType', kBufferBindingTypes)
.combine('extraVisibility', kShaderStages)
.filter(
p => (bufferBindingTypeInfo({ type: p.extraType }).validStages & p.extraVisibility) !== 0
);

// One bind group layout can have a maximum number of each type of dynamic buffer per pipeline. Test
// that the max works, then add one more buffer of the same-or-different type and same-or-different
// visibility.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

g.test('max_dynamic_buffers_per_pipeline,in_bind_group_layout')
.desc(
`
Test that the maximum number of bindings of a given dynamic buffer type per-pipeline cannot be
exceeded in a single bind group layout.
- Test each binding dynamic buffer type.
- Test that creation of a bind group layout using the maximum number of bindings works.
- Test that creation of a bind group layout using the maximum number of bindings + 1 fails.`
)
.params(kMaxResourcesPerPipelineCases)
.fn(t => {
const { maxedType, extraType, maxedVisibility, extraVisibility } = t.params;
const maxedEntry: BGLEntry = { buffer: { type: maxedType, hasDynamicOffset: true } };
const perPipelineLimit = bindingTypeInfo(maxedEntry).perPipelineLimitClass;

const maxResourceBindings: GPUBindGroupLayoutEntry[] = [];
for (let i = 0; i < perPipelineLimit.maxDynamic; i++) {
maxResourceBindings.push({
binding: i,
visibility: maxedVisibility,
...maxedEntry,
});
}

const goodDescriptor = { entries: maxResourceBindings };

// Control
t.device.createBindGroupLayout(goodDescriptor);

// Add an entry that may count towards the same limit. If it does, it should produce a
// validation error.
const extraEntry: BGLEntry = { buffer: { type: extraType, hasDynamicOffset: true } };
const newDescriptor = clone(goodDescriptor);
newDescriptor.entries.push({
binding: perPipelineLimit.maxDynamic,
visibility: extraVisibility,
...extraEntry,
});

const newBindingCountsTowardSamePerPipelineLimit =
perPipelineLimit.class === bindingTypeInfo(extraEntry).perPipelineLimitClass.class;
t.expectValidationError(() => {
t.device.createBindGroupLayout(newDescriptor);
}, newBindingCountsTowardSamePerPipelineLimit);
});

// One pipeline layout can have a maximum number of each type of dynamic buffer per pipeline. Test
// that the max works, then add one more bind group layout with a buffer of the same-or-different
// type and same-or-different visibility.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

g.test('max_dynamic_buffers_per_pipeline,in_pipeline_layout')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is createBindGroupLayout so it's odd to have pipeline layout tests here (I know there are already some). I think we should move all of the limit tests that apply to both createBindGroupLayout and createPipelineLayout to a new file? (and leave behind notes in both createBindGroupLayout and createPipelineLayout's test files)
Can be a followup PR.

.desc(
`
Test that the maximum number of bindings of a given dynamic buffer type per-pipeline cannot be
exceeded across multiple bind group layouts when creating a pipeline layout.
- Test each binding dynamic buffer type.
- Test that creation of a pipeline layout using the maximum number of bindings works.
- Test that creation of a pipeline layout using the maximum number of bindings + 1 fails.`
)
.params(kMaxResourcesPerPipelineCases)
.fn(t => {
const { maxedType, extraType, maxedVisibility, extraVisibility } = t.params;
const maxedEntry: BGLEntry = { buffer: { type: maxedType, hasDynamicOffset: true } };
const perPipelineLimit = bindingTypeInfo(maxedEntry).perPipelineLimitClass;

const maxResourceBindings: GPUBindGroupLayoutEntry[] = [];
for (let i = 0; i < perPipelineLimit.maxDynamic; i++) {
maxResourceBindings.push({
binding: i,
visibility: maxedVisibility,
...maxedEntry,
});
}

const goodLayout = t.device.createBindGroupLayout({ entries: maxResourceBindings });

// Control
t.device.createPipelineLayout({ bindGroupLayouts: [goodLayout] });

// Add an entry bind group layout that has a binding that may count towards the same limit. If
// it does, it should produce a validation error.
const extraEntry: BGLEntry = { buffer: { type: extraType, hasDynamicOffset: true } };
const extraLayout = t.device.createBindGroupLayout({
entries: [
{
binding: 0,
visibility: extraVisibility,
...extraEntry,
},
],
});

const newBindingCountsTowardSamePerPipelineLimit =
perPipelineLimit.class === bindingTypeInfo(extraEntry).perPipelineLimitClass.class;
t.expectValidationError(() => {
t.device.createPipelineLayout({ bindGroupLayouts: [goodLayout, extraLayout] });
}, newBindingCountsTowardSamePerPipelineLimit);
});

g.test('storage_texture,layout_dimension')
.desc(
`
Expand Down