Skip to content

DepthStencilState validation fixes #8840

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.

Sign up for GitHub

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

Open
andyleiserson wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

DepthStencilState validation fixes #8840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,40 @@ Bottom level categories:

## Unreleased

### Major Changes

#### Depth/stencil state changes

BREAKING CHANGE: Pipeline descriptors now use `DepthStencilStateIdl` instead of `DepthStencilState`.
The new struct makes the `depth_write_enabled` and `depth_compare` fields optional, to match WebGPU.

The simplest way to adapt to this change is to use `.into()` to convert to the new struct:

```diff
-depth_stencil: Some(wgpu::DepthStencilState {
- format: wgpu::TextureFormat::Depth32Float,
- depth_write_enabled: true,
- depth_compare: wgpu::CompareFunction::Less,
- stencil: wgpu::StencilState::default(),
- bias: wgpu::DepthBiasState::default(),
-}),
+depth_stencil: Some(wgpu::DepthStencilState {
+ format: wgpu::TextureFormat::Depth32Float,
+ depth_write_enabled: true,
+ depth_compare: wgpu::CompareFunction::Less,
+ stencil: wgpu::StencilState::default(),
+ bias: wgpu::DepthBiasState::default(),
+}.into()),
```

There are also new constructors which may be used instead of a struct literal:
`DepthStencilState::new` (for simultaneous depth and stencil operations),
`DepthStencilState::depth` (for depth operations), and `DepthStencilState::stencil` (for stencil operations).

`DepthStencilState` may be updated to match `DepthStencilStateIdl` in the future.
In anticipation of this, the constructors are associated methods on
`DepthStencilState` even though they construct `DepthStencilStateIdl`.

### New Features

