Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the new checks for readonly stencils #3443

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

JCapucho
Copy link
Collaborator

Checklist

  • Run cargo clippy. (clippy is complaining about other parts of the codebase)
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
None that I know of

Description
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 PR aims to align the wgpu rules with the webgpu specification.

Testing
This changes was tested by using this patch and checking that the validation passes

diff --git a/wgpu/examples/shadow/main.rs b/wgpu/examples/shadow/main.rs
index 3b1f10d4..e2487835 100644
--- a/wgpu/examples/shadow/main.rs
+++ b/wgpu/examples/shadow/main.rs
@@ -519,7 +519,12 @@ impl framework::Example for Example {
                     format: Self::SHADOW_FORMAT,
                     depth_write_enabled: true,
                     depth_compare: wgpu::CompareFunction::LessEqual,
-                    stencil: wgpu::StencilState::default(),
+                    stencil: wgpu::StencilState {
+                        front: wgpu::StencilFaceState::IGNORE,
+                        back: wgpu::StencilFaceState::IGNORE,
+                        read_mask: !0,
+                        write_mask: !0,
+                    },
                     bias: wgpu::DepthBiasState {
                         constant: 2, // corresponds to bilinear filtering
                         slope_scale: 2.0,

and running some more complex applications that had the same problem.

@teoxoy
Copy link
Member

teoxoy commented Feb 1, 2023

Relevant spec section (for reference)

image

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, needs a changelog entry though.

@JCapucho
Copy link
Collaborator Author

JCapucho commented Feb 1, 2023

LGTM, needs a changelog entry though.

@teoxoy would this be considered a change or a bug fix?

@teoxoy
Copy link
Member

teoxoy commented Feb 1, 2023

would this be considered a change or a bug fix?

Could be either, I guess it depends which way you look at it. I usually use "change" if nobody opened an issue for it.

@teoxoy teoxoy added this to the WebGPU Specification V1 milestone Feb 1, 2023
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.
@JCapucho
Copy link
Collaborator Author

JCapucho commented Feb 1, 2023

@teoxoy I've added the changelog entry and also added a little comment referencing the section of the specification.

@teoxoy teoxoy merged commit c371e70 into gfx-rs:master Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants