Improve push constant documentation, including internal docs.

wgpu-core/src/command/bundle.rs

@@ -6,6 +6,18 @@ times, on different encoders. Constructing a render bundle lets `wgpu` validate
 and analyze its commands up front, so that replaying a bundle can be more
 efficient than simply re-recording its commands each time.
 
+Not all commands are available in bundles; for example, a render bundle may not
+contain a [`RenderCommand::SetViewport`] command.
+
+Most of `wgpu`'s backend graphics APIs have something like bundles. For example,
+Vulkan calls them "secondary command buffers", and Metal calls them "indirect
+command buffers". Although we plan to take advantage of these platform features
+at some point in the future, for now `wgpu`'s implementation of render bundles
+does not use them: at the hal level, `wgpu` render bundles just replay the
+commands.
+
+## Render Bundle Isolation
+
 One important property of render bundles is that the draw calls in a render
 bundle depend solely on the pipeline and state established within the render
 bundle itself. A draw call in a bundle will never use a vertex buffer, say, that
@@ -17,14 +29,11 @@ Render passes are also isolated from the effects of bundles. After executing a
 render bundle, a render pass's pipeline, bind groups, and vertex and index
 buffers are are unset, so the bundle cannot affect later draw calls in the pass.
 
-Not all commands are available in bundles; for example, a render bundle may not
-contain a [`RenderCommand::SetViewport`] command.
-
-Most of `wgpu`'s backend graphics APIs have something like bundles. For example,
-Vulkan calls them "secondary command buffers", and Metal calls them "indirect
-command buffers". However, `wgpu`'s implementation of render bundles does not
-take advantage of those underlying platform features. At the hal level, `wgpu`
-render bundles just replay the commands.
+A render pass is not fully isolated from a bundle's effects on push constant
+values. Draw calls following a bundle's execution will see whatever values the
+bundle writes to push constant storage. Setting a pipeline initializes any push
+constant storage it could access to zero, and this initialization may also be
+visible after bundle execution.
 
 ## Render Bundle Lifecycle
 
@@ -372,6 +381,11 @@ impl RenderBundleEncoder {
                         &layout.push_constant_ranges,
                     );
                     commands.push(command);
+
+                    // If this pipeline's push constant ranges aren't the same
+                    // as the ones we were using previously (or if this is the
+                    // first pipeline to use push constants at all), then emit
+                    // commands to zero out the push constant values it will use.
                     if let Some(iter) = state.flush_push_constants() {
                         commands.extend(iter)
                     }