- Added support for cooperative load/store operations in shaders. Currently only WGSL on the input and SPIR-V, METAL, and WGSL on the output are supported. By @kvark in [#8251](https://github.com/gfx-rs/wgpu/issues/8251).
Expand Down
2 changes: 2 additions & 0 deletions cts_runner/test.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ webgpu:api,validation,render_pass,render_pass_descriptor:attachments,*
webgpu:api,validation,render_pass,render_pass_descriptor:color_attachments,*
webgpu:api,validation,render_pass,render_pass_descriptor:resolveTarget,*
webgpu:api,validation,render_pass,resolve:resolve_attachment:*
webgpu:api,validation,render_pipeline,depth_stencil_state:depthCompare_optional:*
webgpu:api,validation,render_pipeline,depth_stencil_state:depthWriteEnabled_optional:*
webgpu:api,validation,render_pipeline,depth_stencil_state:format:*
webgpu:api,validation,render_pipeline,depth_stencil_state:stencil_write:*
webgpu:api,validation,render_pipeline,inter_stage:max_shader_variable_location:isAsync=false;*
Expand Down
11 changes: 3 additions & 8 deletions deno_webgpu/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,15 +881,10 @@ impl GPUDevice {
pass_op: depth_stencil.stencil_back.pass_op.into(),
};

wgpu_types::DepthStencilState {
wgpu_types::DepthStencilStateIdl {
format: depth_stencil.format.into(),
depth_write_enabled: depth_stencil
.depth_write_enabled
.unwrap_or_default(),
depth_compare: depth_stencil
.depth_compare
.map(Into::into)
.unwrap_or(wgpu_types::CompareFunction::Never), // TODO(wgpu): should be optional here
depth_write_enabled: depth_stencil.depth_write_enabled,
depth_compare: depth_stencil.depth_compare.map(Into::into),
stencil: wgpu_types::StencilState {
front,
back,
Expand Down
42 changes: 24 additions & 18 deletions examples/features/src/shadow/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,17 +506,20 @@ impl crate::framework::Example for Example {
.contains(wgpu::Features::DEPTH_CLIP_CONTROL),
..Default::default()
},
depth_stencil: Some(wgpu::DepthStencilState {
format: Self::SHADOW_FORMAT,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState {
constant: 2, // corresponds to bilinear filtering
slope_scale: 2.0,
clamp: 0.0,
},
}),
depth_stencil: Some(
wgpu::DepthStencilState {
format: Self::SHADOW_FORMAT,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState {
constant: 2, // corresponds to bilinear filtering
slope_scale: 2.0,
clamp: 0.0,
},
}
.into(),
),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand Down Expand Up @@ -645,13 +648,16 @@ impl crate::framework::Example for Example {
cull_mode: Some(wgpu::Face::Back),
..Default::default()
},
depth_stencil: Some(wgpu::DepthStencilState {
format: Self::DEPTH_FORMAT,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::Less,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
depth_stencil: Some(
wgpu::DepthStencilState {
format: Self::DEPTH_FORMAT,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::Less,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}
.into(),
),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand Down
34 changes: 20 additions & 14 deletions examples/features/src/skybox/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,16 @@ impl crate::framework::Example for Example {
front_face: wgpu::FrontFace::Cw,
..Default::default()
},
depth_stencil: Some(wgpu::DepthStencilState {
format: Self::DEPTH_FORMAT,
depth_write_enabled: false,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
depth_stencil: Some(
wgpu::DepthStencilState {
format: Self::DEPTH_FORMAT,
depth_write_enabled: false,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}
.into(),
),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand Down Expand Up @@ -244,13 +247,16 @@ impl crate::framework::Example for Example {
front_face: wgpu::FrontFace::Cw,
..Default::default()
},
depth_stencil: Some(wgpu::DepthStencilState {
format: Self::DEPTH_FORMAT,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
depth_stencil: Some(
wgpu::DepthStencilState {
format: Self::DEPTH_FORMAT,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}
.into(),
),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand Down
44 changes: 22 additions & 22 deletions examples/features/src/stencil_triangles/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,25 @@ impl crate::framework::Example for Example {
})],
}),
primitive: Default::default(),
depth_stencil: Some(wgpu::DepthStencilState {
format: wgpu::TextureFormat::Stencil8,
depth_write_enabled: false,
depth_compare: wgpu::CompareFunction::Always,
stencil: wgpu::StencilState {
front: wgpu::StencilFaceState {
compare: wgpu::CompareFunction::Always,
pass_op: wgpu::StencilOperation::Replace,
..Default::default()
depth_stencil: Some(
wgpu::DepthStencilState {
format: wgpu::TextureFormat::Stencil8,
depth_write_enabled: false,
depth_compare: wgpu::CompareFunction::Always,
stencil: wgpu::StencilState {
front: wgpu::StencilFaceState {
compare: wgpu::CompareFunction::Always,
pass_op: wgpu::StencilOperation::Replace,
..Default::default()
},
back: wgpu::StencilFaceState::IGNORE,
read_mask: !0,
write_mask: !0,
},
back: wgpu::StencilFaceState::IGNORE,
read_mask: !0,
write_mask: !0,
},
bias: Default::default(),
}),
bias: Default::default(),
}
.into(),
),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand All @@ -120,11 +123,9 @@ impl crate::framework::Example for Example {
targets: &[Some(config.view_formats[0].into())],
}),
primitive: Default::default(),
depth_stencil: Some(wgpu::DepthStencilState {
format: wgpu::TextureFormat::Stencil8,
depth_write_enabled: false,
depth_compare: wgpu::CompareFunction::Always,
stencil: wgpu::StencilState {
depth_stencil: Some(wgpu::DepthStencilState::stencil(
wgpu::TextureFormat::Stencil8,
wgpu::StencilState {
front: wgpu::StencilFaceState {
compare: wgpu::CompareFunction::Greater,
..Default::default()
Expand All @@ -133,8 +134,7 @@ impl crate::framework::Example for Example {
read_mask: !0,
write_mask: !0,
},
bias: Default::default(),
}),
)),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand Down
20 changes: 9 additions & 11 deletions examples/features/src/water/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,16 +555,14 @@ impl crate::framework::Example for Example {
// will work. Since this is water, we need to read from the
// depth buffer both as a texture in the shader, and as an
// input attachment to do depth-testing. We don't write, so
// depth_write_enabled is set to false. This is called
// RODS or read-only depth stencil.
depth_stencil: Some(wgpu::DepthStencilState {
// We don't use stencil.
format: wgpu::TextureFormat::Depth32Float,
depth_write_enabled: false,
depth_compare: wgpu::CompareFunction::Less,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
// depth_write_enabled is set to false. This is called RODS,
// or read-only depth stencil. Here, we don't use stencil.
depth_stencil: Some(wgpu::DepthStencilState::depth(
wgpu::TextureFormat::Depth32Float,
false,
wgpu::CompareFunction::Less,
wgpu::DepthBiasState::default(),
)),
// No multisampling is used.
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
Expand Down Expand Up @@ -603,7 +601,7 @@ impl crate::framework::Example for Example {
depth_compare: wgpu::CompareFunction::Less,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
}.into()),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/wgpu-gpu/mesh_shader/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ fn mesh_pipeline_build(ctx: &TestingContext, info: MeshPipelineTestInfo) {
cull_mode: Some(wgpu::Face::Back),
..Default::default()
},
depth_stencil: Some(depth_state),
depth_stencil: Some(depth_state.into()),
multisample: Default::default(),
multiview: None,
cache: None,
Expand Down Expand Up @@ -308,7 +308,7 @@ fn mesh_draw(ctx: &TestingContext, draw_type: DrawType, info: MeshPipelineTestIn
cull_mode: Some(wgpu::Face::Back),
..Default::default()
},
depth_stencil: Some(depth_state),
depth_stencil: Some(depth_state.into()),
multisample: Default::default(),
multiview: None,
cache: None,
Expand Down
17 changes: 10 additions & 7 deletions tests/tests/wgpu-gpu/occlusion_query/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ static OCCLUSION_QUERY: GpuTestConfiguration = GpuTestConfiguration::new()
},
fragment: None,
primitive: wgpu::PrimitiveState::default(),
depth_stencil: Some(wgpu::DepthStencilState {
format: wgpu::TextureFormat::Depth32Float,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::Less,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
depth_stencil: Some(
wgpu::DepthStencilState {
format: wgpu::TextureFormat::Depth32Float,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::Less,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}
.into(),
),
multisample: wgpu::MultisampleState::default(),
multiview_mask: None,
cache: None,
Expand Down
17 changes: 10 additions & 7 deletions tests/tests/wgpu-gpu/render_pass_ownership.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,13 +537,16 @@ fn resource_setup(ctx: &TestingContext) -> ResourceSetup {
strip_index_format: Some(wgpu::IndexFormat::Uint32),
..Default::default()
},
depth_stencil: Some(wgpu::DepthStencilState {
format: depth_stencil_format,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}),
depth_stencil: Some(
wgpu::DepthStencilState {
format: depth_stencil_format,
depth_write_enabled: true,
depth_compare: wgpu::CompareFunction::LessEqual,
stencil: wgpu::StencilState::default(),
bias: wgpu::DepthBiasState::default(),
}
.into(),
),
multisample: wgpu::MultisampleState {
count: target_msaa,
mask: !0,
Expand Down
20 changes: 19 additions & 1 deletion wgpu-core/src/device/resource.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4126,6 +4126,7 @@ impl Device {
.map_err(pipeline::CreateRenderPipelineError::ColorAttachment)?;

if let Some(ds) = depth_stencil_state {
// See <https://gpuweb.github.io/gpuweb/#abstract-opdef-validating-gpudepthstencilstate>.
target_specified = true;
let error = 'error: {
if !ds.format.is_depth_stencil_format() {
Expand All @@ -4147,6 +4148,23 @@ impl Device {
}

let aspect = hal::FormatAspects::from(ds.format);
if aspect.contains(hal::FormatAspects::DEPTH) {
let Some(depth_write_enabled) = ds.depth_write_enabled else {
break 'error Some(
pipeline::DepthStencilStateError::MissingDepthWriteEnabled(ds.format),
);
};

let depth_compare_required = depth_write_enabled
|| ds.stencil.front.depth_fail_op != wgt::StencilOperation::Keep
|| ds.stencil.back.depth_fail_op != wgt::StencilOperation::Keep;
if depth_compare_required && ds.depth_compare.is_none() {
break 'error Some(pipeline::DepthStencilStateError::MissingDepthCompare(
ds.format,
));
}
}

if ds.is_depth_enabled() && !aspect.contains(hal::FormatAspects::DEPTH) {
break 'error Some(pipeline::DepthStencilStateError::FormatNotDepth(ds.format));
}
Expand Down Expand Up @@ -4227,7 +4245,7 @@ impl Device {
let stage_desc = &vertex.stage;
let stage = validation::ShaderStageForValidation::Vertex {
topology: desc.primitive.topology,
compare_function: desc.depth_stencil.as_ref().map(|d| d.depth_compare),
compare_function: desc.depth_stencil.as_ref().and_then(|d| d.depth_compare),
};
let stage_bit = stage.to_wgt_bit();

Expand Down
Loading
Loading