deal with bindGroup layouts that don't start at zero and are contiguous

greggman · Jun 17, 2024 · d72101f · d72101f
1 parent 458d4b6
commit d72101f
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 23 deletions.
diff --git a/src/binding-mixin.ts b/src/binding-mixin.ts
@@ -147,7 +147,7 @@ function boundBufferRanges(info: BindGroupInfo, dynamicOffsets: Uint32Array) {
   const result = new Map<string, BoundBufferRange>();
   let dynamicOffsetIndex = 0;
   for (const bindGroupEntry of info.entries) {
-    const bindGroupLayoutEntry = info.layoutPlus.bindGroupLayoutDescriptor.entries[bindGroupEntry.binding];
+    const bindGroupLayoutEntry = info.layoutPlus.entriesById[bindGroupEntry.binding];
     if (!bindGroupLayoutEntry.buffer) {
       continue;
     }
@@ -274,9 +274,8 @@ export function encoderBindGroupsAliasAWritableResource(
 
 function* forEachDynamicBinding(info: BindGroupInfo) {
   let dynamicOffsetIndex = 0;
-  const layout = info.layoutPlus.bindGroupLayoutDescriptor;
   for (const entry of info.entries) {
-    const bindingDescriptor = layout.entries[entry.binding];
+    const bindingDescriptor = info.layoutPlus.entriesById[entry.binding];
     if (bindingDescriptor.buffer?.hasDynamicOffset) {
       const bufferBinding = entry.resource as GPUBufferBinding;
       const bufferLayout = bindingDescriptor.buffer;

diff --git a/src/device.ts b/src/device.ts
@@ -101,12 +101,14 @@ function  bindGroupLayoutDescriptorToBindGroupLayoutDescriptorPlus(
     src: GPUBindGroupLayoutDescriptor,
     autoId: number): BindGroupLayoutDescriptorPlus {
   const bindGroupLayoutDescriptor = {
-    entries: [...src.entries].map(reifyBindGroupLayoutEntry).sort((a, b) => a.binding - b.binding),
+    entries: [...src.entries].map(reifyBindGroupLayoutEntry),
   };
+  const entriesById = Object.fromEntries(bindGroupLayoutDescriptor.entries.map(e => [e.binding, e]));
   const dynamicOffsetCount = bindGroupLayoutDescriptor.entries.reduce((a, v) => a + (v.buffer?.hasDynamicOffset ? 1 : 0), 0);
   const signature = `${JSON.stringify(bindGroupLayoutDescriptor)}${autoId ? `:autoId(${autoId})` : ''})`;
   return {
     bindGroupLayoutDescriptor,
+    entriesById,
     dynamicOffsetCount,
     signature,
   };

diff --git a/src/pipeline.ts b/src/pipeline.ts
@@ -6,11 +6,13 @@ import { arraysEqual, trimNulls } from './utils.js';
 import { wrapFunctionAfter } from "./wrap-api.js";
 
 type BindGroupLayoutDescriptor = {
+  /** this is sparse! */
   entries: GPUBindGroupLayoutEntry[];
 };
 
 export type BindGroupLayoutDescriptorPlus = {
   bindGroupLayoutDescriptor: BindGroupLayoutDescriptor,
+  entriesById: Record<number, GPUBindGroupLayoutEntry>;
   dynamicOffsetCount: number,
   signature: string,
 }

diff --git a/test/tests/binding-mixin-tests.js b/test/tests/binding-mixin-tests.js
@@ -181,8 +181,8 @@ async function createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, d
   device = device || await (await navigator.gpu.requestAdapter()).requestDevice();
   const { pass, pipeline } = await makePassAndPipeline(device, {
     resourceWGSL: `
-      @group(0) @binding(0) var<uniform> u00: f32;
-      @group(0) @binding(1) var<uniform> u01: f32;
+      @group(0) @binding(1) var<uniform> u00: f32;
+      @group(0) @binding(2) var<uniform> u01: f32;
       @group(1) @binding(0) var<uniform> u10: vec4f;
       @group(2) @binding(0) var<uniform> u20: vec4f;
     `,
@@ -200,8 +200,8 @@ async function createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, d
   const bindGroup0 = device.createBindGroup({
     layout: pipeline.getBindGroupLayout(0),
     entries: [
-      { binding: 0, resource: { buffer: u00Buffer }},
-      { binding: 1, resource: { buffer: u01Buffer }},
+      { binding: 1, resource: { buffer: u00Buffer }},
+      { binding: 2, resource: { buffer: u01Buffer }},
     ],
   });
   const bindGroup1 = device.createBindGroup({
@@ -240,8 +240,8 @@ async function createResourcesForExplicitLayoutBindGroupTests({
   const bindGroupLayouts = [
     {
       entries: [
-        { binding: 0, visibility, buffer: {} },
         { binding: 1, visibility, buffer: {} },
+        { binding: 2, visibility, buffer: {} },
       ],
     },
     {
@@ -262,8 +262,8 @@ async function createResourcesForExplicitLayoutBindGroupTests({
   const { pass, pipeline } = await makePassAndPipeline(device, {
     layout,
     resourceWGSL: `
-      @group(0) @binding(0) var<uniform> u00: f32;
-      @group(0) @binding(1) var<uniform> u01: f32;
+      @group(0) @binding(1) var<uniform> u00: f32;
+      @group(0) @binding(2) var<uniform> u01: f32;
       @group(1) @binding(0) var<uniform> u10: vec4f;
       @group(2) @binding(0) var<uniform> u20: vec4f;
     `,
@@ -282,8 +282,8 @@ async function createResourcesForExplicitLayoutBindGroupTests({
   const bindGroup0 = device.createBindGroup({
     layout: bindGroupLayouts[0],
     entries: [
-      { binding: 0, resource: { buffer: u00Buffer }},
-      { binding: 1, resource: { buffer: u01Buffer }},
+      { binding: 1, resource: { buffer: u00Buffer }},
+      { binding: 2, resource: { buffer: u01Buffer }},
     ],
   });
   const bindGroup1 = device.createBindGroup({
@@ -322,7 +322,7 @@ export function addValidateBindGroupTests({
 
     describe('auto layout', () => {
 
-       itWithDevice('works with auto layout', async (device) => {
+      itWithDevice('works with auto layout', async (device) => {
         const { pass, bindGroup0, bindGroup1, bindGroup2 } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         pass.setBindGroup(0, bindGroup0);
         pass.setBindGroup(1, bindGroup1);
@@ -333,7 +333,7 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('fails if missing bindGroup', async (device) => {
+      itWithDevice('fails if missing bindGroup', async (device) => {
         const { pass, bindGroup1, bindGroup2 } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         pass.setBindGroup(1, bindGroup1);
         pass.setBindGroup(2, bindGroup2);
@@ -343,7 +343,7 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('fails if resource is destroyed', async (device) => {
+      itWithDevice('fails if resource is destroyed', async (device) => {
         const { pass, u01Buffer, bindGroup0, bindGroup1, bindGroup2 } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         pass.setBindGroup(0, bindGroup0);
         pass.setBindGroup(1, bindGroup1);
@@ -355,7 +355,7 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('fails if layout is incompatible (auto layout)', async (device) => {
+      itWithDevice('fails if layout is incompatible (auto layout)', async (device) => {
         const { pass, bindGroup0, bindGroup2 } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         const { bindGroup1 } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         pass.setBindGroup(0, bindGroup0);
@@ -367,7 +367,7 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('works if layout is compatible (auto layout)', async (device) => {
+      itWithDevice('works if layout is compatible (auto layout)', async (device) => {
         const { pass, bindGroup0, bindGroup1, bindGroup2 } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         pass.setBindGroup(0, bindGroup0);
         pass.setBindGroup(1, bindGroup2);  // 2 and 1 are swapped but
@@ -378,7 +378,7 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('false if layout is incompatible (auto layout + manual bindGroupLayout)', async (device) => {
+      itWithDevice('false if layout is incompatible (auto layout + manual bindGroupLayout)', async (device) => {
         const { pass, bindGroup0, bindGroup1, u20Buffer } = await createResourcesForAutoLayoutBindGroupTests(makePassAndPipeline, device);
         const bindGroupLayout = device.createBindGroupLayout({
           entries: [
@@ -408,7 +408,7 @@ export function addValidateBindGroupTests({
 
     describe('explicit layout', () => {
 
-       itWithDevice('works with explicit layout', async (device) => {
+      itWithDevice('works with explicit layout', async (device) => {
         const { pass, bindGroup0, bindGroup1, bindGroup2 } = await createResourcesForExplicitLayoutBindGroupTests({ makePassAndPipeline, visibility, device });
         pass.setBindGroup(0, bindGroup0);
         pass.setBindGroup(1, bindGroup1);
@@ -419,7 +419,32 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('works with different explicit layout if they are compatible', async (device) => {
+      itWithDevice('fails with incompatible bind group (bindGroup has out of range bindings)', async (device) => {
+        const { pass, bindGroup1, bindGroup2, u00Buffer } = await createResourcesForExplicitLayoutBindGroupTests({ makePassAndPipeline, visibility, device });
+
+        const bindGroupLayout = device.createBindGroupLayout({
+          entries: [
+            { binding: 3, visibility, buffer: {} },
+          ],
+        });
+
+        const bindGroup0 = device.createBindGroup({
+          layout: bindGroupLayout,
+          entries: [
+            { binding: 3, resource: { buffer: u00Buffer }},
+          ],
+        });
+
+        pass.setBindGroup(0, bindGroup0);
+        pass.setBindGroup(1, bindGroup1);
+        pass.setBindGroup(2, bindGroup2);
+
+        await expectValidationError(true, async () => {
+          await execute(pass);
+        });
+      });
+
+      itWithDevice('works with different explicit layout if they are compatible', async (device) => {
         const { pass, bindGroup0, bindGroup1 } = await createResourcesForExplicitLayoutBindGroupTests({ makePassAndPipeline, visibility, device });
         const { bindGroup2 } = await createResourcesForExplicitLayoutBindGroupTests({ makePassAndPipeline, device, visibility });
         pass.setBindGroup(0, bindGroup0);
@@ -431,7 +456,7 @@ export function addValidateBindGroupTests({
         });
       });
 
-       itWithDevice('fails with incompatible bindGroup', async (device) => {
+      itWithDevice('fails with incompatible bindGroup', async (device) => {
         const { pass, bindGroup1, bindGroup2 } = await createResourcesForExplicitLayoutBindGroupTests({ makePassAndPipeline, visibility, device });
         pass.setBindGroup(0, bindGroup1); // incompatible
         pass.setBindGroup(1, bindGroup1);

diff --git a/test/tests/command-encoder/copyBufferToBuffer-tests.js b/test/tests/command-encoder/copyBufferToBuffer-tests.js
@@ -93,7 +93,7 @@ export default function () {
     ];
     for (const {srcOffset = 0, dstOffset = 0, size = 16, desc} of tests) {
       itWithDevice(desc, async (device) => {
-          const encoder = await createCommandEncoder(device);
+        const encoder = await createCommandEncoder(device);
         const src = device.createBuffer({size: 16, usage: GPUBufferUsage.COPY_SRC});
         const dst = device.createBuffer({size: 32, usage: GPUBufferUsage.COPY_DST});
         await expectValidationError(true, async () => {