diff --git a/CHANGELOG.md b/CHANGELOG.md index 73f64b5d17d..23fd1c475c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,12 @@ SamplerDescriptor { } ``` +### 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..654197212e0 --- /dev/null +++ b/tests/tests/wgpu-validation/api/render_pipeline.rs @@ -0,0 +1,70 @@ +//! Tests of [`wgpu::RenderPipeline`] and related. + +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({}) vec4f {{ + return vec4(1.0, 0.0, 0.0, 1.0); +}} +", + device.limits().max_color_attachments + ); + + 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 the `max_color_attachments` limit (8)" + )), + ); +} diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 2c2f4b36c44..ffb0bf5a175 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -313,6 +313,14 @@ pub enum StageError { MultipleEntryPointsFound, #[error(transparent)] InvalidResource(#[from] InvalidResourceError), + #[error( + "Location[{location}] {var}'s index exceeds the `max_color_attachments` limit ({limit})" + )] + ColorAttachmentLocationTooLarge { + location: u32, + var: InterfaceVar, + limit: u32, + }, } impl WebGpuError for StageError { @@ -334,7 +342,8 @@ impl WebGpuError for StageError { | Self::TooManyVaryings { .. } | Self::MissingEntryPoint(..) | Self::NoEntryPointFound - | Self::MultipleEntryPointsFound => return ErrorType::Validation, + | Self::MultipleEntryPointsFound + | Self::ColorAttachmentLocationTooLarge { .. } => return ErrorType::Validation, }; e.webgpu_error_type() } @@ -1317,29 +1326,55 @@ 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 + 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!( - "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." - ); + 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 + ); + } } } } + 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::ColorAttachmentLocationTooLarge { + location, + var: iv.clone(), + limit: self.limits.max_color_attachments, + }); + } + } + } + _ => (), } if inter_stage_components > self.limits.max_inter_stage_shader_components { @@ -1357,6 +1392,7 @@ impl Interface { Varying::BuiltIn(_) => None, }) .collect(); + Ok(outputs) }