-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use callback for request_adapter
#375
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
Conversation
@@ -301,11 +302,16 @@ pub extern "C" fn wgpu_create_surface_from_windows_hwnd( | |||
)) | |||
} | |||
|
|||
pub fn request_adapter( | |||
pub type RequestAdapterCallback = | |||
extern "C" fn(adapter: *const AdapterId, userdata: *mut c_void); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed we use *mut u8
for userdata
in buffer mapping, but maybe we should consider using *mut c_void
like we do here. For example, we expect userdata
to generally be a user-provided type anyway (not necessarily a byte array), so is there any benefit from requiring *mut u8
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I agree, *mut void seems more appropriate. We might need to file a PR to upstream webgpu header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
So how do you see us proceeding here? If we merge, all the examples are going to be broken.
Thank you for addressing my concerns! |
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 <[email protected]>
Build succeeded |
375: Add water example. r=kvark,cwfitzgerald a=OptimisticPeach Solves gfx-rs#329, water example requested by @kvark. I tuned it to my personal preference of visuals, however it might be different for you. Note: I used https://github.com/ashima/webgl-noise for 3D open simplex noise. I've commented to explain what most of the things in the rust side of the example mean. However, I'm not 100% sure I did the best job at giving a brief overview, and wouldn't mind someone making sure my terminology/definitions are correct. Thanks! Patrik Co-authored-by: OptimisticPeach <[email protected]>
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 aPromise
in JS, so we can represent it with callbacks at the C API, andFuture
in Rust (though we'll probably keep using callbacks initially).I also changed
request_adapter
to provide an adapter withAdapterId::ERROR
when no adapters are available, so we can add basic error handling to wgpu-rs (gfx-rs/wgpu-rs#117).