Skip to content

fix: parse dawnToggles properly#379

Merged
wcandillon merged 4 commits into
wcandillon:mainfrom
pepperoni505:feat/dawn-device-toggles
Jun 3, 2026
Merged

fix: parse dawnToggles properly#379
wcandillon merged 4 commits into
wcandillon:mainfrom
pepperoni505:feat/dawn-device-toggles

Conversation

@pepperoni505
Copy link
Copy Markdown
Contributor

This is a follow-up to #374. I seem to have forgotten to push this commit, which is required to actually parse the property. Sorry about that!

@wcandillon wcandillon self-requested a review June 2, 2026 18:20
Copy link
Copy Markdown
Owner

@wcandillon wcandillon left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. Now we didn't catch this ealier because we didn't try any valid toggles. Could you add the following test if sensible:

diff --git a/packages/webgpu/src/__tests__/DawnToggles.spec.ts b/packages/webgpu/src/__tests__/DawnToggles.spec.ts
index 574a985..96900ac 100644
--- a/packages/webgpu/src/__tests__/DawnToggles.spec.ts
+++ b/packages/webgpu/src/__tests__/DawnToggles.spec.ts
@@ -38,4 +38,59 @@ describe("Dawn toggles", () => {
     });
     expect(result).toBe(true);
   });
+
+  // The tests above only assert that requestDevice resolves, which is true
+  // whether or not the toggle is parsed and chained onto the device descriptor.
+  // This test instead activates a real toggle (skip_validation) and observes a
+  // behavioral difference, so it fails if dawnToggles ever stops being applied.
+  it("applies skip_validation so a normally-invalid buffer creates without error", async () => {
+    // dawnToggles is a non-standard Dawn extension; browsers ignore it, so the
+    // behavioral difference only exists on the native (Dawn) backends.
+    if (client.OS === "web") {
+      return;
+    }
+    const result = await client.eval(({ gpu }) => {
+      // MAP_READ may only be combined with COPY_DST and MAP_WRITE only with
+      // COPY_SRC, so MAP_READ | MAP_WRITE is a validation error by default. The
+      // buffer is never used on the GPU, so creating it with validation skipped
+      // is harmless on every backend.
+      const invalid = {
+        size: 16,
+        usage: GPUBufferUsage.MAP_READ | GPUBufferUsage.MAP_WRITE,
+      };
+      // A fresh adapter per device: an adapter only vends a single device.
+      return gpu
+        .requestAdapter()
+        .then((adapter) => adapter!.requestDevice())
+        .then((control) => {
+          control.pushErrorScope("validation");
+          control.createBuffer(invalid);
+          return control.popErrorScope();
+        })
+        .then((controlError) =>
+          gpu
+            .requestAdapter()
+            .then((adapter) =>
+              adapter!.requestDevice({
+                dawnToggles: { enabledToggles: ["skip_validation"] },
+              }),
+            )
+            .then((toggled) => {
+              toggled.pushErrorScope("validation");
+              toggled.createBuffer(invalid);
+              return toggled.popErrorScope();
+            })
+            .then((toggledError) => ({
+              controlHadError: controlError !== null,
+              toggledHadError: toggledError !== null,
+            })),
+        );
+    });
+    // The operation is genuinely invalid on this build (guards against the test
+    // silently passing because the buffer became valid).
+    expect(result.controlHadError).toBe(true);
+    // skip_validation took effect, which is only possible if dawnToggles was
+    // parsed and chained onto the device descriptor.
+    expect(result.toggledHadError).toBe(false);
+  });
 });

@pepperoni505
Copy link
Copy Markdown
Contributor Author

Done, thank you!

@wcandillon wcandillon self-requested a review June 3, 2026 05:29
@wcandillon wcandillon merged commit 97d593c into wcandillon:main Jun 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants