Skip to content

Commit afa0f71

Browse files
[deno] Fix dropping of command encoders/buffers, and an enum typo (gfx-rs#7808)
Fixes gfx-rs#7797
1 parent 371c8fd commit afa0f71

6 files changed

Lines changed: 35 additions & 24 deletions

File tree

cts_runner/test.lst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ webgpu:api,operation,compute,basic:memcpy:*
99
//FAIL: webgpu:api,operation,compute,basic:large_dispatch:*
1010
webgpu:api,operation,compute_pipeline,overrides:*
1111
webgpu:api,operation,device,lost:*
12+
webgpu:api,validation,queue,submit:command_buffer,device_mismatch:*
13+
webgpu:api,validation,queue,submit:command_buffer,duplicate_buffers:*
14+
webgpu:api,validation,queue,submit:command_buffer,submit_invalidates:*
15+
//FAIL: webgpu:api,validation,queue,submit:command_buffer,invalid_submit_invalidates:*
16+
// https://github.com/gfx-rs/wgpu/issues/3911#issuecomment-2972995675
1217
webgpu:api,operation,render_pipeline,overrides:*
1318
webgpu:api,operation,rendering,basic:clear:*
1419
webgpu:api,operation,rendering,basic:fullscreen_quad:*

deno_webgpu/command_buffer.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
// Copyright 2018-2025 the Deno authors. MIT license.
22

3-
use std::cell::OnceCell;
4-
53
use deno_core::op2;
64
use deno_core::GarbageCollected;
75
use deno_core::WebIDL;
@@ -12,15 +10,11 @@ pub struct GPUCommandBuffer {
1210
pub instance: Instance,
1311
pub id: wgpu_core::id::CommandBufferId,
1412
pub label: String,
15-
16-
pub consumed: OnceCell<()>,
1713
}
1814

1915
impl Drop for GPUCommandBuffer {
2016
fn drop(&mut self) {
21-
if self.consumed.get().is_none() {
22-
self.instance.command_buffer_drop(self.id);
23-
}
17+
self.instance.command_buffer_drop(self.id);
2418
}
2519
}
2620

deno_webgpu/command_encoder.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use std::borrow::Cow;
44
use std::cell::RefCell;
5+
use std::sync::atomic::{AtomicBool, Ordering};
56

67
use deno_core::cppgc::Ptr;
78
use deno_core::op2;
@@ -28,11 +29,19 @@ pub struct GPUCommandEncoder {
2829

2930
pub id: wgpu_core::id::CommandEncoderId,
3031
pub label: String,
32+
33+
pub finished: AtomicBool,
3134
}
3235

3336
impl Drop for GPUCommandEncoder {
3437
fn drop(&mut self) {
35-
self.instance.command_encoder_drop(self.id);
38+
// Command encoders and command buffers are both the same wgpu object.
39+
// At the time `finished` is set, ownership of the id (and
40+
// responsibility for dropping it) transfers from the encoder to the
41+
// buffer.
42+
if !self.finished.load(Ordering::SeqCst) {
43+
self.instance.command_encoder_drop(self.id);
44+
}
3645
}
3746
}
3847

@@ -407,23 +416,34 @@ impl GPUCommandEncoder {
407416
fn finish(
408417
&self,
409418
#[webidl] descriptor: crate::command_buffer::GPUCommandBufferDescriptor,
410-
) -> GPUCommandBuffer {
419+
) -> Result<GPUCommandBuffer, JsErrorBox> {
411420
let wgpu_descriptor = wgpu_types::CommandBufferDescriptor {
412421
label: crate::transform_label(descriptor.label.clone()),
413422
};
414423

424+
// TODO(https://github.com/gfx-rs/wgpu/issues/7812): This is not right,
425+
// it should be a validation error, and it would be nice if we can just
426+
// let wgpu generate it for us. The problem is that if the encoder was
427+
// already finished, we transferred ownership of the id to a command
428+
// buffer, so we have to bail out before we mint a duplicate command
429+
// buffer with the same id below.
430+
if self.finished.fetch_or(true, Ordering::SeqCst) {
431+
return Err(JsErrorBox::type_error(
432+
"The command encoder has already finished.",
433+
));
434+
}
435+
415436
let (id, err) = self
416437
.instance
417438
.command_encoder_finish(self.id, &wgpu_descriptor);
418439

419440
self.error_handler.push_error(err);
420441

421-
GPUCommandBuffer {
442+
Ok(GPUCommandBuffer {
422443
instance: self.instance.clone(),
423444
id,
424445
label: descriptor.label,
425-
consumed: Default::default(),
426-
}
446+
})
427447
}
428448

429449
fn push_debug_group(&self, #[webidl] group_label: String) {

deno_webgpu/device.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ impl GPUDevice {
498498
error_handler: self.error_handler.clone(),
499499
id,
500500
label,
501+
finished: Default::default(),
501502
}
502503
}
503504

deno_webgpu/queue.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,8 @@ impl GPUQueue {
5959
) -> Result<v8::Local<v8::Value>, JsErrorBox> {
6060
let ids = command_buffers
6161
.into_iter()
62-
.enumerate()
63-
.map(|(i, cb)| {
64-
if cb.consumed.set(()).is_err() {
65-
Err(JsErrorBox::type_error(format!(
66-
"The command buffer at position {i} has already been submitted."
67-
)))
68-
} else {
69-
Ok(cb.id)
70-
}
71-
})
72-
.collect::<Result<Vec<_>, _>>()?;
62+
.map(|cb| cb.id)
63+
.collect::<Vec<_>>();
7364

7465
let err = self.instance.queue_submit(self.id, &ids).err();
7566

deno_webgpu/texture.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ impl From<GPUTextureFormat> for TextureFormat {
550550
channel: AstcChannel::Unorm,
551551
},
552552
GPUTextureFormat::Astc4x4UnormSrgb => Self::Astc {
553-
block: AstcBlock::B5x4,
553+
block: AstcBlock::B4x4,
554554
channel: AstcChannel::UnormSrgb,
555555
},
556556
GPUTextureFormat::Astc5x4Unorm => Self::Astc {

0 commit comments

Comments
 (0)