Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

Panic when no adapters are available #117

Closed
parasyte opened this issue Nov 7, 2019 · 3 comments
Closed

Panic when no adapters are available #117

parasyte opened this issue Nov 7, 2019 · 3 comments

Comments

@parasyte
Copy link
Contributor

parasyte commented Nov 7, 2019

Similar to #105
Originally reported in parasyte/pixels#36

We try to catch any errors from wgpu::Adapter::request to provide a "helpful" error message, but our error handler never gets called: https://github.com/parasyte/pixels/blob/98983d201db0639f51c264ade5cf876f8df65d1e/src/lib.rs#L412-L413

The backtrace points to the panic occurring here in wgpu-native: https://github.com/gfx-rs/wgpu/blob/951641dcc50b355eda894c9a5cd6da057d48765b/wgpu-native/src/instance.rs#L473-L475

The wgpu::Adapter::request method itself is suspicious. The docs claim it returns None when an adapter cannot be found, but it is clearly hardcoded to return Some<T>:

wgpu-rs/src/lib.rs

Lines 538 to 547 in cbdd74f

/// Retrieves an [`Adapter`] which matches the given options.
///
/// Some options are "soft", so treated as non-mandatory. Others are "hard".
///
/// If no adapters are found that suffice all the "hard" options, `None` is returned.
pub fn request(options: &wgn::RequestAdapterOptions) -> Option<Self> {
Some(Adapter {
id: wgn::wgpu_request_adapter(Some(options)),
})
}

Given these two issues, we're unable to capture the error (transform the None into Result<_, E>) because this method never returns None. I would work on this, but unfortunately I don't know the best way to approach the extern "C" interface. Should it just return Option<AdapterId>?

@fogti
Copy link

fogti commented Nov 7, 2019

Actually, the unwrap lies in rust code here:

https://github.com/gfx-rs/wgpu/blob/c33b1e81fddf480f24e543cd3689b8991029d893/wgpu-native/src/instance.rs#L473-L475

It would be a good idea to call request_adapter directly.

@grovesNL
Copy link
Collaborator

grovesNL commented Nov 7, 2019

It would be a good idea to call request_adapter directly.

In general we should avoid skipping the C API for functional changes like this, since we also want to allow other languages using the C API to fail gracefully too.

I would work on this, but unfortunately I don't know the best way to approach the extern "C" interface. Should it just return Option<AdapterId>?

We can't return Option<AdapterId> in the C API because we don't have Option available in C, but we need to be able to use the return value there.

We haven't implemented full WebGPU error handling yet, so for now the C API could use the special Id::ERROR for None and check for this in wgpu-rs. This is similar to the following check which we already have for swapchain errors:

if output.view_id == wgn::Id::ERROR {

@parasyte
Copy link
Contributor Author

parasyte commented Nov 7, 2019

Duplicate of gfx-rs/wgpu#368

@parasyte parasyte closed this as completed Nov 7, 2019
bors bot added a commit to gfx-rs/wgpu that referenced this issue Nov 8, 2019
375: Use callback for `request_adapter` r=kvark a=grovesNL

For now this is mostly a signature change which allows us to start using a callback for `request_adapter`. Once we have an event loop, the callback could be scheduled to make this asynchronous. At that point the examples and server would have to be updated, because they currently rely on the callback running immediately (inline).

The reason for this change is that `request_adapter` is supposed to return a `Promise` in JS, so we can represent it with callbacks at the C API, and `Future` in Rust (though we'll probably keep using callbacks initially).

I also changed `request_adapter` to provide an adapter with `AdapterId::ERROR` when no adapters are available, so we can add basic error handling to wgpu-rs (gfx-rs/wgpu-rs#117).

Co-authored-by: Joshua Groves <josh@joshgroves.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants