Skip to content

Commit 1b4b07f

Browse files
fix(core): cover clip_distances bindings in max_inter_stage_shader_variables limit check
1 parent ab59418 commit 1b4b07f

3 files changed

Lines changed: 63 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ Bottom level categories:
4848

4949
- `Features::CLIP_DISTANCE`, `naga::Capabilities::CLIP_DISTANCE`, and `naga::BuiltIn::ClipDistance` have been renamed to `CLIP_DISTANCES` and `ClipDistances` (viz., pluralized) as appropriate, to match the WebGPU spec. By @ErichDonGubler in [#9267](https://github.com/gfx-rs/wgpu/pull/9267).
5050

51+
#### Validation
52+
53+
- Add clip distances validation for `maxInterStageShaderVariables`. By @ErichDonGubler in [8762](https://github.com/gfx-rs/wgpu/pull/8762). This may break some existing programs, but it compiles with the WebGPU spec.
54+
5155
### Bug Fixes
5256

5357
#### General

wgpu-core/src/validation.rs

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ enum BuiltIn {
162162
ViewIndex,
163163
BaseInstance,
164164
BaseVertex,
165-
ClipDistances,
165+
ClipDistances { array_size: u32 },
166166
CullDistance,
167167
InstanceIndex,
168168
PointSize,
@@ -216,7 +216,7 @@ impl BuiltIn {
216216
Self::ViewIndex => naga::BuiltIn::ViewIndex,
217217
Self::BaseInstance => naga::BuiltIn::BaseInstance,
218218
Self::BaseVertex => naga::BuiltIn::BaseVertex,
219-
Self::ClipDistances => naga::BuiltIn::ClipDistances,
219+
Self::ClipDistances { .. } => naga::BuiltIn::ClipDistances,
220220
Self::CullDistance => naga::BuiltIn::CullDistance,
221221
Self::InstanceIndex => naga::BuiltIn::InstanceIndex,
222222
Self::PointSize => naga::BuiltIn::PointSize,
@@ -1078,6 +1078,33 @@ impl Interface {
10781078
}
10791079
return;
10801080
}
1081+
naga::TypeInner::Array { base, size, stride }
1082+
if matches!(
1083+
binding,
1084+
Some(naga::Binding::BuiltIn(naga::BuiltIn::ClipDistances)),
1085+
) =>
1086+
{
1087+
// NOTE: We should already have validated these in `naga`.
1088+
debug_assert_eq!(
1089+
&arena[base].inner,
1090+
&naga::TypeInner::Scalar(naga::Scalar::F32)
1091+
);
1092+
debug_assert_eq!(stride, 4);
1093+
1094+
let naga::ArraySize::Constant(array_size) = size else {
1095+
// NOTE: Based on the
1096+
// [spec](https://gpuweb.github.io/gpuweb/wgsl/#fixed-footprint-types):
1097+
//
1098+
// > The only valid use of a fixed-size array with an element count that is an
1099+
// > override-expression that is not a const-expression is as a memory view in
1100+
// > the workgroup address space.
1101+
unreachable!("non-constant array size for `clip_distances`")
1102+
};
1103+
let array_size = array_size.get();
1104+
1105+
list.push(Varying::BuiltIn(BuiltIn::ClipDistances { array_size }));
1106+
return;
1107+
}
10811108
ref other => {
10821109
//Note: technically this should be at least `log::error`, but
10831110
// the reality is - every shader coming from `glslc` outputs an array
@@ -1110,7 +1137,7 @@ impl Interface {
11101137
naga::BuiltIn::ViewIndex => BuiltIn::ViewIndex,
11111138
naga::BuiltIn::BaseInstance => BuiltIn::BaseInstance,
11121139
naga::BuiltIn::BaseVertex => BuiltIn::BaseVertex,
1113-
naga::BuiltIn::ClipDistances => BuiltIn::ClipDistances,
1140+
naga::BuiltIn::ClipDistances => unreachable!(),
11141141
naga::BuiltIn::CullDistance => BuiltIn::CullDistance,
11151142
naga::BuiltIn::InstanceIndex => BuiltIn::InstanceIndex,
11161143
naga::BuiltIn::PointSize => BuiltIn::PointSize,
@@ -1598,7 +1625,21 @@ impl Interface {
15981625
None
15991626
};
16001627

1601-
let deductions = point_list_deduction.into_iter();
1628+
let clip_distance_deductions = entry_point.outputs.iter().filter_map(|output| {
1629+
if let &Varying::BuiltIn(BuiltIn::ClipDistances { array_size }) = output {
1630+
Some(MaxVertexShaderOutputDeduction::ClipDistances { array_size })
1631+
} else {
1632+
None
1633+
}
1634+
});
1635+
debug_assert!(
1636+
clip_distance_deductions.clone().count() <= 1,
1637+
"multiple `clip_distances` outputs found"
1638+
);
1639+
1640+
let deductions = point_list_deduction
1641+
.into_iter()
1642+
.chain(clip_distance_deductions);
16021643

16031644
for deduction in deductions.clone() {
16041645
// NOTE: Deductions, in the current version of the spec. we implement, do not

wgpu-core/src/validation/shader_io_deductions.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,32 @@ pub enum MaxVertexShaderOutputDeduction {
1212
/// When a pipeline's [`crate::pipeline::RenderPipelineDescriptor::primitive`] is set to
1313
/// [`wgt::PrimitiveTopology::PointList`].
1414
PointListPrimitiveTopology,
15+
/// When a clip distances array primitive is used in an output.
16+
ClipDistances { array_size: u32 },
17+
}
18+
19+
impl MaxVertexShaderOutputDeduction {
20+
fn variables_from_clip_distance_slot(num_slots: u32) -> u32 {
21+
num_slots.div_ceil(4)
22+
}
1523
}
1624

1725
impl MaxVertexShaderOutputDeduction {
1826
pub fn for_variables(self) -> u32 {
1927
match self {
2028
Self::PointListPrimitiveTopology => 1,
29+
Self::ClipDistances { array_size } => {
30+
Self::variables_from_clip_distance_slot(array_size)
31+
}
2132
}
2233
}
2334

2435
pub fn for_location(self) -> u32 {
2536
match self {
2637
Self::PointListPrimitiveTopology => 0,
38+
Self::ClipDistances { array_size } => {
39+
Self::variables_from_clip_distance_slot(array_size)
40+
}
2741
}
2842
}
2943
}

0 commit comments

Comments
 (0)