Open melvdlin opened 3 weeks ago
If the cause is exactly as described, perhaps a simple fix for this would be to add:
impl Drop for ComputePass { fn drop(&mut self) {} }
impl Drop for RenderPass { fn drop(&mut self) {} }
This should require the lifetime to be valid when the pass is dropped.
I can confirm the crash on my Win11 machine. The workaround typically shown in examples right now is to scope the compute_pass
so that it's dropped before calling encoder.finish()
:
--- a/src/main.rs
+++ b/src/main.rs
@@ -5,8 +5,10 @@
pollster::block_on(adapter.request_device(&Default::default(), None)).unwrap();
let mut encoder = device.create_command_encoder(&Default::default());
- let compute_pass = encoder.begin_compute_pass(&Default::default());
- // set up the compute pass...
+ {
+ let compute_pass = encoder.begin_compute_pass(&Default::default());
+ // set up the compute pass...
+ }
let cmd_buffer = encoder.finish();
assert!(device
…but, of course, this is still a paper cut we should strive to eliminate in our Rust-y interpretation of the WebGPU API.
I interpret @kpreid's suggestion to mean a diff. like this against current trunk
:
diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs
index eb929740c8..f216d1b40f 100644
--- a/wgpu-core/src/command/compute.rs
+++ b/wgpu-core/src/command/compute.rs
@@ -95,6 +95,10 @@
}
}
+impl Drop for ComputePass {
+ fn drop(&mut self) {}
+}
+
#[derive(Clone, Debug, Default)]
pub struct ComputePassDescriptor<'a> {
pub label: Label<'a>,
diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs
index e4d93b042e..efc80d7475 100644
--- a/wgpu-core/src/command/render.rs
+++ b/wgpu-core/src/command/render.rs
@@ -321,6 +321,10 @@
}
}
+impl Drop for RenderPass {
+ fn drop(&mut self) {}
+}
+
#[derive(Debug, PartialEq)]
enum OptionalState {
Unused,
…but this doesn't change any behavior in the MRE posted in the OP.
Render/compute passes no longer have lifetimes (https://github.com/gfx-rs/wgpu/issues/1453 was recently closed by @Wumpf's PRs).
We might want to expose the end/finish methods and require usage of those instead of implicitly doing it on drop.
They do still have lifetimes to their encoder.
No, the RenderPass drop needs to be added in wgpu
itself.
Ah, I see, I should have checked before writing that :)
Description Borrow shortening can cause
CommandEncoder::finish
to be callable before a derivedComputePass
is automatically dropped. Since theDrop
impl onComputePassInner
is what unlocks theCommandEncoder
, this leads to an unexpected validation error, which can be avoided by explicitly dropping of theComputePass
before callingCommandEncoder::finish
.The same likely applies to
RenderPass
.Repro steps
Expected vs observed behavior A panic occurs at line 12:
Since
ComputePass
's lifetime is bounded by a mutable borrow of theCommandEncoder
, one would expect theCommandEncoder
to be unlocked and a call tofinish
to be valid as soon as said borrow is invalidated. This is, however, not the case, since theDrop
impl ofComputePassInner
is only run at the end of scope, not as soon as the borrow is invalidated.The documentation of
CommandEncoder::begin_compute_pass
does contain the following line:As long as the returned ComputePass has not ended, any mutating operation on this command encoder causes an error and invalidates it.
. It does not specify that "a compute pass ending" specifically means "theComputePass
being dropped".Platform
wgpu = "22.1.0"