From c371e7039dac763b08ada0a35f6c11cd71052010 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= <jcapucho7@gmail.com>
Date: Wed, 1 Feb 2023 20:13:46 +0000
Subject: [PATCH] Implement the new checks for readonly stencils (#3443)

wgpu currently checks if the `write_mask` is 0 to determine wether a
stencil is used as readonly or not. However Webgpu contains a more
complex ruleset that also checks the cull mode and face operations to
determine if the stencil is readonly or not.

This commit brings these new rules to wgpu.
---
 CHANGELOG.md                  |  1 +
 wgpu-core/src/device/mod.rs   |  2 +-
 wgpu-hal/src/vulkan/device.rs |  2 +-
 wgpu-types/src/lib.rs         | 30 ++++++++++++++++++++++++------
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d2a0dfb659..d8f9e30845 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,7 @@ Bottom level categories:
 #### WebGPU
 
 - Implement `CommandEncoder::clear_buffer`. By @raphlinus in [#3426](https://github.com/gfx-rs/wgpu/pull/3426)
+- Implement the new checks for readonly stencils. By @JCapucho in [#3443](https://github.com/gfx-rs/wgpu/pull/3443)
 
 #### Vulkan
 
diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs
index f894efcbd0..0e2d31e887 100644
--- a/wgpu-core/src/device/mod.rs
+++ b/wgpu-core/src/device/mod.rs
@@ -3072,7 +3072,7 @@ impl<A: HalApi> Device<A> {
             if !ds.is_depth_read_only() {
                 flags |= pipeline::PipelineFlags::WRITES_DEPTH;
             }
-            if !ds.is_stencil_read_only() {
+            if !ds.is_stencil_read_only(desc.primitive.cull_mode) {
                 flags |= pipeline::PipelineFlags::WRITES_STENCIL;
             }
         }
diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs
index e7c3856e7e..3d4ab018e9 100644
--- a/wgpu-hal/src/vulkan/device.rs
+++ b/wgpu-hal/src/vulkan/device.rs
@@ -1607,7 +1607,7 @@ impl crate::Device<super::Api> for super::Device {
         let mut vk_depth_stencil = vk::PipelineDepthStencilStateCreateInfo::builder();
         if let Some(ref ds) = desc.depth_stencil {
             let vk_format = self.shared.private_caps.map_texture_format(ds.format);
-            let vk_layout = if ds.is_read_only() {
+            let vk_layout = if ds.is_read_only(desc.primitive.cull_mode) {
                 vk::ImageLayout::DEPTH_STENCIL_READ_ONLY_OPTIMAL
             } else {
                 vk::ImageLayout::DEPTH_STENCIL_ATTACHMENT_OPTIMAL
diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs
index ac9df674fe..8307ad564a 100644
--- a/wgpu-types/src/lib.rs
+++ b/wgpu-types/src/lib.rs
@@ -3289,8 +3289,19 @@ impl StencilState {
             && (self.read_mask != 0 || self.write_mask != 0)
     }
     /// Returns true if the state doesn't mutate the target values.
-    pub fn is_read_only(&self) -> bool {
-        self.write_mask == 0
+    pub fn is_read_only(&self, cull_mode: Option<Face>) -> bool {
+        // The rules are defined in step 7 of the "Device timeline initialization steps"
+        // subsection of the "Render Pipeline Creation" section of WebGPU
+        // (link to the section: https://gpuweb.github.io/gpuweb/#render-pipeline-creation)
+
+        if self.write_mask == 0 {
+            return true;
+        }
+
+        let front_ro = cull_mode == Some(Face::Front) || self.front.is_read_only();
+        let back_ro = cull_mode == Some(Face::Back) || self.back.is_read_only();
+
+        front_ro && back_ro
     }
     /// Returns true if the stencil state uses the reference value for testing.
     pub fn needs_ref_value(&self) -> bool {
@@ -3380,13 +3391,13 @@ impl DepthStencilState {
     }
 
     /// Returns true if the state doesn't mutate the stencil.
-    pub fn is_stencil_read_only(&self) -> bool {
-        self.stencil.is_read_only()
+    pub fn is_stencil_read_only(&self, cull_mode: Option<Face>) -> bool {
+        self.stencil.is_read_only(cull_mode)
     }
 
     /// Returns true if the state doesn't mutate either depth or stencil of the target.
-    pub fn is_read_only(&self) -> bool {
-        self.is_depth_read_only() && self.is_stencil_read_only()
+    pub fn is_read_only(&self, cull_mode: Option<Face>) -> bool {
+        self.is_depth_read_only() && self.is_stencil_read_only(cull_mode)
     }
 }
 
@@ -3476,6 +3487,13 @@ impl StencilFaceState {
             || self.depth_fail_op == StencilOperation::Replace
             || self.pass_op == StencilOperation::Replace
     }
+
+    /// Returns true if the face state doesn't mutate the target values.
+    pub fn is_read_only(&self) -> bool {
+        self.pass_op == StencilOperation::Keep
+            && self.depth_fail_op == StencilOperation::Keep
+            && self.fail_op == StencilOperation::Keep
+    }
 }
 
 impl Default for StencilFaceState {