Sitelet https://github.com/gpuweb/gpuweb/pull/521
Skip to content

requestDevice returns null if adapter is lost - #521

Merged
kainino0x merged 2 commits into
gpuweb:masterfrom
kainino0x:requestDevice-null
Feb 4, 2020
Merged

kainino0x merged 2 commits into
gpuweb:masterfrom
kainino0x:requestDevice-null

Conversation

@kainino0x

Copy link
Copy Markdown
Contributor

This means all rejections from requestAdapter/requestDevice are "give up on webgpu" scenarios, simplifying the logic.

Comment thread design/ErrorHandling.md Outdated
Comment thread design/ErrorHandling.md
} catch (e) {
console.error("device request failed", e);
// That failed; try a new adapter entirely.
await tryEnsureDeviceOnCurrentAdapter();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: this call to tryEnsureDeviceOnCurrentAdapter() can be folded inside the loop.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The first one tries to reuse the current adapter while the later ones get a new adapter, though maybe this is wrong. I think this code still needs some revision.

@kainino0x

Copy link
Copy Markdown
Contributor Author

TODO: Investigate making requestAdapter return null if no adapters are available.

@Kangz

Kangz commented Jan 6, 2020

Copy link
Copy Markdown
Contributor

This was discussed in the 2020-01-06 meeting

Comment thread design/ErrorHandling.md
An app should never give up on getting WebGPU access due to
`requestDevice` returning null or `GPUDevice.lost` resolving.
It should only give up based on a `requestAdapter` rejection.
(It should also give up on a `requestDevice` rejection, as that indicates an app programming error.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It feels to me that "should give up" and "shouldn't give up" distinction is too high level. We could maybe specify it in terms of "this is a function of Xxx", which implies to the user whether they should try again or not. I.e. we can say that the rejection is deterministic based on the adapter capabilities and the provided options. This is better than "should give up" because it also allows for a case where the user probes for different limits, so they would actually want to try again with different options.
At the same time, we'd say that returning null is an indication of a temporary state that doesn't depend on the parameters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(this is just the design doc, so we can make it more formal in spec)

requestDevice rejects if you ask for higher limits than its adapter has. Apps shouldn't be doing that (they should check the adapter limits instead) making this rejection an app programming error.

requestAdapter rejects if no adapters could possibly be returned. Given the only adapter request option right now is powerPreference, the options should never have an impact on whether or not you get an adapter. Perhaps in the future it would though.

@kainino0x

Copy link
Copy Markdown
Contributor Author

Discussed today. Resolution: merge!

@kainino0x
kainino0x merged commit c4b7c2d into gpuweb:master Feb 4, 2020
@kainino0x
kainino0x deleted the requestDevice-null branch February 4, 2020 00:01
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
If a texture format support multisample, it should be renderable:
color-renderable, depth-renderable, or stencil-renderable.

See gpuweb issue at gpuweb#1522.

This change will fix Dawn issue 731:
https://bugs.chromium.org/p/dawn/issues/detail?id=731.
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.

4 participants