From 30046290cba86c70c2bd098bb19998154e4364e7 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 22 Jul 2021 21:59:40 +0200 Subject: [PATCH] Use max(4, formatSize) for vertex attrib offset alignment (#661) --- .../vertex_state/correctness.spec.ts | 49 ++++++++++--------- .../api/validation/vertex_state.spec.ts | 37 ++++++++------ 2 files changed, 48 insertions(+), 38 deletions(-) diff --git a/src/webgpu/api/operation/vertex_state/correctness.spec.ts b/src/webgpu/api/operation/vertex_state/correctness.spec.ts index 0158fa35ae69..db2b03cf57f2 100644 --- a/src/webgpu/api/operation/vertex_state/correctness.spec.ts +++ b/src/webgpu/api/operation/vertex_state/correctness.spec.ts @@ -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 => { @@ -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 })) ); @@ -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, ]); }) ) diff --git a/src/webgpu/api/validation/vertex_state.spec.ts b/src/webgpu/api/validation/vertex_state.spec.ts index d612f369e004..e10b3c452cea 100644 --- a/src/webgpu/api/validation/vertex_state.spec.ts +++ b/src/webgpu/api/validation/vertex_state.spec.ts @@ -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() @@ -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); }); @@ -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])