Skip to content

Commit

Permalink
Use max(4, formatSize) for vertex attrib offset alignment (#661)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kangz authored Jul 22, 2021
1 parent 524aa50 commit 3004629
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 38 deletions.
49 changes: 25 additions & 24 deletions src/webgpu/api/operation/vertex_state/correctness.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,19 +646,20 @@ g.test('setVertexBuffer_offset_and_attribute_offset')
.combine('arrayStride', [128])
.expand('offset', p => {
const formatInfo = kVertexFormatInfo[p.format];
const componentSize = formatInfo.bytesPerComponent;
const formatSize = componentSize * formatInfo.componentCount;
return [
const formatSize = formatInfo.bytesPerComponent * formatInfo.componentCount;
return new Set([
0,
componentSize,
componentSize * 2,
componentSize * 3,
4,
8,
formatSize,
formatSize * 2,
p.arrayStride / 2,
p.arrayStride - formatSize - componentSize * 3,
p.arrayStride - formatSize - componentSize * 2,
p.arrayStride - formatSize - componentSize,
p.arrayStride - formatSize - 4,
p.arrayStride - formatSize - 8,
p.arrayStride - formatSize - formatSize,
p.arrayStride - formatSize - formatSize * 2,
p.arrayStride - formatSize,
];
]);
})
)
.fn(t => {
Expand Down Expand Up @@ -693,21 +694,21 @@ g.test('non_zero_array_stride_and_attribute_offset')
.beginSubcases()
.expand('arrayStride', p => {
const formatInfo = kVertexFormatInfo[p.format];
const componentSize = formatInfo.bytesPerComponent;
const formatSize = componentSize * formatInfo.componentCount;
const formatSize = formatInfo.bytesPerComponent * formatInfo.componentCount;

return [align(formatSize, 4), align(formatSize, 4) + 4, kMaxVertexBufferArrayStride];
})
.expand('offset', p => {
const formatInfo = kVertexFormatInfo[p.format];
const componentSize = formatInfo.bytesPerComponent;
const formatSize = componentSize * formatInfo.componentCount;
const formatSize = formatInfo.bytesPerComponent * formatInfo.componentCount;
return new Set(
[
0,
componentSize,
formatSize,
4,
p.arrayStride / 2,
p.arrayStride - formatSize - componentSize,
p.arrayStride - formatSize * 2,
p.arrayStride - formatSize - 4,
p.arrayStride - formatSize,
].map(offset => clamp(offset, { min: 0, max: p.arrayStride - formatSize }))
);
Expand Down Expand Up @@ -982,18 +983,18 @@ g.test('array_stride_zero')
.combine('stepMode', ['vertex', 'instance'] as const)
.expand('offset', p => {
const formatInfo = kVertexFormatInfo[p.format];
const componentSize = formatInfo.bytesPerComponent;
const formatSize = componentSize * formatInfo.componentCount;
const formatSize = formatInfo.bytesPerComponent * formatInfo.componentCount;
return new Set([
0,
componentSize,
componentSize * 2,
componentSize * 3,
4,
8,
formatSize,
formatSize * 2,
kMaxVertexBufferArrayStride / 2,
kMaxVertexBufferArrayStride - formatSize - componentSize * 3,
kMaxVertexBufferArrayStride - formatSize - componentSize * 2,
kMaxVertexBufferArrayStride - formatSize - componentSize,
kMaxVertexBufferArrayStride - formatSize - 4,
kMaxVertexBufferArrayStride - formatSize - 8,
kMaxVertexBufferArrayStride - formatSize,
kMaxVertexBufferArrayStride - formatSize * 2,
]);
})
)
Expand Down
37 changes: 23 additions & 14 deletions src/webgpu/api/validation/vertex_state.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,14 +513,17 @@ g.test('vertex_attribute_offset_alignment')
.expand('offset', p => {
const { bytesPerComponent, componentCount } = kVertexFormatInfo[p.format];
const formatSize = bytesPerComponent * componentCount;
const halfAlignment = Math.floor(bytesPerComponent / 2);

return new Set([
0,
halfAlignment,
bytesPerComponent,
Math.floor(formatSize / 2),
formatSize,
2,
4,
p.arrayStride - formatSize,
p.arrayStride - formatSize - halfAlignment,
p.arrayStride - formatSize - Math.floor(formatSize / 2),
p.arrayStride - formatSize - 4,
p.arrayStride - formatSize - 2,
]);
})
.beginSubcases()
Expand Down Expand Up @@ -549,7 +552,10 @@ g.test('vertex_attribute_offset_alignment')
const vertexBuffers = [];
vertexBuffers[vertexBufferIndex] = { arrayStride, attributes };

const success = offset % kVertexFormatInfo[format].bytesPerComponent === 0;
const formatInfo = kVertexFormatInfo[format];
const formatSize = formatInfo.bytesPerComponent * formatInfo.componentCount;
const success = offset % Math.min(4, formatSize) === 0;

t.testVertexState(success, vertexBuffers);
});

Expand Down Expand Up @@ -577,17 +583,20 @@ g.test('vertex_attribute_contained_in_stride')
const { bytesPerComponent, componentCount } = kVertexFormatInfo[p.format];
const formatSize = bytesPerComponent * componentCount;
yield 0;
yield bytesPerComponent;
yield 4;

// arrayStride = 0 is a special case because for the offset validation it acts the same
// as arrayStride = kMaxVertexBufferArrayStride. We branch so as to avoid adding negative
// offsets that would cause an IDL exception to be thrown instead of a validation error.
if (p.arrayStride === 0) {
yield kMaxVertexBufferArrayStride - formatSize;
yield kMaxVertexBufferArrayStride - formatSize + bytesPerComponent;
} else {
yield p.arrayStride - formatSize;
yield p.arrayStride - formatSize + bytesPerComponent;
// as arrayStride = kMaxVertexBufferArrayStride. We special case here so as to avoid adding
// negative offsets that would cause an IDL exception to be thrown instead of a validation
// error.
const stride = p.arrayStride !== 0 ? p.arrayStride : kMaxVertexBufferArrayStride;
yield stride - formatSize;
yield stride - formatSize + 4;

// Avoid adding duplicate cases when formatSize == 4 (it is already tested above)
if (formatSize !== 4) {
yield formatSize;
yield stride;
}
})
.combine('vertexBufferIndex', [0, 1, kMaxVertexBuffers - 1])
Expand Down

0 comments on commit 3004629

Please sign in to comment.