[core] Support optional ops in PassChannel (gfx-rs#6716)

* Support optional ops in PassChannel Signed-off-by: sagudev <[email protected]> * respect view format aspect Signed-off-by: sagudev <[email protected]> --------- Signed-off-by: sagudev <[email protected]>
sagudev · Dec 16, 2024 · 4da7c26 · 4da7c26
1 parent 411ffa7
commit 4da7c26
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 34 deletions.
diff --git a/deno_webgpu/command_encoder.rs b/deno_webgpu/command_encoder.rs
@@ -155,22 +155,14 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
             Some(wgpu_core::command::RenderPassDepthStencilAttachment {
                 view: texture_view_resource.1,
                 depth: wgpu_core::command::PassChannel {
-                    load_op: attachment
-                        .depth_load_op
-                        .unwrap_or(wgpu_core::command::LoadOp::Load),
-                    store_op: attachment
-                        .depth_store_op
-                        .unwrap_or(wgpu_core::command::StoreOp::Store),
+                    load_op: attachment.depth_load_op,
+                    store_op: attachment.depth_store_op,
                     clear_value: attachment.depth_clear_value,
                     read_only: attachment.depth_read_only,
                 },
                 stencil: wgpu_core::command::PassChannel {
-                    load_op: attachment
-                        .stencil_load_op
-                        .unwrap_or(wgpu_core::command::LoadOp::Load),
-                    store_op: attachment
-                        .stencil_store_op
-                        .unwrap_or(wgpu_core::command::StoreOp::Store),
+                    load_op: attachment.stencil_load_op,
+                    store_op: attachment.stencil_store_op,
                     clear_value: attachment.stencil_clear_value,
                     read_only: attachment.stencil_read_only,
                 },

diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs
@@ -676,6 +676,8 @@ pub enum CommandEncoderError {
     #[error(transparent)]
     InvalidColorAttachment(#[from] ColorAttachmentError),
     #[error(transparent)]
+    InvalidAttachment(#[from] AttachmentError),
+    #[error(transparent)]
     InvalidResource(#[from] InvalidResourceError),
     #[error(transparent)]
     MissingFeatures(#[from] MissingFeatures),

diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs
@@ -102,15 +102,15 @@ impl StoreOp {
 #[repr(C)]
 #[derive(Clone, Debug, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct PassChannel<V> {
+pub struct PassChannel<V, L = Option<LoadOp>, S = Option<StoreOp>> {
     /// Operation to perform to the output attachment at the start of a
     /// renderpass.
     ///
     /// This must be clear if it is the first renderpass rendering to a swap
     /// chain image.
-    pub load_op: LoadOp,
+    pub load_op: L,
     /// Operation to perform to the output attachment at the end of a renderpass.
-    pub store_op: StoreOp,
+    pub store_op: S,
     /// If load_op is [`LoadOp::Clear`], the attachment will be cleared to this
     /// color.
     pub clear_value: V,
@@ -120,12 +120,52 @@ pub struct PassChannel<V> {
     pub read_only: bool,
 }
 
-impl<V> PassChannel<V> {
+impl<V> PassChannel<V, LoadOp, StoreOp> {
     fn hal_ops(&self) -> hal::AttachmentOps {
         self.load_op.hal_ops() | self.store_op.hal_ops()
     }
 }
 
+impl<V: Default> Default for PassChannel<V, LoadOp, StoreOp> {
+    fn default() -> Self {
+        PassChannel {
+            load_op: LoadOp::Load,
+            store_op: StoreOp::Store,
+            clear_value: V::default(),
+            read_only: false,
+        }
+    }
+}
+
+impl<V: Copy> PassChannel<V, Option<LoadOp>, Option<StoreOp>> {
+    fn resolve(&self) -> Result<PassChannel<V, LoadOp, StoreOp>, AttachmentError> {
+        let load_op = if self.read_only {
+            if self.load_op.is_some() {
+                return Err(AttachmentError::ReadOnlyWithLoad);
+            } else {
+                LoadOp::Load
+            }
+        } else {
+            self.load_op.ok_or(AttachmentError::NoLoad)?
+        };
+        let store_op = if self.read_only {
+            if self.store_op.is_some() {
+                return Err(AttachmentError::ReadOnlyWithStore);
+            } else {
+                StoreOp::Store
+            }
+        } else {
+            self.store_op.ok_or(AttachmentError::NoStore)?
+        };
+        Ok(PassChannel {
+            load_op,
+            store_op,
+            clear_value: self.clear_value,
+            read_only: self.read_only,
+        })
+    }
+}
+
 /// Describes a color attachment to a render pass.
 #[repr(C)]
 #[derive(Clone, Debug, PartialEq)]
@@ -176,19 +216,19 @@ pub struct RenderPassDepthStencilAttachment {
     /// The view to use as an attachment.
     pub view: id::TextureViewId,
     /// What operations will be performed on the depth part of the attachment.
-    pub depth: PassChannel<f32>,
+    pub depth: PassChannel<f32, Option<LoadOp>, Option<StoreOp>>,
     /// What operations will be performed on the stencil part of the attachment.
-    pub stencil: PassChannel<u32>,
+    pub stencil: PassChannel<u32, Option<LoadOp>, Option<StoreOp>>,
 }
 /// Describes a depth/stencil attachment to a render pass.
 #[derive(Debug)]
 pub struct ArcRenderPassDepthStencilAttachment {
     /// The view to use as an attachment.
     pub view: Arc<TextureView>,
     /// What operations will be performed on the depth part of the attachment.
-    pub depth: PassChannel<f32>,
+    pub depth: PassChannel<f32, LoadOp, StoreOp>,
     /// What operations will be performed on the stencil part of the attachment.
-    pub stencil: PassChannel<u32>,
+    pub stencil: PassChannel<u32, LoadOp, StoreOp>,
 }
 
 impl ArcRenderPassDepthStencilAttachment {
@@ -596,6 +636,21 @@ pub enum ColorAttachmentError {
     TooManyBytesPerSample { total: u32, limit: u32 },
 }
 
+#[derive(Clone, Debug, Error)]
+#[non_exhaustive]
+pub enum AttachmentError {
+    #[error("The format of the depth-stencil attachment ({0:?}) is not a depth-or-stencil format")]
+    InvalidDepthStencilAttachmentFormat(wgt::TextureFormat),
+    #[error("Read-only attachment with load")]
+    ReadOnlyWithLoad,
+    #[error("Read-only attachment with store")]
+    ReadOnlyWithStore,
+    #[error("Attachment without load")]
+    NoLoad,
+    #[error("Attachment without store")]
+    NoStore,
+}
+
 /// Error encountered when performing a render pass.
 #[derive(Clone, Debug, Error)]
 pub enum RenderPassErrorInner {
@@ -607,8 +662,6 @@ pub enum RenderPassErrorInner {
     Encoder(#[from] CommandEncoderError),
     #[error("Parent encoder is invalid")]
     InvalidParentEncoder,
-    #[error("The format of the depth-stencil attachment ({0:?}) is not a depth-stencil format")]
-    InvalidDepthStencilAttachmentFormat(wgt::TextureFormat),
     #[error("The format of the {location} ({format:?}) is not resolvable")]
     UnsupportedResolveTargetFormat {
         location: AttachmentErrorLocation,
@@ -935,16 +988,10 @@ impl<'d> RenderPassInfo<'d> {
 
         if let Some(at) = depth_stencil_attachment.as_ref() {
             let view = &at.view;
-            view.same_device(device)?;
             check_multiview(view)?;
             add_view(view, AttachmentErrorLocation::Depth)?;
 
             let ds_aspects = view.desc.aspects();
-            if ds_aspects.contains(hal::FormatAspects::COLOR) {
-                return Err(RenderPassErrorInner::InvalidDepthStencilAttachmentFormat(
-                    view.desc.format,
-                ));
-            }
 
             if !ds_aspects.contains(hal::FormatAspects::STENCIL)
                 || (at.stencil.load_op == at.depth.load_op
@@ -1414,14 +1461,30 @@ impl Global {
             }
 
             arc_desc.depth_stencil_attachment =
+            // https://gpuweb.github.io/gpuweb/#abstract-opdef-gpurenderpassdepthstencilattachment-gpurenderpassdepthstencilattachment-valid-usage
                 if let Some(depth_stencil_attachment) = desc.depth_stencil_attachment {
                     let view = texture_views.get(depth_stencil_attachment.view).get()?;
                     view.same_device(device)?;
 
+                    let format = view.desc.format;
+                    if !format.is_depth_stencil_format() {
+                        return Err(CommandEncoderError::InvalidAttachment(AttachmentError::InvalidDepthStencilAttachmentFormat(
+                            view.desc.format,
+                        )));
+                    }
+
                     Some(ArcRenderPassDepthStencilAttachment {
                         view,
-                        depth: depth_stencil_attachment.depth.clone(),
-                        stencil: depth_stencil_attachment.stencil.clone(),
+                        depth: if format.has_depth_aspect() {
+                            depth_stencil_attachment.depth.resolve()?
+                        } else {
+                            Default::default()
+                        },
+                        stencil: if format.has_stencil_aspect() {
+                            depth_stencil_attachment.stencil.resolve()?
+                        } else {
+                            Default::default()
+                        },
                     })
                 } else {
                     None

diff --git a/wgpu/src/backend/wgpu_core.rs b/wgpu/src/backend/wgpu_core.rs
@@ -418,15 +418,15 @@ fn map_pass_channel<V: Copy + Default>(
         Some(&Operations { load, store }) => {
             let (load_op, clear_value) = map_load_op(load);
             wgc::command::PassChannel {
-                load_op,
-                store_op: map_store_op(store),
+                load_op: Some(load_op),
+                store_op: Some(map_store_op(store)),
                 clear_value,
                 read_only: false,
             }
         }
         None => wgc::command::PassChannel {
-            load_op: wgc::command::LoadOp::Load,
-            store_op: wgc::command::StoreOp::Store,
+            load_op: None,
+            store_op: None,
             clear_value: V::default(),
             read_only: true,
         },