Skip to content

Commit

Permalink
Compat: Limit draw test attributes in compat (gpuweb#2954)
Browse files Browse the repository at this point in the history
In compat mode, @Builtin(vertex_index) and @Builtin(instance_index)
each take an attribute so adjust this test to account for that.

I don't know if this test should be refactored to make sure
16 attributes are tested (So without using the builtins).
My feeling is this is probably fine?

The test in
webgpu/compat/api/validation/encoding/render_pipeline/vertex_state.spec.ts
does a simple test that you can create a pipeline with maxVertexAttributes
but it does not actually draw with that pipeline.
  • Loading branch information
greggman authored Sep 14, 2023
1 parent 617369d commit b4dfdd2
Showing 1 changed file with 27 additions and 6 deletions.
33 changes: 27 additions & 6 deletions src/webgpu/api/operation/rendering/draw.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,10 @@ g.test('vertex_attributes,basic')
const vertexCount = 4;
const instanceCount = 4;

const attributesPerVertexBuffer =
t.params.vertex_attribute_count / t.params.vertex_buffer_count;
assert(Math.round(attributesPerVertexBuffer) === attributesPerVertexBuffer);
// In compat mode, @builtin(vertex_index) and @builtin(instance_index) each take an attribute.
const maxAttributes = t.device.limits.maxVertexAttributes - (t.isCompatibility ? 2 : 0);
const numAttributes = Math.min(maxAttributes, t.params.vertex_attribute_count);
const maxAttributesPerVertexBuffer = Math.ceil(numAttributes / t.params.vertex_buffer_count);

let shaderLocation = 0;
let attributeValue = 0;
Expand Down Expand Up @@ -477,7 +478,12 @@ g.test('vertex_attributes,basic')
}

const attributes: GPUVertexAttribute[] = [];
for (let a = 0; a < attributesPerVertexBuffer; ++a) {
const numAttributesForBuffer = Math.min(
maxAttributesPerVertexBuffer,
maxAttributes - b * maxAttributesPerVertexBuffer
);

for (let a = 0; a < numAttributesForBuffer; ++a) {
const attribute: GPUVertexAttribute = {
format: t.params.vertex_format,
shaderLocation,
Expand All @@ -490,7 +496,7 @@ g.test('vertex_attributes,basic')
}

for (let v = 0; v < vertexOrInstanceCount; ++v) {
for (let a = 0; a < attributesPerVertexBuffer; ++a) {
for (let a = 0; a < numAttributesForBuffer; ++a) {
vertexBufferValues.push(attributeValue);
attributeValue += 1.234; // Values will get rounded later if we make a Uint32Array.
}
Expand Down Expand Up @@ -570,7 +576,7 @@ g.test('vertex_attributes,basic')
let accumulateVariableDeclarationsInFragmentShader = '';
let accumulateVariableAssignmentsInFragmentShader = '';
// The remaining 3 vertex attributes
if (t.params.vertex_attribute_count === 16) {
if (numAttributes === 16) {
accumulateVariableDeclarationsInVertexShader = `
@location(13) @interpolate(flat) outAttrib13 : vec4<${wgslFormat}>,
`;
Expand All @@ -587,6 +593,21 @@ g.test('vertex_attributes,basic')
outBuffer.primitives[input.primitiveId].attrib14 = input.attrib13.z;
outBuffer.primitives[input.primitiveId].attrib15 = input.attrib13.w;
`;
} else if (numAttributes === 14) {
accumulateVariableDeclarationsInVertexShader = `
@location(13) @interpolate(flat) outAttrib13 : vec4<${wgslFormat}>,
`;
accumulateVariableAssignmentsInVertexShader = `
output.outAttrib13 =
vec4<${wgslFormat}>(input.attrib12, input.attrib13, 0, 0);
`;
accumulateVariableDeclarationsInFragmentShader = `
@location(13) @interpolate(flat) attrib13 : vec4<${wgslFormat}>,
`;
accumulateVariableAssignmentsInFragmentShader = `
outBuffer.primitives[input.primitiveId].attrib12 = input.attrib13.x;
outBuffer.primitives[input.primitiveId].attrib13 = input.attrib13.y;
`;
}

const pipeline = t.device.createRenderPipeline({
Expand Down

0 comments on commit b4dfdd2

Please sign in to comment.