Skip to content

Commit

Permalink
Add warning when using CompareFunction::*Equal without an invariant A…
Browse files Browse the repository at this point in the history
…ttribute (#2887)
  • Loading branch information
cwfitzgerald authored Jul 21, 2022
1 parent 6448c60 commit 537c6be
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 2 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,32 @@ Bottom level categories:

## Unreleased

### Major Changes

#### @invariant Warning

When using CompareFunction::Equal or CompareFunction::NotEqual on a pipeline, there is now a warning logged if the vertex
shader does not have a @invariant tag on it. On some machines, rendering the same triangles multiple times without an
@invariant tag will result in slightly different depths for every pixel. Because the *Equal functions rely on depth being
the same every time it is rendered, we now warn if it is missing.

```diff
-@vertex
-fn vert_main(v_in: VertexInput) -> @builtin(position) vec4<f32> {...}
+@vertex
+fn vert_main(v_in: VertexInput) -> @builtin(position) @invariant vec4<f32> {...}
```

### Bug Fixes

#### General
- Improve the validation and error reporting of buffer mappings by @nical in [#2848](https://github.com/gfx-rs/wgpu/pull/2848)

### Changes

#### General
- Add warning when using CompareFunction::*Equal with vertex shader that is missing @invariant tag by @cwfitzgerald in [#2887](https://github.com/gfx-rs/wgpu/pull/2887)

#### Metal
- Extract the generic code into `get_metal_layer` by @jinleili in [#2826](https://github.com/gfx-rs/wgpu/pull/2826)

Expand Down
3 changes: 3 additions & 0 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2379,6 +2379,7 @@ impl<A: HalApi> Device<A> {
&desc.stage.entry_point,
flag,
io,
None,
)?;
}
}
Expand Down Expand Up @@ -2704,6 +2705,7 @@ impl<A: HalApi> Device<A> {
&stage.entry_point,
flag,
io,
desc.depth_stencil.as_ref().map(|d| d.depth_compare),
)
.map_err(|error| pipeline::CreateRenderPipelineError::Stage {
stage: flag,
Expand Down Expand Up @@ -2750,6 +2752,7 @@ impl<A: HalApi> Device<A> {
&fragment.stage.entry_point,
flag,
io,
desc.depth_stencil.as_ref().map(|d| d.depth_compare),
)
.map_err(|error| pipeline::CreateRenderPipelineError::Stage {
stage: flag,
Expand Down
16 changes: 16 additions & 0 deletions wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,7 @@ impl Interface {
entry_point_name: &str,
stage_bit: wgt::ShaderStages,
inputs: StageIo,
compare_function: Option<wgt::CompareFunction>,
) -> Result<StageIo, StageError> {
// Since a shader module can have multiple entry points with the same name,
// we need to look for one with the right execution model.
Expand Down Expand Up @@ -1183,6 +1184,21 @@ impl Interface {
Varying::Local { ref iv, .. } => iv.ty.dim.num_components(),
Varying::BuiltIn(_) => 0,
};

if let Some(
cmp @ wgt::CompareFunction::Equal | cmp @ wgt::CompareFunction::NotEqual,
) = compare_function
{
if let Varying::BuiltIn(naga::BuiltIn::Position { invariant: false }) = *output
{
log::warn!(
"Vertex shader with entry point {entry_point_name} outputs a @builtin(position) without the @invariant \
attribute and is used in a pipeline with {cmp:?}. On some machines, this can cause bad artifacting as {cmp:?} assumes \
the values output from the vertex shader exactly match the value in the depth buffer. The @invariant attribute on the \
@builtin(position) vertex output ensures that the exact same pixel depths are used every render."
);
}
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2602,13 +2602,17 @@ pub enum CompareFunction {
Never = 1,
/// Function passes if new value less than existing value
Less = 2,
/// Function passes if new value is equal to existing value
/// Function passes if new value is equal to existing value. When using
/// this compare function, make sure to mark your Vertex Shader's `@builtin(position)`
/// output as `@invariant` to prevent artifacting.
Equal = 3,
/// Function passes if new value is less than or equal to existing value
LessEqual = 4,
/// Function passes if new value is greater than existing value
Greater = 5,
/// Function passes if new value is not equal to existing value
/// Function passes if new value is not equal to existing value. When using
/// this compare function, make sure to mark your Vertex Shader's `@builtin(position)`
/// output as `@invariant` to prevent artifacting.
NotEqual = 6,
/// Function passes if new value is greater than or equal to existing value
GreaterEqual = 7,
Expand Down

0 comments on commit 537c6be

Please sign in to comment.