The Context trait #281
The Context trait #281
Conversation
| } | ||
| } | ||
| } | ||
|
|
||
| impl Queue { | ||
| /// Submits a series of finished command buffers for execution. | ||
| pub fn submit(&self, command_buffers: &[CommandBuffer]) { | ||
| backend::queue_submit(&self.id, command_buffers); | ||
| pub fn submit<I: IntoIterator<Item = CommandBuffer>>(&self, command_buffers: I) { |
There was a problem hiding this comment.
Couldn't this be inefficient for webgpu.h, since that takes a pointer to a buffer of command buffers? You'd have to allocate a Vec or similar to use that. Whereas, a slice could just be passed efficiently.
There was a problem hiding this comment.
Nah, this is more flexible than a slice. You can always go from a slice to an iterator for free, but not the other way around. I.e. wgpu-native would do something like: command_buffer_slice.iter().cloned() for passing it here.
| ); | ||
| } | ||
|
|
||
| trait Context: Sized { |
There was a problem hiding this comment.
I think the traits should be in a different file so it's easier to read this one.
There was a problem hiding this comment.
It could be, yes. I find it convenient here, because it uses all the types in the proximity, and everything uses it. We can follow-up with moving it if we see fit, after the initial work is done.
|
@lachlansneff thank you for the review!
It's a fair thing to be concerned about. My understanding is that this is an extremely light overhead, since it only happens at object creation time, which is dominated by other factors. |
Main change here is the introduction of the Context trait. The "direct" implementation of it goes straight to wgpu-core. This commit also has small improvements: - consuming command buffers on submit - Instance type - proper call to destructors
kvark
changed the title
|
@grovesNL I think this is done. Please take a final (?) look. |
| } | ||
|
|
||
| /// Creates a surface from a raw window handle. | ||
| pub unsafe fn create_surface<W: raw_window_handle::HasRawWindowHandle>( |
There was a problem hiding this comment.
Maybe we could eventually avoid unsafe here later if we fully validate the handles internally
There was a problem hiding this comment.
I'd very much like that, but not sure if this is achievable via RawWindowHandle: after all, it's just a bunch of raw pointers. I.e. a winit-based API could be safe, but once we lower to the raw handles, safety zone is gone.
| options: &crate::RequestAdapterOptions<'_>, | ||
| _backends: wgt::BackendBit, | ||
| ) -> Self::RequestAdapterFuture { | ||
| //assert!(backends.contains(wgt::BackendBit::BROWSER_WEBGPU)); |
There was a problem hiding this comment.
Why do we skip this now?
There was a problem hiding this comment.
It's annoying/hard to get this check back in (but possible). I've added a detailed TODO here.
|
@grovesNL thank you for the quick review! |
281: Force pipeline barriers between unordered usages r=grovesNL a=kvark Fixes gfx-rs#272 Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
302: Fixed pipeline barriers that are not transitions r=grovesNL a=kvark The actual fix is a one-liner: `u.start != u.end` bit in `PendingTransition::record`. The case is relatively new - as of gfx-rs#281, which I haven't tested extensively. The PR also improves our logging for further assistance with similar issues... but the most annoying piece is that I would find this much much earlier if I didn't ignore the result here: `let _ = state.change(...)`. Let it be the lesson to all of us :) Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
The main motivation here is to avoid blocking the wgpu-core updates by
wgpu-native. Instead,wgpu-nativebecomes a branch, and the dependency ofwgpu-rs->wgpu-nativestarts adhering to the contract/API of the standard webgpu-native headers.The biggest change is the introduction of the Context trait. I recall us discussing 2 downsides to having this trait:
What this gives in return is a well established contract with the backends. Unlike gfx-rs, the backend code is right here, a part of the crate, so the contract is only for internal use.
Fixes #156 : the "direct" implementation of it goes straight to wgpu-core. What this gives us is less overhead for command recording, since there is no longer an extra indirection on every command, and no heap allocation at the end of a render pass.
The downside of this PR is one extra
Arc(with addref) per object.This commit also has small improvements:
submit#267)request_device