@@ -1044,7 +1058,15 @@ struct BindState {
 
 #[derive(Debug)]
 struct PushConstantState {
+    /// Push constant ranges used by the most recently set pipeline.
+    ///
+    /// Before any pipeline has been set, this is empty.
     ranges: ArrayVec<wgt::PushConstantRange, { SHADER_STAGE_COUNT }>,
+
+    /// True if this bundle has ever set a pipeline that uses push constants.
+    ///
+    /// If this is true, then every time we set a pipeline, we will emit
+    /// `SetPushConstant` commands to clear the push constants it uses.
     is_dirty: bool,
 }
 impl PushConstantState {
@@ -1246,6 +1268,9 @@ impl<A: HalApi> State<A> {
         self.index.as_mut().and_then(|index| index.flush())
     }
 
+    /// Return a sequence of commands to zero the push constant ranges that will
+    /// be used by the current pipeline. If no initialization is necessary,
+    /// return `None`.
     fn flush_push_constants(&mut self) -> Option<impl Iterator<Item = RenderCommand>> {
         let is_dirty = self.push_constant_ranges.is_dirty;
 
@@ -1260,7 +1285,7 @@ impl<A: HalApi> State<A> {
                         stages: range.stages,
                         offset: range.range.start,
                         size_bytes: range.range.end - range.range.start,
-                        values_offset: None,
+                        values_offset: None, // write zeros
                     }),
             )
         } else {

wgpu-core/src/command/compute.rs

@@ -43,11 +43,24 @@ pub enum ComputeCommand {
         bind_group_id: id::BindGroupId,
     },
     SetPipeline(id::ComputePipelineId),
+
+    /// Set a range of push constants to values stored in [`BasePass::push_constant_data`].
     SetPushConstant {
+        /// The byte offset within the push constant storage to write to. This
+        /// must be a multiple of four.
         offset: u32,
+
+        /// The number of bytes to write. This must be a multiple of four.
         size_bytes: u32,
+
+        /// Index in [`BasePass::push_constant_data`] of the start of the data
+        /// to be written.
+        ///
+        /// Note: this is not a byte offset like `offset`. Rather, it is the
+        /// index of the first `u32` element in `push_constant_data` to read.
         values_offset: u32,
     },
+
     Dispatch([u32; 3]),
     DispatchIndirect {
         buffer_id: id::BufferId,

wgpu-core/src/command/draw.rs

@@ -172,13 +172,32 @@ pub enum RenderCommand {
         depth_max: f32,
     },
     SetScissor(Rect<u32>),
+
+    /// Set a range of push constants to values stored in [`BasePass::push_constant_data`].
+    ///
+    /// See [`wgpu::RenderPass::set_push_constants`] for a detailed explanation
+    /// of the restrictions these commands must satisfy.
     SetPushConstant {
+        /// Which stages we are setting push constant values for.
         stages: wgt::ShaderStages,
+
+        /// The byte offset within the push constant storage to write to.  This
+        /// must be a multiple of four.
         offset: u32,
+
+        /// The number of bytes to write. This must be a multiple of four.
         size_bytes: u32,
-        /// None means there is no data and the data should be an array of zeros.
+
+        /// Index in [`BasePass::push_constant_data`] of the start of the data
+        /// to be written.
+        ///
+        /// Note: this is not a byte offset like `offset`. Rather, it is the
+        /// index of the first `u32` element in `push_constant_data` to read.
         ///
-        /// Facilitates clears in renderbundles which explicitly do their clears.
+        /// `None` means zeros should be written to the destination range, and
+        /// there is no corresponding data in `push_constant_data`. This is used
+        /// by render bundles, which explicitly clear out any state that
+        /// post-bundle code might see.
         values_offset: Option<u32>,
     },
     Draw {

wgpu-core/src/command/mod.rs

@@ -287,6 +287,10 @@ pub struct BasePass<C> {
     /// instruction consumes the next `len` bytes from this vector.
     pub string_data: Vec<u8>,
 
+    /// Data used by `SetPushConstant` instructions.
+    ///
+    /// See the documentation for [`RenderCommand::SetPushConstant`]
+    /// and [`ComputeCommand::SetPushConstant`] for details.
     pub push_constant_data: Vec<u32>,
 }
 

wgpu/src/lib.rs

@@ -3016,34 +3016,46 @@ impl<'a> RenderPass<'a> {
 
 /// [`Features::PUSH_CONSTANTS`] must be enabled on the device in order to call these functions.
 impl<'a> RenderPass<'a> {
-    /// Set push constant data.
+    /// Set push constant data for subsequent draw calls.
     ///
-    /// Offset is measured in bytes, but must be a multiple of [`PUSH_CONSTANT_ALIGNMENT`].
+    /// Write the bytes in `data` at offset `offset` within push constant
+    /// storage, all of which are accessible by all the pipeline stages in
+    /// `stages`, and no others.  Both `offset` and the length of `data` must be
+    /// multiples of [`PUSH_CONSTANT_ALIGNMENT`], which is always 4.
     ///
-    /// Data size must be a multiple of 4 and must have an alignment of 4.
-    /// For example, with an offset of 4 and an array of `[u8; 8]`, that will write to the range
-    /// of 4..12.
+    /// For example, if `offset` is `4` and `data` is eight bytes long, this
+    /// call will write `data` to bytes `4..12` of push constant storage.
     ///
-    /// For each byte in the range of push constant data written, the union of the stages of all push constant
-    /// ranges that covers that byte must be exactly `stages`. There's no good way of explaining this simply,
-    /// so here are some examples:
+    /// # Stage matching
     ///
-    /// ```text
-    /// For the given ranges:
-    /// - 0..4 Vertex
-    /// - 4..8 Fragment
-    /// ```
+    /// Every byte in the affected range of push constant storage must be
+    /// accessible to exactly the same set of pipeline stages, which must match
+    /// `stages`. If there are two bytes of storage that are accessible by
+    /// different sets of pipeline stages - say, one is accessible by fragment
+    /// shaders, and the other is accessible by both fragment shaders and vertex
+    /// shaders - then no single `set_push_constants` call may affect both of
+    /// them; to write both, you must make multiple calls, each with the
+    /// appropriate `stages` value.
     ///
-    /// You would need to upload this in two set_push_constants calls. First for the `Vertex` range, second for the `Fragment` range.
+    /// Which pipeline stages may access a given byte is determined by the
+    /// pipeline's [`PushConstant`] global variable and (if it is a struct) its
+    /// members' offsets.
     ///
-    /// ```text
-    /// For the given ranges:
-    /// - 0..8  Vertex
-    /// - 4..12 Fragment
-    /// ```
+    /// For example, suppose you have twelve bytes of push constant storage,
+    /// where bytes `0..8` are accessed by the vertex shader, and bytes `4..12`
+    /// are accessed by the fragment shader. This means there are three byte
+    /// ranges each accessed by a different set of stages:
     ///
-    /// You would need to upload this in three set_push_constants calls. First for the `Vertex` only range 0..4, second
-    /// for the `Vertex | Fragment` range 4..8, third for the `Fragment` range 8..12.
+    /// - Bytes `0..4` are accessed only by the fragment shader.
+    ///
+    /// - Bytes `4..8` are accessed by both the fragment shader and the vertex shader.
+    ///
+    /// - Bytes `8..12 are accessed only by the vertex shader.
+    ///
+    /// To write all twelve bytes requires three `set_push_constants` calls, one
+    /// for each range, each passing the matching `stages` mask.
+    ///
+    /// [`PushConstant`]: https://docs.rs/naga/latest/naga/enum.StorageClass.html#variant.PushConstant
     pub fn set_push_constants(&mut self, stages: ShaderStages, offset: u32, data: &[u8]) {
         self.id.set_push_constants(stages, offset, data);
     }
@@ -3151,13 +3163,14 @@ impl<'a> ComputePass<'a> {
 
 /// [`Features::PUSH_CONSTANTS`] must be enabled on the device in order to call these functions.
 impl<'a> ComputePass<'a> {
-    /// Set push constant data.
+    /// Set push constant data for subsequent dispatch calls.
     ///
-    /// Offset is measured in bytes, but must be a multiple of [`PUSH_CONSTANT_ALIGNMENT`].
+    /// Write the bytes in `data` at offset `offset` within push constant
+    /// storage.  Both `offset` and the length of `data` must be
+    /// multiples of [`PUSH_CONSTANT_ALIGNMENT`], which is always 4.
     ///
-    /// Data size must be a multiple of 4 and must have an alignment of 4.
-    /// For example, with an offset of 4 and an array of `[u8; 8]`, that will write to the range
-    /// of 4..12.
+    /// For example, if `offset` is `4` and `data` is eight bytes long, this
+    /// call will write `data` to bytes `4..12` of push constant storage.
     pub fn set_push_constants(&mut self, offset: u32, data: &[u8]) {
         self.id.set_push_constants(offset, data);
     }