Merge #1088

1088: Fix and improve object labels and command markers r=cwfitzgerald a=kvark

**Connections**
Fixes a part of #1089 

**Description**
PR contains a bunch of small but important things:
  - reset the command buffer label after submission
  - add labels to compute and pass descriptors
  - actually push/pop markers for the scope of render bundle, compute passes
  - set the label to render pass command buffers
  - set labels on pipelines

**Testing**
Not tested much

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

wgpu-core/src/command/allocator.rs

@@ -87,37 +87,30 @@ impl<B: GfxBackend> CommandAllocator<B> {
         #[cfg(feature = "trace")] enable_tracing: bool,
     ) -> Result<CommandBuffer<B>, CommandAllocatorError> {
         //debug_assert_eq!(device_id.backend(), B::VARIANT);
-        let _ = label; // silence warning on release
         let thread_id = thread::current().id();
         let mut inner = self.inner.lock();
 
         use std::collections::hash_map::Entry;
-        let pool = match inner.pools.entry(thread_id) {
-            Entry::Occupied(e) => e.into_mut(),
-            Entry::Vacant(e) => {
-                tracing::info!("Starting on thread {:?}", thread_id);
-                let raw = unsafe {
-                    device
-                        .create_command_pool(
-                            self.queue_family,
-                            hal::pool::CommandPoolCreateFlags::RESET_INDIVIDUAL,
-                        )
-                        .or(Err(DeviceError::OutOfMemory))?
-                };
-                let pool = CommandPool {
-                    raw,
-                    total: 0,
-                    available: Vec::new(),
-                    pending: Vec::new(),
-                };
-                e.insert(pool)
-            }
-        };
-
-        let init = pool.allocate();
+        if let Entry::Vacant(e) = inner.pools.entry(thread_id) {
+            tracing::info!("Starting on thread {:?}", thread_id);
+            let raw = unsafe {
+                device
+                    .create_command_pool(
+                        self.queue_family,
+                        hal::pool::CommandPoolCreateFlags::RESET_INDIVIDUAL,
+                    )
+                    .or(Err(DeviceError::OutOfMemory))?
+            };
+            e.insert(CommandPool {
+                raw,
+                total: 0,
+                available: Vec::new(),
+                pending: Vec::new(),
+            });
+        }
 
         Ok(CommandBuffer {
-            raw: vec![init],
+            raw: Vec::new(),
             is_recording: true,
             recorded_thread_id: thread_id,
             device_id,
@@ -131,7 +124,6 @@ impl<B: GfxBackend> CommandAllocator<B> {
             } else {
                 None
             },
-            #[cfg(debug_assertions)]
             label: label.to_string_or_default(),
         })
     }
@@ -209,15 +201,26 @@ impl<B: hal::Backend> CommandAllocator<B> {
             .push((raw, submit_index));
     }
 
