Skip to content

Commit 537c6be

Browse files
authored
Add warning when using CompareFunction::*Equal without an invariant Attribute (#2887)
1 parent 6448c60 commit 537c6be

File tree

4 files changed

+44
-2
lines changed

4 files changed

+44
-2
lines changed

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,32 @@ Bottom level categories:
4040

4141
## Unreleased
4242

43+
### Major Changes
44+
45+
#### @invariant Warning
46+
47+
When using CompareFunction::Equal or CompareFunction::NotEqual on a pipeline, there is now a warning logged if the vertex
48+
shader does not have a @invariant tag on it. On some machines, rendering the same triangles multiple times without an
49+
@invariant tag will result in slightly different depths for every pixel. Because the *Equal functions rely on depth being
50+
the same every time it is rendered, we now warn if it is missing.
51+
52+
```diff
53+
-@vertex
54+
-fn vert_main(v_in: VertexInput) -> @builtin(position) vec4<f32> {...}
55+
+@vertex
56+
+fn vert_main(v_in: VertexInput) -> @builtin(position) @invariant vec4<f32> {...}
57+
```
58+
4359
### Bug Fixes
4460

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

4864
### Changes
4965

66+
#### General
67+
- 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)
68+
5069
#### Metal
5170
- Extract the generic code into `get_metal_layer` by @jinleili in [#2826](https://github.com/gfx-rs/wgpu/pull/2826)
5271

wgpu-core/src/device/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2379,6 +2379,7 @@ impl<A: HalApi> Device<A> {
23792379
&desc.stage.entry_point,
23802380
flag,
23812381
io,
2382+
None,
23822383
)?;
23832384
}
23842385
}
@@ -2704,6 +2705,7 @@ impl<A: HalApi> Device<A> {
27042705
&stage.entry_point,
27052706
flag,
27062707
io,
2708+
desc.depth_stencil.as_ref().map(|d| d.depth_compare),
27072709
)
27082710
.map_err(|error| pipeline::CreateRenderPipelineError::Stage {
27092711
stage: flag,
@@ -2750,6 +2752,7 @@ impl<A: HalApi> Device<A> {
27502752
&fragment.stage.entry_point,
27512753
flag,
27522754
io,
2755+
desc.depth_stencil.as_ref().map(|d| d.depth_compare),
27532756
)
27542757
.map_err(|error| pipeline::CreateRenderPipelineError::Stage {
27552758
stage: flag,

wgpu-core/src/validation.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,7 @@ impl Interface {
963963
entry_point_name: &str,
964964
stage_bit: wgt::ShaderStages,
965965
inputs: StageIo,
966+
compare_function: Option<wgt::CompareFunction>,
966967
) -> Result<StageIo, StageError> {
967968
// Since a shader module can have multiple entry points with the same name,
968969
// we need to look for one with the right execution model.
@@ -1183,6 +1184,21 @@ impl Interface {
11831184
Varying::Local { ref iv, .. } => iv.ty.dim.num_components(),
11841185
Varying::BuiltIn(_) => 0,
11851186
};
1187+
1188+
if let Some(
1189+
cmp @ wgt::CompareFunction::Equal | cmp @ wgt::CompareFunction::NotEqual,
1190+
) = compare_function
1191+
{
1192+
if let Varying::BuiltIn(naga::BuiltIn::Position { invariant: false }) = *output
1193+
{
1194+
log::warn!(
1195+
"Vertex shader with entry point {entry_point_name} outputs a @builtin(position) without the @invariant \
1196+
attribute and is used in a pipeline with {cmp:?}. On some machines, this can cause bad artifacting as {cmp:?} assumes \
1197+
the values output from the vertex shader exactly match the value in the depth buffer. The @invariant attribute on the \
1198+
@builtin(position) vertex output ensures that the exact same pixel depths are used every render."
1199+
);
1200+
}
1201+
}
11861202
}
11871203
}
11881204

wgpu-types/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2602,13 +2602,17 @@ pub enum CompareFunction {
26022602
Never = 1,
26032603
/// Function passes if new value less than existing value
26042604
Less = 2,
2605-
/// Function passes if new value is equal to existing value
2605+
/// Function passes if new value is equal to existing value. When using
2606+
/// this compare function, make sure to mark your Vertex Shader's `@builtin(position)`
2607+
/// output as `@invariant` to prevent artifacting.
26062608
Equal = 3,
26072609
/// Function passes if new value is less than or equal to existing value
26082610
LessEqual = 4,
26092611
/// Function passes if new value is greater than existing value
26102612
Greater = 5,
2611-
/// Function passes if new value is not equal to existing value
2613+
/// Function passes if new value is not equal to existing value. When using
2614+
/// this compare function, make sure to mark your Vertex Shader's `@builtin(position)`
2615+
/// output as `@invariant` to prevent artifacting.
26122616
NotEqual = 6,
26132617
/// Function passes if new value is greater than or equal to existing value
26142618
GreaterEqual = 7,

0 commit comments

Comments
 (0)