Track depth and stencil writability separately.

wgpu-core/src/command/bundle.rs

@@ -122,7 +122,8 @@ pub struct RenderBundleEncoder {
     base: BasePass<RenderCommand>,
     parent_id: id::DeviceId,
     pub(crate) context: RenderPassContext,
-    pub(crate) is_ds_read_only: bool,
+    pub(crate) is_depth_read_only: bool,
+    pub(crate) is_stencil_read_only: bool,
 
     // Resource binding dedupe state.
     #[cfg_attr(feature = "serial-pass", serde(skip))]
@@ -137,6 +138,20 @@ impl RenderBundleEncoder {
         parent_id: id::DeviceId,
         base: Option<BasePass<RenderCommand>>,
     ) -> Result<Self, CreateRenderBundleError> {
+        let (is_depth_read_only, is_stencil_read_only) = match desc.depth_stencil {
+            Some(ds) => {
+                let aspects = hal::FormatAspects::from(ds.format);
+                (
+                    !aspects.contains(hal::FormatAspects::DEPTH) || ds.depth_read_only,
+                    !aspects.contains(hal::FormatAspects::STENCIL) || ds.stencil_read_only,
+                )
+            }
+            // There's no depth/stencil attachment, so these values just don't
+            // matter.  Choose the most accommodating value, to simplify
+            // validation.
+            None => (true, true),
+        };
+
         //TODO: validate that attachment formats are renderable,
         // have expected aspects, support multisampling.
         Ok(Self {
@@ -161,15 +176,9 @@ impl RenderBundleEncoder {
                 },
                 multiview: desc.multiview,
             },
-            is_ds_read_only: match desc.depth_stencil {
-                Some(ds) => {
-                    let aspects = hal::FormatAspects::from(ds.format);
-                    (!aspects.contains(hal::FormatAspects::DEPTH) || ds.depth_read_only)
-                        && (!aspects.contains(hal::FormatAspects::STENCIL) || ds.stencil_read_only)
-                }
-                None => false,
-            },
 
+            is_depth_read_only,
+            is_stencil_read_only,
             current_bind_groups: BindGroupStateChange::new(),
             current_pipeline: StateChange::new(),
         })
@@ -188,7 +197,8 @@ impl RenderBundleEncoder {
                 sample_count: 0,
                 multiview: None,
             },
-            is_ds_read_only: false,
+            is_depth_read_only: false,
+            is_stencil_read_only: false,
 
             current_bind_groups: BindGroupStateChange::new(),
             current_pipeline: StateChange::new(),
@@ -344,8 +354,10 @@ impl RenderBundleEncoder {
                         .map_err(RenderCommandError::IncompatiblePipelineTargets)
                         .map_pass_err(scope)?;
 
-                    if pipeline.flags.contains(PipelineFlags::WRITES_DEPTH_STENCIL)
-                        && self.is_ds_read_only
+                    if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH)
+                        && self.is_depth_read_only)
+                        || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL)
+                            && self.is_stencil_read_only)
                     {
                         return Err(RenderCommandError::IncompatiblePipelineRods)
                             .map_pass_err(scope);
@@ -608,7 +620,8 @@ impl RenderBundleEncoder {
                 string_data: Vec::new(),
                 push_constant_data: Vec::new(),
             },
-            is_ds_read_only: self.is_ds_read_only,
+            is_depth_read_only: self.is_depth_read_only,
+            is_stencil_read_only: self.is_stencil_read_only,
             device_id: Stored {
                 value: id::Valid(self.parent_id),
                 ref_count: device.life_guard.add_ref(),
@@ -686,7 +699,8 @@ pub struct RenderBundle<A: HalApi> {
     // Normalized command stream. It can be executed verbatim,
     // without re-binding anything on the pipeline change.
     base: BasePass<RenderCommand>,
-    pub(super) is_ds_read_only: bool,
+    pub(super) is_depth_read_only: bool,
+    pub(super) is_stencil_read_only: bool,
     pub(crate) device_id: Stored<id::DeviceId>,
     pub(crate) used: RenderBundleScope<A>,
     pub(super) buffer_memory_init_actions: Vec<BufferInitTrackerAction>,

wgpu-core/src/command/render.rs

@@ -131,20 +131,40 @@ pub struct RenderPassDepthStencilAttachment {
 }
 
 impl RenderPassDepthStencilAttachment {
-    fn is_read_only(&self, aspects: hal::FormatAspects) -> Result<bool, RenderPassErrorInner> {
-        if aspects.contains(hal::FormatAspects::DEPTH) && !self.depth.read_only {
-            return Ok(false);
-        }
-        if (self.depth.load_op, self.depth.store_op) != (LoadOp::Load, StoreOp::Store) {
-            return Err(RenderPassErrorInner::InvalidDepthOps);
-        }
-        if aspects.contains(hal::FormatAspects::STENCIL) && !self.stencil.read_only {
-            return Ok(false);
+    /// Validate the given aspects' read-only flags against their load
+    /// and store ops.
+    ///
+    /// When an aspect is read-only, its load and store ops must be
+    /// `LoadOp::Load` and `StoreOp::Store`.
+    ///
+    /// On success, return a pair `(depth, stencil)` indicating
+    /// whether the depth and stencil passes are read-only.
+    fn depth_stencil_read_only(
+        &self,
+        aspects: hal::FormatAspects,
+    ) -> Result<(bool, bool), RenderPassErrorInner> {
+        let mut depth_read_only = true;
+        let mut stencil_read_only = true;
+
+        if aspects.contains(hal::FormatAspects::DEPTH) {
+            if self.depth.read_only
+                && (self.depth.load_op, self.depth.store_op) != (LoadOp::Load, StoreOp::Store)
+            {
+                return Err(RenderPassErrorInner::InvalidDepthOps);
+            }
+            depth_read_only = self.depth.read_only;
         }
-        if (self.stencil.load_op, self.stencil.store_op) != (LoadOp::Load, StoreOp::Store) {
-            return Err(RenderPassErrorInner::InvalidStencilOps);
+
+        if aspects.contains(hal::FormatAspects::STENCIL) {
+            if self.stencil.read_only
+                && (self.stencil.load_op, self.stencil.store_op) != (LoadOp::Load, StoreOp::Store)
+            {
+                return Err(RenderPassErrorInner::InvalidStencilOps);
+            }
+            stencil_read_only = self.stencil.read_only;
         }
-        Ok(true)
+
+        Ok((depth_read_only, stencil_read_only))
     }
 }
 
@@ -474,8 +494,18 @@ pub enum RenderPassErrorInner {
     ResourceUsageConflict(#[from] UsageConflict),
     #[error("render bundle has incompatible targets, {0}")]
     IncompatibleBundleTargets(#[from] RenderPassCompatibilityError),
-    #[error("render bundle has an incompatible read-only depth/stencil flag: bundle is {bundle}, while the pass is {pass}")]
-    IncompatibleBundleRods { pass: bool, bundle: bool },
+    #[error(
+        "render bundle has incompatible read-only flags: \
+             bundle has flags depth = {bundle_depth} and stencil = {bundle_stencil}, \
+             while the pass has flags depth = {pass_depth} and stencil = {pass_stencil}. \
+             Read-only renderpasses are only compatible with read-only bundles for that aspect."
+    )]
+    IncompatibleBundleRods {
+        pass_depth: bool,
+        pass_stencil: bool,
+        bundle_depth: bool,
+        bundle_stencil: bool,
+    },
     #[error(transparent)]
     RenderCommand(#[from] RenderCommandError),
     #[error(transparent)]
@@ -565,7 +595,8 @@ struct RenderPassInfo<'a, A: HalApi> {
     context: RenderPassContext,
     usage_scope: UsageScope<A>,
     render_attachments: AttachmentDataVec<RenderAttachment<'a>>, // All render attachments, including depth/stencil
-    is_ds_read_only: bool,
+    is_depth_read_only: bool,
+    is_stencil_read_only: bool,
     extent: wgt::Extent3d,
     _phantom: PhantomData<A>,
 
@@ -626,7 +657,8 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
         // We default to false intentionally, even if depth-stencil isn't used at all.
         // This allows us to use the primary raw pipeline in `RenderPipeline`,
         // instead of the special read-only one, which would be `None`.
-        let mut is_ds_read_only = false;
+        let mut is_depth_read_only = false;
+        let mut is_stencil_read_only = false;
 
         let mut render_attachments = AttachmentDataVec::<RenderAttachment>::new();
         let mut discarded_surfaces = AttachmentDataVec::new();
@@ -786,8 +818,9 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
                 }
             }
 
-            let usage = if at.is_read_only(ds_aspects)? {
-                is_ds_read_only = true;
+            (is_depth_read_only, is_stencil_read_only) = at.depth_stencil_read_only(ds_aspects)?;
+
+            let usage = if is_depth_read_only && is_stencil_read_only {
                 hal::TextureUses::DEPTH_STENCIL_READ | hal::TextureUses::RESOURCE
             } else {
                 hal::TextureUses::DEPTH_STENCIL_WRITE
@@ -937,7 +970,8 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
             context,
             usage_scope: UsageScope::new(buffer_guard, texture_guard),
             render_attachments,
-            is_ds_read_only,
+            is_depth_read_only,
+            is_stencil_read_only,
             extent,
             _phantom: PhantomData,
             pending_discard_init_fixups,
@@ -1234,8 +1268,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
                         state.pipeline_flags = pipeline.flags;
 
-                        if pipeline.flags.contains(PipelineFlags::WRITES_DEPTH_STENCIL)
-                            && info.is_ds_read_only
+                        if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH)
+                            && info.is_depth_read_only)
+                            || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL)
+                                && info.is_stencil_read_only)
                         {
                             return Err(RenderCommandError::IncompatiblePipelineRods)
                                 .map_pass_err(scope);
@@ -1890,10 +1926,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                             .map_err(RenderPassErrorInner::IncompatibleBundleTargets)
                             .map_pass_err(scope)?;
 
-                        if info.is_ds_read_only != bundle.is_ds_read_only {
+                        if (info.is_depth_read_only && !bundle.is_depth_read_only)
+                            || (info.is_stencil_read_only && !bundle.is_stencil_read_only)
+                        {
                             return Err(RenderPassErrorInner::IncompatibleBundleRods {
-                                pass: info.is_ds_read_only,
-                                bundle: bundle.is_ds_read_only,
+                                pass_depth: info.is_depth_read_only,
+                                pass_stencil: info.is_stencil_read_only,
+                                bundle_depth: bundle.is_depth_read_only,
+                                bundle_stencil: bundle.is_stencil_read_only,
                             })
                             .map_pass_err(scope);
                         }

wgpu-core/src/device/mod.rs

@@ -2765,8 +2765,11 @@ impl<A: HalApi> Device<A> {
             if ds.stencil.is_enabled() && ds.stencil.needs_ref_value() {
                 flags |= pipeline::PipelineFlags::STENCIL_REFERENCE;
             }
-            if !ds.is_read_only() {
-                flags |= pipeline::PipelineFlags::WRITES_DEPTH_STENCIL;
+            if !ds.is_depth_read_only() {
+                flags |= pipeline::PipelineFlags::WRITES_DEPTH;
+            }
+            if !ds.is_stencil_read_only() {
+                flags |= pipeline::PipelineFlags::WRITES_STENCIL;
             }
         }
 
@@ -4378,7 +4381,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                     desc: trace::new_render_bundle_encoder_descriptor(
                         desc.label.clone(),
                         &bundle_encoder.context,
-                        bundle_encoder.is_ds_read_only,
+                        bundle_encoder.is_depth_read_only,
+                        bundle_encoder.is_stencil_read_only,
                     ),
                     base: bundle_encoder.to_base_pass(),
                 });

wgpu-core/src/device/trace.rs

@@ -13,17 +13,17 @@ pub const FILE_NAME: &str = "trace.ron";
 pub(crate) fn new_render_bundle_encoder_descriptor<'a>(
     label: crate::Label<'a>,
     context: &'a super::RenderPassContext,
-    is_ds_read_only: bool,
+    depth_read_only: bool,
+    stencil_read_only: bool,
 ) -> crate::command::RenderBundleEncoderDescriptor<'a> {
     crate::command::RenderBundleEncoderDescriptor {
         label,
         color_formats: Cow::Borrowed(&context.attachments.colors),
         depth_stencil: context.attachments.depth_stencil.map(|format| {
-            let aspects = hal::FormatAspects::from(format);
             wgt::RenderBundleDepthStencil {
                 format,
-                depth_read_only: is_ds_read_only && aspects.contains(hal::FormatAspects::DEPTH),
-                stencil_read_only: is_ds_read_only && aspects.contains(hal::FormatAspects::STENCIL),
+                depth_read_only,
+                stencil_read_only,
             }
         }),
         sample_count: context.sample_count,

wgpu-core/src/pipeline.rs

@@ -352,7 +352,8 @@ bitflags::bitflags! {
     pub struct PipelineFlags: u32 {
         const BLEND_CONSTANT = 1 << 0;
         const STENCIL_REFERENCE = 1 << 1;
-        const WRITES_DEPTH_STENCIL = 1 << 2;
+        const WRITES_DEPTH = 1 << 2;
+        const WRITES_STENCIL = 1 << 3;
     }
 }
 

wgpu-types/src/lib.rs

@@ -2419,9 +2419,20 @@ impl DepthStencilState {
     pub fn is_depth_enabled(&self) -> bool {
         self.depth_compare != CompareFunction::Always || self.depth_write_enabled
     }
+
+    /// Returns true if the state doesn't mutate the depth buffer.
+    pub fn is_depth_read_only(&self) -> bool {
+        !self.depth_write_enabled
+    }
+
+    /// Returns true if the state doesn't mutate the stencil.
+    pub fn is_stencil_read_only(&self) -> bool {
+        self.stencil.is_read_only()
+    }
+
     /// Returns true if the state doesn't mutate either depth or stencil of the target.
     pub fn is_read_only(&self) -> bool {
-        !self.depth_write_enabled && self.stencil.is_read_only()
+        self.is_depth_read_only() && self.is_stencil_read_only()
     }
 }