From 3c0eeda7a5820c51e4ce6638f09062675e58d7aa Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Tue, 7 Oct 2025 09:43:11 -0400 Subject: [PATCH 1/3] =?UTF-8?q?refactor:=20`Interface::check=5Fstage`:=20u?= =?UTF-8?q?se=20`concat!(=E2=80=A6)`=20for=20non-invariant=20`@builtin(pos?= =?UTF-8?q?ition)`=20warning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- wgpu-core/src/validation.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 2c2f4b36c44..e29f088a987 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -1332,10 +1332,18 @@ impl Interface { 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." + concat!( + "Vertex shader with entry point {} 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." + ), + entry_point_name, + cmp = cmp ); } } From 2e2d9b55de3330f4eec9359211d3ab009f418905 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Mon, 6 Oct 2025 16:16:12 -0400 Subject: [PATCH 2/3] refactor: `check_stage`: convert `if` to `match` for I/O checks --- wgpu-core/src/validation.rs | 59 ++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index e29f088a987..13a3257f9c8 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -1317,37 +1317,42 @@ impl Interface { } } - if shader_stage == naga::ShaderStage::Vertex { - for output in entry_point.outputs.iter() { - //TODO: count builtins towards the limit? - inter_stage_components += match *output { - 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 + #[expect(clippy::single_match)] + match shader_stage { + naga::ShaderStage::Vertex => { + for output in entry_point.outputs.iter() { + //TODO: count builtins towards the limit? + inter_stage_components += match *output { + 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 { - log::warn!( - concat!( - "Vertex shader with entry point {} 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." - ), - entry_point_name, - cmp = cmp - ); + if let Varying::BuiltIn(naga::BuiltIn::Position { invariant: false }) = + *output + { + log::warn!( + concat!( + "Vertex shader with entry point {} 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." + ), + entry_point_name, + cmp = cmp + ); + } } } } + _ => (), } if inter_stage_components > self.limits.max_inter_stage_shader_components { From 5f9d9bca2e98b5b3d4795e0e9b3d638681ff2f81 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Mon, 6 Oct 2025 16:16:12 -0400 Subject: [PATCH 3/3] fix: reject fragment shader output `location`s > `max_color_attachments` limit --- CHANGELOG.md | 6 ++ tests/tests/wgpu-validation/api/mod.rs | 1 + .../wgpu-validation/api/render_pipeline.rs | 70 +++++++++++++++++++ wgpu-core/src/validation.rs | 23 +++++- 4 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 tests/tests/wgpu-validation/api/render_pipeline.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fa8bd6303c..5845dea261c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,12 @@ This is a breaking change By @R-Cramer4 in [#8230](https://github.com/gfx-rs/wgpu/pull/8230) +### Bug Fixes + +#### General + +- Reject fragment shader output `location`s > `max_color_attachments` limit. By @ErichDonGubler in [#8316](https://github.com/gfx-rs/wgpu/pull/8316). + ## v27.0.2 (2025-10-03) ### Bug Fixes diff --git a/tests/tests/wgpu-validation/api/mod.rs b/tests/tests/wgpu-validation/api/mod.rs index a9c6abf1d16..541647bcc26 100644 --- a/tests/tests/wgpu-validation/api/mod.rs +++ b/tests/tests/wgpu-validation/api/mod.rs @@ -7,4 +7,5 @@ mod device; mod experimental; mod external_texture; mod instance; +mod render_pipeline; mod texture; diff --git a/tests/tests/wgpu-validation/api/render_pipeline.rs b/tests/tests/wgpu-validation/api/render_pipeline.rs new file mode 100644 index 00000000000..f7dca6845b9 --- /dev/null +++ b/tests/tests/wgpu-validation/api/render_pipeline.rs @@ -0,0 +1,70 @@ +//! Tests of [`wgpu::RenderPipeline`] and related. + +use wgpu_hal::MAX_COLOR_ATTACHMENTS; +use wgpu_test::fail; + +#[test] +fn reject_fragment_shader_output_over_max_color_attachments() { + let (device, _queue) = wgpu::Device::noop(&Default::default()); + + // NOTE: Vertex shader is a boring quad. The fragment shader is the interesting part. + let source = format!( + "\ +@vertex +fn vert(@builtin(vertex_index) vertex_index : u32) -> @builtin(position) vec4f {{ + var pos = array( + vec2(0.0, 0.5), + vec2(-0.5, -0.5), + vec2(0.5, -0.5) + ); + return vec4f(pos[vertex_index], 0.0, 1.0); +}} + +@fragment +fn frag() -> @location({MAX_COLOR_ATTACHMENTS}) vec4f {{ + return vec4(1.0, 0.0, 0.0, 1.0); +}} +" + ); + + let module = device.create_shader_module(wgpu::ShaderModuleDescriptor { + label: None, + source: wgpu::ShaderSource::Wgsl(source.into()), + }); + let module = &module; + + fail( + &device, + || { + device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { + layout: None, + label: None, + vertex: wgpu::VertexState { + module, + entry_point: None, + compilation_options: Default::default(), + buffers: &[], + }, + fragment: Some(wgpu::FragmentState { + module, + entry_point: None, + compilation_options: Default::default(), + targets: &[Some(wgpu::ColorTargetState { + format: wgpu::TextureFormat::Rgba8Unorm, + blend: None, + write_mask: Default::default(), + })], + }), + primitive: Default::default(), + depth_stencil: None, + multisample: Default::default(), + multiview: None, + cache: None, + }) + }, + Some(concat!( + "Location[8] Float32x4 interpolated as Some(Perspective) ", + "with sampling Some(Center)'s index exceeds `max_color_attachments` (8)" + )), + ); +} diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 13a3257f9c8..166b5931fa6 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -313,6 +313,11 @@ pub enum StageError { MultipleEntryPointsFound, #[error(transparent)] InvalidResource(#[from] InvalidResourceError), + #[error( + "Location[{location}] {var}'s index exceeds `max_color_attachments` ({})", + hal::MAX_COLOR_ATTACHMENTS + )] + ColorAttachmentLocationExceedsLimit { location: u32, var: InterfaceVar }, } impl WebGpuError for StageError { @@ -334,7 +339,8 @@ impl WebGpuError for StageError { | Self::TooManyVaryings { .. } | Self::MissingEntryPoint(..) | Self::NoEntryPointFound - | Self::MultipleEntryPointsFound => return ErrorType::Validation, + | Self::MultipleEntryPointsFound + | Self::ColorAttachmentLocationExceedsLimit { .. } => return ErrorType::Validation, }; e.webgpu_error_type() } @@ -1317,7 +1323,6 @@ impl Interface { } } - #[expect(clippy::single_match)] match shader_stage { naga::ShaderStage::Vertex => { for output in entry_point.outputs.iter() { @@ -1352,6 +1357,19 @@ impl Interface { } } } + naga::ShaderStage::Fragment => { + for output in &entry_point.outputs { + let &Varying::Local { location, ref iv } = output else { + continue; + }; + if location >= self.limits.max_color_attachments { + return Err(StageError::ColorAttachmentLocationExceedsLimit { + location, + var: iv.clone(), + }); + } + } + } _ => (), } @@ -1370,6 +1388,7 @@ impl Interface { Varying::BuiltIn(_) => None, }) .collect(); + Ok(outputs) }