Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show a dialog if WebGPU isn't available or there's an error #423

Merged
merged 2 commits into from
Jul 20, 2024

Conversation

kainino0x
Copy link
Collaborator

This won't catch JS exceptions in the middle of the actual sample, but it at least shows a nice message when WebGPU is not available for some reason, or there's a validation error (usually because the browser doesn't support the sample).

I would like to catch JS exceptions but it would be a bit more complicated. Thinking of just registering an onunhandledrejection handler and wrapping each sample in a (async () => { ... })() but that's a bit silly.

Anyway what do you think of this - is the DOM stuff too heavy handed for a sample util?

@greggman
Copy link
Collaborator

greggman commented Jun 15, 2024

I'm not against this but my gut says this seems further from showing WebGPU?

What about something like

const adapter = await navigator.gpu?.requestAdapter();
const device = await adapter?.requestDevice();
showMessageIfWebGPUFailedOrIsNotAvailable(adapter, device);  // from util.ts

Then the code shows more straight forward WebGPU in the example. You can still check if navigator.gpu is null, and/or if adapter was null, and/or if device was null or lost in showWebGPUIssues

That won't handle the case when asking for limits, features though.

@kainino0x
Copy link
Collaborator Author

100% agree, I had some similar thoughts in the evening after I posted this. I think I can come up with something but it will have to wait a little bit.

@kainino0x kainino0x marked this pull request as draft June 18, 2024 01:11
@kainino0x kainino0x force-pushed the error-dialog branch 2 times, most recently from 8c9df57 to 211370f Compare July 20, 2024 02:16
This won't catch JS exceptions in the middle of the actual sample, but
it at least shows a nice message when WebGPU is not available for some
reason, or there's an error (the browser doesn't support the sample).
@kainino0x kainino0x marked this pull request as ready for review July 20, 2024 02:20
@kainino0x
Copy link
Collaborator Author

Finally revised this - PTAL

Copy link
Collaborator

@greggman greggman left a comment

Choose a reason for hiding this comment

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

LGTM!

just random comments

  • does it need a button?

    The user clicks 'OK' for what purpose? I'm not saying to remove the button. Maybe the fact that it has one makes them more likely to read the message.

  • not getting an adapter doesn't technically mean WebGPU is not available on this system. Compat for example, and in the future fl2. I can't think of a simple message though it's probably best as is.

@kainino0x
Copy link
Collaborator Author

kainino0x commented Jul 20, 2024

  • does it need a button?
    The user clicks 'OK' for what purpose? I'm not saying to remove the button. Maybe the fact that it has one makes them more likely to read the message.

Not super necessary, but sometimes there will be an error yet the sample keeps working - for example the reversedZ sample does this in Firefox right now.

Also if some error comes up in an interactive demo like samplerParameters, it could be useful to dismiss it and change the settings back to the mode that didn't have an error.

  • not getting an adapter doesn't technically mean WebGPU is not available on this system. Compat for example, and in the future fl2. I can't think of a simple message though it's probably best as is.

Good point, I'll tweak it a little.

@kainino0x kainino0x enabled auto-merge (squash) July 20, 2024 03:46
@kainino0x kainino0x merged commit fa7b022 into webgpu:main Jul 20, 2024
1 check passed
@kainino0x kainino0x deleted the error-dialog branch July 20, 2024 03:48
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