Skip to content
Open
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/tests/wgpu-validation/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ mod device;
mod experimental;
mod external_texture;
mod instance;
mod render_pipeline;
mod texture;
70 changes: 70 additions & 0 deletions tests/tests/wgpu-validation/api/render_pipeline.rs
Original file line number Diff line number Diff line change
@@ -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<vec2f, 3>(
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)"
)),
);
}
72 changes: 52 additions & 20 deletions wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name bikeshedding: I would prefer ...TooLarge or ...OutOfBounds (strings that we already use in error variants) over ExceedsLimit. (Although I do see we have ExceededLimit... appearing in one place.)

I also might be inclined to make the name general enough to cover the case that the location lands on a gap between defined color targets (e.g. InvalidColorAttachmentLocation), although the primary responsibility for checking that seems to be later in create_render_pipeline (where it presently only generates a log::warn!), so maybe it's not important.

};
e.webgpu_error_type()
}
Expand Down Expand Up @@ -1317,29 +1323,54 @@ 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true today that this will always be equal to hal::MAX_COLOR_ATTACHMENTS (used for the error message), but I don't know if we want to rely on that.

There's also a question whether it is an error for an output that is not consumed by the pipeline to be placed at a location above maxColorAttachments. The spec describes maxColorAttachments as a limit on the number of entries in various arrays appearing in descriptors.

return Err(StageError::ColorAttachmentLocationExceedsLimit {
location,
var: iv.clone(),
});
}
}
}
_ => (),
}

if inter_stage_components > self.limits.max_inter_stage_shader_components {
Expand All @@ -1357,6 +1388,7 @@ impl Interface {
Varying::BuiltIn(_) => None,
})
.collect();

Ok(outputs)
}

Expand Down
Loading