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

Add warning when using CompareFunction::*Equal without an invariant Attribute #2887

Merged
merged 2 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -2371,6 +2371,7 @@ impl<A: HalApi> Device<A> {
&desc.stage.entry_point,
flag,
io,
None,
)?;
}
}
Expand Down Expand Up @@ -2696,6 +2697,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 @@ -2742,6 +2744,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