From bfb4dddfd41970fc8700ec64708172a16b41c706 Mon Sep 17 00:00:00 2001 From: Andy Leiserson Date: Fri, 17 Oct 2025 11:44:59 -0400 Subject: [PATCH] Disallow mixing the raw encoding API with the wgpu API --- CHANGELOG.md | 1 + tests/tests/wgpu-validation/api/encoding.rs | 50 ++++++++++++++++ tests/tests/wgpu-validation/api/mod.rs | 1 + wgpu-core/src/as_hal.rs | 6 ++ wgpu-core/src/command/mod.rs | 63 +++++++++++++++++++-- wgpu-core/src/command/ray_tracing.rs | 1 + wgpu-core/src/device/queue.rs | 1 + wgpu/src/api/command_encoder.rs | 12 +++- 8 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 tests/tests/wgpu-validation/api/encoding.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e42655189b..ee20916941c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ SamplerDescriptor { #### General - Texture now has `from_custom`. By @R-Cramer4 in [#8315](https://github.com/gfx-rs/wgpu/pull/8315). +- Using both the wgpu command encoding APIs and `CommandEncoder::as_hal_mut` on the same encoder will now result in a panic. ### Bug Fixes diff --git a/tests/tests/wgpu-validation/api/encoding.rs b/tests/tests/wgpu-validation/api/encoding.rs new file mode 100644 index 00000000000..26eff2dc093 --- /dev/null +++ b/tests/tests/wgpu-validation/api/encoding.rs @@ -0,0 +1,50 @@ +//! Tests of [`wgpu::CommandEncoder`] and related. + +#[test] +fn as_hal() { + // Sanity-test that the raw encoding API isn't completely broken. + + let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default()); + + let mut encoder = device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); + unsafe { + encoder.as_hal_mut::(|_| ()); + } + encoder.finish(); +} + +#[test] +#[should_panic = "Mixing the wgpu encoding API with the raw encoding API is not permitted"] +fn mix_apis_wgpu_then_hal() { + let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default()); + + let buffer = device.create_buffer(&wgpu::BufferDescriptor { + label: None, + size: 256, + usage: wgpu::BufferUsages::COPY_DST, + mapped_at_creation: false, + }); + let mut encoder = device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); + encoder.clear_buffer(&buffer, 0, None); + unsafe { + encoder.as_hal_mut::(|_| ()); + } +} + +#[test] +#[should_panic = "Mixing the wgpu encoding API with the raw encoding API is not permitted"] +fn mix_apis_hal_then_wgpu() { + let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default()); + + let buffer = device.create_buffer(&wgpu::BufferDescriptor { + label: None, + size: 256, + usage: wgpu::BufferUsages::COPY_DST, + mapped_at_creation: false, + }); + let mut encoder = device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); + unsafe { + encoder.as_hal_mut::(|_| ()); + } + encoder.clear_buffer(&buffer, 0, None); +} diff --git a/tests/tests/wgpu-validation/api/mod.rs b/tests/tests/wgpu-validation/api/mod.rs index 541647bcc26..414d7aac0f4 100644 --- a/tests/tests/wgpu-validation/api/mod.rs +++ b/tests/tests/wgpu-validation/api/mod.rs @@ -4,6 +4,7 @@ mod buffer_mapping; mod buffer_slice; mod command_buffer_actions; mod device; +mod encoding; mod experimental; mod external_texture; mod instance; diff --git a/wgpu-core/src/as_hal.rs b/wgpu-core/src/as_hal.rs index 390c496236e..cb09f03ab6b 100644 --- a/wgpu-core/src/as_hal.rs +++ b/wgpu-core/src/as_hal.rs @@ -330,6 +330,12 @@ impl Global { }) } + /// Encode commands using the raw HAL command encoder. + /// + /// # Panics + /// + /// If the command encoder has already been used with the wgpu encoding API. + /// /// # Safety /// /// - The raw command encoder handle must not be manually destroyed diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 26317bd4f41..77848a312a0 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -177,6 +177,7 @@ impl CommandEncoderStatus { ) -> Result<(), EncoderStateError> { match self { Self::Recording(cmd_buf_data) => { + cmd_buf_data.encoder.api.set(EncodingApi::Wgpu); match f() { Ok(cmd) => cmd_buf_data.commands.push(cmd), Err(err) => { @@ -220,10 +221,12 @@ impl CommandEncoderStatus { E: Clone + Into, >( &mut self, + api: EncodingApi, f: F, ) -> Result<(), EncoderStateError> { match self { - Self::Recording(_) => { + Self::Recording(inner) => { + inner.encoder.api.set(api); RecordingGuard { inner: self }.record(f); Ok(()) } @@ -256,7 +259,10 @@ impl CommandEncoderStatus { f: F, ) -> T { match self { - Self::Recording(_) => RecordingGuard { inner: self }.record_as_hal_mut(f), + Self::Recording(inner) => { + inner.encoder.api.set(EncodingApi::Raw); + RecordingGuard { inner: self }.record_as_hal_mut(f) + } Self::Locked(_) => { self.invalidate(EncoderStateError::Locked); f(None) @@ -358,8 +364,11 @@ impl CommandEncoderStatus { // state or an error, to be transferred to the command buffer. match mem::replace(self, Self::Consumed) { Self::Recording(inner) => { - // Nothing should have opened the encoder yet. - assert!(!inner.encoder.is_open); + // Raw encoding leaves the encoder open in `command_encoder_as_hal_mut`. + // Otherwise, nothing should have opened it yet. + if inner.encoder.api != EncodingApi::Raw { + assert!(!inner.encoder.is_open); + } Self::Finished(inner) } Self::Consumed | Self::Finished(_) => Self::Error(EncoderStateError::Ended.into()), @@ -480,6 +489,38 @@ impl Drop for CommandEncoder { } } +/// The encoding API being used with a `CommandEncoder`. +/// +/// Mixing APIs on the same encoder is not allowed. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum EncodingApi { + // The regular wgpu encoding APIs are being used. + Wgpu, + + // The raw hal encoding API is being used. + Raw, + + // Neither encoding API has been called yet. + Undecided, + + // The encoder is used internally by wgpu. + InternalUse, +} + +impl EncodingApi { + pub(crate) fn set(&mut self, api: EncodingApi) { + match *self { + EncodingApi::Undecided => { + *self = api; + } + self_api if self_api != api => { + panic!("Mixing the wgpu encoding API with the raw encoding API is not permitted"); + } + _ => {} + } + } +} + /// A raw [`CommandEncoder`][rce], and the raw [`CommandBuffer`][rcb]s built from it. /// /// Each wgpu-core [`CommandBuffer`] owns an instance of this type, which is @@ -528,6 +569,13 @@ pub(crate) struct InnerCommandEncoder { /// [`wgpu_hal::CommandEncoder`]: hal::CommandEncoder pub(crate) is_open: bool, + /// Tracks which API is being used to encode commands. + /// + /// Mixing the wgpu encoding API with access to the raw hal encoder via + /// `as_hal_mut` is not supported. this field tracks which API is being used + /// in order to detect and reject invalid usage. + pub(crate) api: EncodingApi, + pub(crate) label: String, } @@ -790,6 +838,7 @@ impl CommandEncoder { list: Vec::new(), device: device.clone(), is_open: false, + api: EncodingApi::Undecided, label: label.to_string(), }, trackers: Tracker::new(), @@ -916,6 +965,12 @@ impl CommandEncoder { let snatch_guard = self.device.snatchable_lock.read(); let mut debug_scope_depth = 0; + if cmd_buf_data.encoder.api == EncodingApi::Raw { + // Should have panicked on the first call that switched APIs, + // but lets be sure. + assert!(cmd_buf_data.commands.is_empty()); + } + let mut commands = mem::take(&mut cmd_buf_data.commands); for command in commands.drain(..) { if matches!( diff --git a/wgpu-core/src/command/ray_tracing.rs b/wgpu-core/src/command/ray_tracing.rs index 9f1c0066776..ac4547ff232 100644 --- a/wgpu-core/src/command/ray_tracing.rs +++ b/wgpu-core/src/command/ray_tracing.rs @@ -87,6 +87,7 @@ impl Global { let mut cmd_buf_data = cmd_enc.data.lock(); cmd_buf_data.with_buffer( + crate::command::EncodingApi::Raw, |cmd_buf_data| -> Result<(), BuildAccelerationStructureError> { let device = &cmd_enc.device; device.check_is_valid()?; diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 0f36ad1e5f7..931effcd31e 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -406,6 +406,7 @@ impl PendingWrites { list: vec![cmd_buf], device: device.clone(), is_open: false, + api: crate::command::EncodingApi::InternalUse, label: "(wgpu internal) PendingWrites command encoder".into(), }, trackers: Tracker::new(), diff --git a/wgpu/src/api/command_encoder.rs b/wgpu/src/api/command_encoder.rs index 30afbb90905..9f3daf06a22 100644 --- a/wgpu/src/api/command_encoder.rs +++ b/wgpu/src/api/command_encoder.rs @@ -310,8 +310,16 @@ impl CommandEncoder { /// [`Features::EXPERIMENTAL_RAY_QUERY`] must be enabled on the device in order to call these functions. impl CommandEncoder { - /// Mark acceleration structures as being built. ***Should only*** be used with wgpu-hal - /// functions, all wgpu functions already mark acceleration structures as built. + /// When encoding the acceleration structure build with the raw Hal encoder + /// (obtained from [`CommandEncoder::as_hal_mut`]), this function marks the + /// acceleration structures as having been built. + /// + /// This function must only be used with the raw encoder API. When using the + /// wgpu encoding API, acceleration structure build is tracked automatically. + /// + /// # Panics + /// + /// - If the encoder is being used with the wgpu encoding API. /// /// # Safety ///