-    pub fn after_submit(&self, cmd_buf: CommandBuffer<B>, submit_index: SubmissionIndex) {
+    pub fn after_submit(
+        &self,
+        cmd_buf: CommandBuffer<B>,
+        device: &B::Device,
+        submit_index: SubmissionIndex,
+    ) {
         // Record this command buffer as pending
         let mut inner = self.inner.lock();
+        let clear_label = !cmd_buf.label.is_empty();
         inner
             .pools
             .get_mut(&cmd_buf.recorded_thread_id)
             .unwrap()
             .pending
-            .extend(cmd_buf.raw.into_iter().map(|raw| (raw, submit_index)));
+            .extend(cmd_buf.raw.into_iter().map(|mut raw| {
+                if clear_label {
+                    unsafe { device.set_command_buffer_name(&mut raw, "") };
+                }
+                (raw, submit_index)
+            }));
     }
 
     pub fn maintain(&self, device: &B::Device, last_done_index: SubmissionIndex) {

wgpu-core/src/command/bundle.rs

@@ -93,7 +93,7 @@ impl RenderBundleEncoder {
     ) -> Result<Self, CreateRenderBundleError> {
         span!(_guard, INFO, "RenderBundleEncoder::new");
         Ok(Self {
-            base: base.unwrap_or_else(BasePass::new),
+            base: base.unwrap_or_else(|| BasePass::new(&desc.label)),
             parent_id,
             context: RenderPassContext {
                 attachments: AttachmentData {
@@ -114,7 +114,7 @@ impl RenderBundleEncoder {
 
     pub fn dummy(parent_id: id::DeviceId) -> Self {
         Self {
-            base: BasePass::new(),
+            base: BasePass::new(&None),
             parent_id,
             context: RenderPassContext {
                 attachments: AttachmentData {
@@ -419,9 +419,9 @@ impl RenderBundleEncoder {
             }
         }
 
-        let _ = desc.label; //TODO: actually use
         Ok(RenderBundle {
             base: BasePass {
+                label: desc.label.as_ref().map(|cow| cow.to_string()),
                 commands,
                 dynamic_offsets: state.flat_dynamic_offsets,
                 string_data: Vec::new(),
@@ -517,6 +517,9 @@ impl RenderBundle {
 
         let mut offsets = self.base.dynamic_offsets.as_slice();
         let mut pipeline_layout_id = None::<id::Valid<id::PipelineLayoutId>>;
+        if let Some(ref label) = self.base.label {
+            cmd_buf.begin_debug_marker(label, 0);
+        }
 
         for command in self.base.commands.iter() {
             match *command {
@@ -679,6 +682,10 @@ impl RenderBundle {
             }
         }
 
+        if let Some(_) = self.base.label {
+            cmd_buf.end_debug_marker();
+        }
+
         Ok(())
     }
 }

wgpu-core/src/command/compute.rs

@@ -15,7 +15,7 @@ use crate::{
     span,
     track::{TrackerSet, UsageConflict},
     validation::{check_buffer_usage, MissingBufferUsageError},
-    MAX_BIND_GROUPS,
+    Label, MAX_BIND_GROUPS,
 };
 
 use arrayvec::ArrayVec;
@@ -70,9 +70,9 @@ pub struct ComputePass {
 }
 
 impl ComputePass {
-    pub fn new(parent_id: id::CommandEncoderId) -> Self {
+    pub fn new(parent_id: id::CommandEncoderId, desc: &ComputePassDescriptor) -> Self {
         Self {
-            base: BasePass::new(),
+            base: BasePass::new(&desc.label),
             parent_id,
         }
     }
@@ -99,10 +99,9 @@ impl fmt::Debug for ComputePass {
     }
 }
 
-#[repr(C)]
 #[derive(Clone, Debug, Default)]
-pub struct ComputePassDescriptor {
-    pub todo: u32,
+pub struct ComputePassDescriptor<'a> {
+    pub label: Label<'a>,
 }
 
 #[derive(Clone, Debug, Error, PartialEq)]
@@ -251,6 +250,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             });
         }
 
+        if let Some(ref label) = base.label {
+            unsafe {
+                raw.begin_debug_marker(label, 0);
+            }
+        }
+
         let (_, mut token) = hub.render_bundles.read(&mut token);
         let (pipeline_layout_guard, mut token) = hub.pipeline_layouts.read(&mut token);
         let (bind_group_guard, mut token) = hub.bind_groups.read(&mut token);
@@ -510,6 +515,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             }
         }
 
+        if let Some(_) = base.label {
+            unsafe {
+                raw.end_debug_marker();
+            }
+        }
+
         Ok(())
     }
 }

wgpu-core/src/command/mod.rs

@@ -47,7 +47,6 @@ pub struct CommandBuffer<B: hal::Backend> {
     private_features: PrivateFeatures,
     #[cfg(feature = "trace")]
     pub(crate) commands: Option<Vec<crate::device::trace::Command>>,
-    #[cfg(debug_assertions)]
     pub(crate) label: String,
 }
 
@@ -120,6 +119,7 @@ impl<B: hal::Backend> crate::hub::Resource for CommandBuffer<B> {
 
 #[derive(Copy, Clone, Debug)]
 pub struct BasePassRef<'a, C> {
+    pub label: Option<&'a str>,
     pub commands: &'a [C],
     pub dynamic_offsets: &'a [wgt::DynamicOffset],
     pub string_data: &'a [u8],
@@ -137,6 +137,7 @@ pub struct BasePassRef<'a, C> {
     derive(serde::Deserialize)
 )]
 pub struct BasePass<C> {
+    pub label: Option<String>,
     pub commands: Vec<C>,
     pub dynamic_offsets: Vec<wgt::DynamicOffset>,
     pub string_data: Vec<u8>,
@@ -144,8 +145,9 @@ pub struct BasePass<C> {
 }
 
 impl<C: Clone> BasePass<C> {
-    fn new() -> Self {
+    fn new(label: &Label) -> Self {
         Self {
+            label: label.as_ref().map(|cow| cow.to_string()),
             commands: Vec::new(),
             dynamic_offsets: Vec::new(),
             string_data: Vec::new(),
@@ -156,6 +158,7 @@ impl<C: Clone> BasePass<C> {
     #[cfg(feature = "trace")]
     fn from_ref(base: BasePassRef<C>) -> Self {
         Self {
+            label: base.label.map(str::to_string),
             commands: base.commands.to_vec(),
             dynamic_offsets: base.dynamic_offsets.to_vec(),
             string_data: base.string_data.to_vec(),
@@ -165,6 +168,7 @@ impl<C: Clone> BasePass<C> {
 
     pub fn as_ref(&self) -> BasePassRef<C> {
         BasePassRef {
+            label: self.label.as_ref().map(String::as_str),
             commands: &self.commands,
             dynamic_offsets: &self.dynamic_offsets,
             string_data: &self.string_data,
@@ -227,10 +231,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let (mut cmd_buf_guard, _) = hub.command_buffers.write(&mut token);
         let cmd_buf = CommandBuffer::get_encoder(&mut *cmd_buf_guard, encoder_id)?;
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
 
         unsafe {
-            cmb_raw.begin_debug_marker(label, 0);
+            cmd_buf_raw.begin_debug_marker(label, 0);
         }
         Ok(())
     }
@@ -247,10 +251,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let (mut cmd_buf_guard, _) = hub.command_buffers.write(&mut token);
         let cmd_buf = CommandBuffer::get_encoder(&mut *cmd_buf_guard, encoder_id)?;
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
 
         unsafe {
-            cmb_raw.insert_debug_marker(label, 0);
+            cmd_buf_raw.insert_debug_marker(label, 0);
         }
         Ok(())
     }
@@ -266,10 +270,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let (mut cmd_buf_guard, _) = hub.command_buffers.write(&mut token);
         let cmd_buf = CommandBuffer::get_encoder(&mut *cmd_buf_guard, encoder_id)?;
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
 
         unsafe {
-            cmb_raw.end_debug_marker();
+            cmd_buf_raw.end_debug_marker();
         }
         Ok(())
     }

wgpu-core/src/command/render.rs

@@ -23,11 +23,11 @@ use crate::{
     validation::{
         check_buffer_usage, check_texture_usage, MissingBufferUsageError, MissingTextureUsageError,
     },
-    Stored, MAX_BIND_GROUPS,
+    Label, Stored, MAX_BIND_GROUPS,
 };
 
 use arrayvec::ArrayVec;
-use hal::command::CommandBuffer as _;
+use hal::{command::CommandBuffer as _, device::Device as _};
 use thiserror::Error;
 use wgt::{
     BufferAddress, BufferSize, BufferUsage, Color, IndexFormat, InputStepMode, TextureUsage,
@@ -138,6 +138,7 @@ impl DepthStencilAttachmentDescriptor {
 /// Describes the attachments of a render pass.
 #[derive(Clone, Debug, Default, PartialEq)]
 pub struct RenderPassDescriptor<'a> {
+    pub label: Label<'a>,
     /// The color attachments of the render pass.
     pub color_attachments: Cow<'a, [ColorAttachmentDescriptor]>,
     /// The depth and stencil attachment of the render pass, if any.
@@ -153,9 +154,9 @@ pub struct RenderPass {
 }
 
 impl RenderPass {
-    pub fn new(parent_id: id::CommandEncoderId, desc: RenderPassDescriptor) -> Self {
+    pub fn new(parent_id: id::CommandEncoderId, desc: &RenderPassDescriptor) -> Self {
         Self {
-            base: BasePass::new(),
+            base: BasePass::new(&desc.label),
             parent_id,
             color_targets: desc.color_attachments.iter().cloned().collect(),
             depth_stencil_target: desc.depth_stencil_attachment.cloned(),
@@ -497,6 +498,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         }
 
         unsafe {
+            if let Some(ref label) = base.label {
+                device.raw.set_command_buffer_name(&mut raw, label);
+            }
             raw.begin_primary(hal::command::CommandBufferFlags::ONE_TIME_SUBMIT);
         }
 

wgpu-core/src/command/transfer.rs

@@ -404,14 +404,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             dst: destination_offset,
             size,
         };
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
         unsafe {
-            cmb_raw.pipeline_barrier(
+            cmd_buf_raw.pipeline_barrier(
                 all_buffer_stages()..hal::pso::PipelineStage::TRANSFER,
                 hal::memory::Dependencies::empty(),
                 barriers,
             );
-            cmb_raw.copy_buffer(src_raw, dst_raw, iter::once(region));
+            cmd_buf_raw.copy_buffer(src_raw, dst_raw, iter::once(region));
         }
         Ok(())
     }
@@ -539,14 +539,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             image_offset: dst_offset,
             image_extent: conv::map_extent(&image_extent, dst_texture.dimension),
         };
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
         unsafe {
-            cmb_raw.pipeline_barrier(
+            cmd_buf_raw.pipeline_barrier(
                 all_buffer_stages() | all_image_stages()..hal::pso::PipelineStage::TRANSFER,
                 hal::memory::Dependencies::empty(),
                 src_barriers.chain(dst_barriers),
             );
-            cmb_raw.copy_buffer_to_image(
+            cmd_buf_raw.copy_buffer_to_image(
                 src_raw,
                 dst_raw,
                 hal::image::Layout::TransferDstOptimal,
@@ -680,14 +680,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             image_offset: src_offset,
             image_extent: conv::map_extent(&image_extent, src_texture.dimension),
         };
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
         unsafe {
-            cmb_raw.pipeline_barrier(
+            cmd_buf_raw.pipeline_barrier(
                 all_buffer_stages() | all_image_stages()..hal::pso::PipelineStage::TRANSFER,
                 hal::memory::Dependencies::empty(),
                 src_barriers.chain(dst_barrier),
             );
-            cmb_raw.copy_image_to_buffer(
+            cmd_buf_raw.copy_image_to_buffer(
                 src_raw,
                 hal::image::Layout::TransferSrcOptimal,
                 dst_raw,
@@ -817,14 +817,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             dst_offset,
             extent: conv::map_extent(&image_extent, src_texture.dimension),
         };
-        let cmb_raw = cmd_buf.raw.last_mut().unwrap();
+        let cmd_buf_raw = cmd_buf.raw.last_mut().unwrap();
         unsafe {
-            cmb_raw.pipeline_barrier(
+            cmd_buf_raw.pipeline_barrier(
                 all_image_stages()..hal::pso::PipelineStage::TRANSFER,
                 hal::memory::Dependencies::empty(),
                 barriers,
             );
-            cmb_raw.copy_image(
+            cmd_buf_raw.copy_image(
                 src_raw,
                 hal::image::Layout::TransferSrcOptimal,
                 dst_raw,

wgpu-core/src/device/mod.rs

@@ -1725,7 +1725,7 @@ impl<B: GfxBackend> Device<B> {
             parent,
         };
 
-        let raw = match unsafe { self.raw.create_compute_pipeline(&pipeline_desc, None) } {
+        let mut raw = match unsafe { self.raw.create_compute_pipeline(&pipeline_desc, None) } {
             Ok(pipeline) => pipeline,
             Err(hal::pso::CreationError::OutOfMemory(_)) => {
                 return Err(pipeline::CreateComputePipelineError::Device(
@@ -1734,8 +1734,8 @@ impl<B: GfxBackend> Device<B> {
             }
             other => panic!("Compute pipeline creation error: {:?}", other),
         };
-        if let Some(_) = desc.label {
-            //TODO-0.6: self.raw.set_compute_pipeline_name(&mut raw, label);
+        if let Some(ref label) = desc.label {
+            unsafe { self.raw.set_compute_pipeline_name(&mut raw, label) };
         }
 
         let pipeline = pipeline::ComputePipeline {
@@ -2123,7 +2123,7 @@ impl<B: GfxBackend> Device<B> {
             parent,
         };
         // TODO: cache
-        let raw = unsafe {
+        let mut raw = unsafe {
             self.raw
                 .create_graphics_pipeline(&pipeline_desc, None)
                 .map_err(|err| match err {
@@ -2131,8 +2131,8 @@ impl<B: GfxBackend> Device<B> {
                     _ => panic!("failed to create graphics pipeline: {}", err),
                 })?
         };
-        if let Some(_) = desc.label {
-            //TODO-0.6: self.set_graphics_pipeline_name(&mut raw, label)
+        if let Some(ref label) = desc.label {
+            unsafe { self.raw.set_graphics_pipeline_name(&mut raw, label) };
         }
 
         let pass_context = RenderPassContext {
@@ -3391,15 +3391,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 Err(e) => break e,
             };
 
+            let mut raw = device.cmd_allocator.extend(&command_buffer);
             unsafe {
-                let raw_command_buffer = command_buffer.raw.last_mut().unwrap();
                 if let Some(ref label) = desc.label {
-                    device
-                        .raw
-                        .set_command_buffer_name(raw_command_buffer, label);
+                    device.raw.set_command_buffer_name(&mut raw, label);
                 }
-                raw_command_buffer.begin_primary(hal::command::CommandBufferFlags::ONE_TIME_SUBMIT);
+                raw.begin_primary(hal::command::CommandBufferFlags::ONE_TIME_SUBMIT);
             }
+            command_buffer.raw.push(raw);
 
             let id = hub
                 .command_buffers

wgpu-core/src/device/queue.rs

@@ -653,7 +653,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             // finally, return the command buffers to the allocator
             for &cmb_id in command_buffer_ids {
                 if let (Some(cmd_buf), _) = hub.command_buffers.unregister(cmb_id, &mut token) {
-                    device.cmd_allocator.after_submit(cmd_buf, submit_index);
+                    device
+                        .cmd_allocator
+                        .after_submit(cmd_buf, &device.raw, submit_index);
                 }
             }
 

wgpu-core/src/hub.rs

@@ -626,9 +626,10 @@ impl<B: GfxBackend, F: GlobalIdentityHandlerFactory> Hub<B, F> {
         }
         for element in self.command_buffers.data.write().map.drain(..) {
             if let Element::Occupied(command_buffer, _) = element {
-                devices[command_buffer.device_id.value]
+                let device = &devices[command_buffer.device_id.value];
+                device
                     .cmd_allocator
-                    .after_submit(command_buffer, 0);
+                    .after_submit(command_buffer, &device.raw, 0);
             }
         }
         for element in self.bind_groups.data.write().map.drain(..) {