Skip to content

Commit 5633ae8

Browse files
kpreidcwfitzgerald
authored andcommitted
Show command encoder label in validation errors.
The plumbing to get it out of `wgpu_core` is awkward but I didn’t see a clearly better option. I think it would make sense to have something like `Global::get_label<T>(id: Id<T>) -> String` but that looks like a lot more work.
1 parent 586b344 commit 5633ae8

5 files changed

Lines changed: 44 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ SamplerDescriptor {
8787
- Using both the wgpu command encoding APIs and `CommandEncoder::as_hal_mut` on the same encoder will now result in a panic.
8888
- Allow `include_spirv!` and `include_spirv_raw!` macros to be used in constants and statics. By @clarfonthey in [#8250](https://github.com/gfx-rs/wgpu/pull/8250).
8989
- Added support for rendering onto multi-planar textures. By @noituri in [#8307](https://github.com/gfx-rs/wgpu/pull/8307).
90+
- Validation errors from `CommandEncoder::finish()` will report the label of the invalid encoder. By @kpreid in [#8449](https://github.com/gfx-rs/wgpu/pull/8449).
9091
- Corrected documentation of the minimum alignment of the *end* of a mapped range of a buffer (it is 4, not 8). By @kpreid in [#8450](https://github.com/gfx-rs/wgpu/pull/8450).
9192

9293
### Bug Fixes

deno_webgpu/command_encoder.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,12 +437,14 @@ impl GPUCommandEncoder {
437437
label: crate::transform_label(descriptor.label.clone()),
438438
};
439439

440-
let (id, err) =
440+
let (id, opt_label_and_err) =
441441
self
442442
.instance
443443
.command_encoder_finish(self.id, &wgpu_descriptor, None);
444444

445-
self.error_handler.push_error(err);
445+
self
446+
.error_handler
447+
.push_error(opt_label_and_err.map(|(_label, err)| err));
446448

447449
GPUCommandBuffer {
448450
instance: self.instance.clone(),

tests/tests/wgpu-validation/api/encoding.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,23 @@ fn mix_apis_hal_then_wgpu() {
4848
}
4949
encoder.clear_buffer(&buffer, 0, None);
5050
}
51+
52+
/// Test that the command encoder’s label is remembered and used in errors.
53+
#[test]
54+
#[should_panic = "In a CommandEncoder, label = 'my encoder'"]
55+
fn encoding_error_contains_label_of_encoder() {
56+
let (device, queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
57+
let buffer = device.create_buffer(&wgpu::BufferDescriptor {
58+
label: Some("my buffer"),
59+
size: 1024,
60+
usage: wgpu::BufferUsages::MAP_READ,
61+
mapped_at_creation: false,
62+
});
63+
64+
let mut encoder = device.create_command_encoder(&wgpu::CommandEncoderDescriptor {
65+
label: Some("my encoder"),
66+
});
67+
// This is erroneous because it is copying to the same buffer.
68+
encoder.copy_buffer_to_buffer(&buffer, 0, &buffer, 0, 10);
69+
queue.submit([encoder.finish()]);
70+
}

wgpu-core/src/command/mod.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1589,21 +1589,31 @@ impl Global {
15891589
self.hub.query_sets.get(query_set_id).get()
15901590
}
15911591

1592+
/// Finishes a command encoder, creating a command buffer and returning errors that were
1593+
/// deferred until now.
1594+
///
1595+
/// The returned `String` is the label of the command encoder, supplied so that `wgpu` can
1596+
/// include the label when printing deferred errors without having its own copy of the label.
1597+
/// This is a kludge and should be replaced if we think of a better solution to propagating
1598+
/// labels.
15921599
pub fn command_encoder_finish(
15931600
&self,
15941601
encoder_id: id::CommandEncoderId,
15951602
desc: &wgt::CommandBufferDescriptor<Label>,
15961603
id_in: Option<id::CommandBufferId>,
1597-
) -> (id::CommandBufferId, Option<CommandEncoderError>) {
1604+
) -> (id::CommandBufferId, Option<(String, CommandEncoderError)>) {
15981605
profiling::scope!("CommandEncoder::finish");
15991606

16001607
let hub = &self.hub;
16011608
let cmd_enc = hub.command_encoders.get(encoder_id);
16021609

1603-
let (cmd_buf, error) = cmd_enc.finish(desc);
1610+
let (cmd_buf, opt_error) = cmd_enc.finish(desc);
16041611
let cmd_buf_id = hub.command_buffers.prepare(id_in).assign(cmd_buf);
16051612

1606-
(cmd_buf_id, error)
1613+
(
1614+
cmd_buf_id,
1615+
opt_error.map(|error| (cmd_enc.label.clone(), error)),
1616+
)
16071617
}
16081618

16091619
pub fn command_encoder_push_debug_group(

wgpu/src/backend/wgpu_core.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,13 +2577,13 @@ impl dispatch::CommandEncoderInterface for CoreCommandEncoder {
25772577

25782578
fn finish(&mut self) -> dispatch::DispatchCommandBuffer {
25792579
let descriptor = wgt::CommandBufferDescriptor::default();
2580-
let (id, error) = self
2581-
.context
2582-
.0
2583-
.command_encoder_finish(self.id, &descriptor, None);
2584-
if let Some(cause) = error {
2580+
let (id, opt_label_and_error) =
2581+
self.context
2582+
.0
2583+
.command_encoder_finish(self.id, &descriptor, None);
2584+
if let Some((label, cause)) = opt_label_and_error {
25852585
self.context
2586-
.handle_error_nolabel(&self.error_sink, cause, "a CommandEncoder");
2586+
.handle_error(&self.error_sink, cause, Some(&label), "a CommandEncoder");
25872587
}
25882588
CoreCommandBuffer {
25892589
context: self.context.clone(),

0 commit comments

Comments
 (0)