diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 6121ab8f9f..625aa144b8 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -122,7 +122,8 @@ pub struct RenderBundleEncoder { base: BasePass, 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>, ) -> Result { + 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 { // Normalized command stream. It can be executed verbatim, // without re-binding anything on the pipeline change. base: BasePass, - 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, pub(crate) used: RenderBundleScope, pub(super) buffer_memory_init_actions: Vec, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 9443bea895..5ce6fb3a66 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -131,20 +131,40 @@ pub struct RenderPassDepthStencilAttachment { } impl RenderPassDepthStencilAttachment { - fn is_read_only(&self, aspects: hal::FormatAspects) -> Result { - 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, render_attachments: AttachmentDataVec>, // 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, @@ -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::::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 Global { 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 Global { .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); } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 8a9eb0fbbe..a590903bbb 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -2765,8 +2765,11 @@ impl Device { 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 Global { 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(), }); diff --git a/wgpu-core/src/device/trace.rs b/wgpu-core/src/device/trace.rs index 3e0c791829..636a23de20 100644 --- a/wgpu-core/src/device/trace.rs +++ b/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, diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 34a57efcd0..21b99b6547 100644 --- a/wgpu-core/src/pipeline.rs +++ b/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; } } diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 82c3edcf38..0ba849e3e6 100644 --- a/wgpu-types/src/lib.rs +++ b/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() } }