1826681 - Warn in console when `GPUDevice.createShaderModule` fails

Status: RESOLVED FIXED

(Core :: Graphics: WebGPU, enhancement)

Description

[Kelsey Gilbert [:jgilbert]]

This is optional, but is really nice and we should have it.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Erich expressed interest in implementing this, so I'll just assign it so no one else picks it up in the meantime.

Comment 2

[Erich Gubler [:ErichDonGubler] (he/him)]

This sort of functionality is called for in the current draft of section 9.1.1 of the WebGPU v1 spec. ("Shader Module Creation"), more specifically the createrShaderModule(descriptor) item's note on "Device timeline initialization steps":

Note: User agents should not include detailed compiler error messages or shader text in the message text of validation errors arising here: these details are accessible via getCompilationInfo(). User agents should surface human-readable, formatted error details to developers for easier debugging (for example as a warning in the browser developer console, expandable to show full shader source).

As shader compilation errors should be rare in production applications, user agents could choose to surface them to developers regardless of error handling (GPU error scopes or uncapturederror event handlers), e.g. as an expandable warning. If not, they should provide and document another way for developers to access human-readable error details, for example by adding a checkbox to show errors unconditionally, or by showing human-readable details when logging a GPUCompilationInfo object to the console.

Comment 3

[Erich Gubler [:ErichDonGubler] (he/him)]

:jgilbert also mentioned that we have some precedent for this behavior in WebGL at WebGLProgram.cpp:924-937.

Comment 4

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: desired-behavior-1.html

Attaching an HTML file with an embedded JS script that creates and reports to the dev. console from GPUShaderModule.compilationInfo. This basically implements the easy-to-implement behavior I think we should have for this ticket, subject to feedback from :jimb.

Comment 5

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: feat(webgpu): log shader creation msgs. to console r=#webgpu-reviewers

Comment 6

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: desired-behavior-2-hoist-first-err.html

This attachment describes behavior close to what :jimb has requested in our conversation in internal Matrix chat for WebGPU. The only thing it doesn't do is print only the first line of the first error or warning in the "sentinel" message printed before the collapsed console message group.

Comment 7

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: desired-behavior-3-aggregated-callout.html

Comment 8

[Kelsey Gilbert [:jgilbert]]

(In reply to Erich Gubler [:ErichDonGubler] from comment #2)

This sort of functionality is called for in the current draft of section 9.1.1 of the WebGPU v1 spec. ("Shader Module Creation"), more specifically the createrShaderModule(descriptor) item's note on "Device timeline initialization steps":

Not that this is a bad idea per se, but we aren't compelled by it. This was written by companies other than us and merged without collaboration. We should (on our own) pick what we think is best.

Comment 9

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: feat(webgpu): print counts of shader compilation messages by type r=#webgpu-reviewers

Refine the console group title for shader compilation messages by adding aggregated counts of each message by type.

Depends on D175303

Comment 10

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: refactor: factor out `console->{Error,Warn}` in `DeviceCreateShaderModule` r=#webgpu-reviewers

This results in lukewarm gains at best for the code that currently exists, but subsequent patches in this stack will look much better for it. So do it now.

Depends on D175402

Comment 11

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: feat(webgpu): add sentinel shader compilation msg. in console r=#webgpu-reviewers

@jimb expressed concern that a collapsed group of compilation messages for WebGPU shaders in the JS console isn't easy enough to discern from scanning a console log manually. We can't associate a log level with the group itself (which would let us paint the group with color for contrast), unfortunately. So, we decided to have a "sentinel" message right before groups that uses the highest "severity" of message in the entire set of compilation messages.

Depends on D175403

Comment 12

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: fix(webgpu): propagate `label` to new shader modules r=#webgpu-reviewers

We don't set the GPUObjectBase.label of the shaderModule we create in our implementation of GPUDevice.createShaderModule, but we should. Fix it! You can see this reflected in the error diagnostics currently returned by Naga in the GPUCompilationMessage.message field.

Depends on D175404

Comment 13

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: fix(webgpu): present `label` in shader comp. msgs. in console r=#webgpu-reviewers

This context is crucial for debugging, kthxbai.

Depends on D175405

Comment 14

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: desired-behavior-4-aggregated-callout-with-label.html

Adding a new test file that (1) demonstrates the behavior that was prototyped, as before, and (2) exercises cases where the GPUObjectDescriptorBase.label for GPUShaderModuleDescriptor is omitted and specified.

Comment 15

[Erich Gubler [:ErichDonGubler] (he/him)]

Attachment: Bug 1826681: refactor: factor out new `reportCompilationMessagesToConsole` func. r=#webgpu-reviewers,nical

Depends on D175406

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2a6476fe2d4
feat(webgpu): log shader creation msgs. to console r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/f99d74976f29
feat(webgpu): print counts of shader compilation messages by type r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/b71415253dd2
feat(webgpu): add sentinel shader compilation msg. in console r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/47a2a358111e
fix(webgpu): propagate `label` to new shader modules r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/17c9673361fc
fix(webgpu): present `label` in shader comp. msgs. in console r=webgpu-reviewers,nical

Comment 17

[Cristian Tuns]

Backed out for causing build bustages in WebGPUChild.cpp/Device.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/dom/webgpu/ipc/WebGPUChild.cpp:817:10: error: call to member function 'AppendInt' is ambiguous
  /builds/worker/checkouts/gecko/dom/webgpu/Device.cpp:195:10: error: arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'mBridge->' is neither.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4ddc5f8bf51
feat(webgpu): log shader creation msgs. to console r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/627cbf1749d5
feat(webgpu): print counts of shader compilation messages by type r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/3db3f3df4f44
feat(webgpu): add sentinel shader compilation msg. in console r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/8aca8c275beb
fix(webgpu): propagate `label` to new shader modules r=webgpu-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/5e7c8062bed4
fix(webgpu): present `label` in shader comp. msgs. in console r=webgpu-reviewers,nical

Comment 19

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b4ddc5f8bf51
https://hg.mozilla.org/mozilla-central/rev/627cbf1749d5
https://hg.mozilla.org/mozilla-central/rev/3db3f3df4f44
https://hg.mozilla.org/mozilla-central/rev/8aca8c275beb
https://hg.mozilla.org/mozilla-central/rev/5e7c8062bed4