Skip to content

Commit

Permalink
Implement the new checks for readonly stencils (#3443)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JCapucho authored Feb 1, 2023
1 parent 48d8666 commit c371e70
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/vulkan/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 24 additions & 6 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit c371e70

Please sign in to comment.