Disallow mixing the raw encoding API with the wgpu API (#8373)

Author: andyleiserson

CHANGELOG.md

@@ -78,6 +78,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
 

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::<wgpu_hal::api::Noop, _, ()>(|_| ());
+    }
+    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::<wgpu_hal::api::Noop, _, ()>(|_| ());
+    }
+}
+
+#[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::<wgpu_hal::api::Noop, _, ()>(|_| ());
+    }
+    encoder.clear_buffer(&buffer, 0, None);
+}

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;

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

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<CommandEncoderError>,
     >(
         &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!(

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()?;

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(),

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
     ///