Skip to content

Commit 664d06e

Browse files
committed
fix(core): Fix validation of shader I/O numeric types
- Inter-stage types must match exactly - Fragment outputs only require that scalar kind matches. Scalar width may differ in either direction.
1 parent c2d5fa4 commit 664d06e

4 files changed

Lines changed: 75 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ Bottom level categories:
9393

9494
- Validate that the arguments are within the indirect buffer when encoding an indirect draw to a render bundle. Moves some indirect draw errors from `RenderPassErrorInner` to `RenderCommandError`. By @andyleiserson in [#9871](https://github.com/gfx-rs/wgpu/pull/9871).
9595
- When creating render pipelines, validate that a corresponding shader output is present for each color attachment with non-zero write mask, and that shader output includes alpha when the blend operation uses source alpha. By @andyleiserson in [#9939](https://github.com/gfx-rs/wgpu/pull/9939).
96+
- Numeric types must now match exactly on inter-stage interfaces. Previously, the receiving type was only required to be a subtype of the originating type. By @andyleiserson in TBD.
97+
- Relaxed requirement that the pipeline scalar type for a color output be at least as wide as the shader output type. Now, only the scalar kind must match. By @andyleiserson in TBD.
9698

9799
#### Naga
98100

cts_runner/test.lst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ webgpu:api,validation,render_pipeline,fragment_state:targets_write_mask:*
277277
webgpu:api,validation,render_pipeline,inter_stage:location,*
278278
webgpu:api,validation,render_pipeline,inter_stage:max_shader_variable_location:*
279279
fails-if(dx12) webgpu:api,validation,render_pipeline,inter_stage:max_variables_count,*
280+
webgpu:api,validation,render_pipeline,inter_stage:type:*
280281
webgpu:api,validation,render_pipeline,misc:basic:*
281282
fails-if(vulkan) webgpu:api,validation,render_pipeline,misc:external_texture:*
282283
webgpu:api,validation,render_pipeline,misc:no_attachment:*

tests/tests/wgpu-validation/api/render_pipeline.rs

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Tests of [`wgpu::RenderPipeline`] and related.
22
3-
use wgpu_test::fail;
3+
use wgpu_test::{fail, valid};
44

55
#[test]
66
fn reject_fragment_shader_output_over_max_color_attachments() {
@@ -68,3 +68,69 @@ fn frag() -> @location({}) vec4f {{
6868
)),
6969
);
7070
}
71+
72+
/// A fragment shader may output an `f16` value to a color target whose sample
73+
/// type is `float`, even when the format's components are 32-bit. The WebGPU
74+
/// spec only requires the output's scalar *kind* (floating-point) to match the
75+
/// format's sample type, not its bit width.
76+
#[test]
77+
fn accept_f16_fragment_output_to_f32_target() {
78+
let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor {
79+
required_features: wgpu::Features::SHADER_F16,
80+
..Default::default()
81+
});
82+
83+
// NOTE: Vertex shader is a boring quad. The fragment shader is the interesting part.
84+
let source = "\
85+
enable f16;
86+
87+
@vertex
88+
fn vert(@builtin(vertex_index) vertex_index : u32) -> @builtin(position) vec4f {
89+
var pos = array<vec2f, 3>(
90+
vec2(0.0, 0.5),
91+
vec2(-0.5, -0.5),
92+
vec2(0.5, -0.5)
93+
);
94+
return vec4f(pos[vertex_index], 0.0, 1.0);
95+
}
96+
97+
@fragment
98+
fn frag() -> @location(0) vec4h {
99+
return vec4h(1.0h, 0.0h, 0.0h, 1.0h);
100+
}
101+
";
102+
103+
let module = device.create_shader_module(wgpu::ShaderModuleDescriptor {
104+
label: None,
105+
source: wgpu::ShaderSource::Wgsl(source.into()),
106+
});
107+
let module = &module;
108+
109+
valid(&device, || {
110+
device.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
111+
layout: None,
112+
label: None,
113+
vertex: wgpu::VertexState {
114+
module,
115+
entry_point: None,
116+
compilation_options: Default::default(),
117+
buffers: &[],
118+
},
119+
fragment: Some(wgpu::FragmentState {
120+
module,
121+
entry_point: None,
122+
compilation_options: Default::default(),
123+
targets: &[Some(wgpu::ColorTargetState {
124+
format: wgpu::TextureFormat::Rgba8Unorm,
125+
blend: None,
126+
write_mask: Default::default(),
127+
})],
128+
}),
129+
primitive: Default::default(),
130+
depth_stencil: None,
131+
multisample: Default::default(),
132+
multiview_mask: None,
133+
cache: None,
134+
})
135+
});
136+
}

wgpu-core/src/validation.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,14 +1029,11 @@ impl NumericType {
10291029
}
10301030
}
10311031

1032-
fn is_subtype_of(self, other: NumericType) -> bool {
1033-
if self.scalar.width > other.scalar.width {
1032+
fn compatible_with_shader_output(self, shader: NumericType) -> bool {
1033+
if self.scalar.kind != shader.scalar.kind {
10341034
return false;
10351035
}
1036-
if self.scalar.kind != other.scalar.kind {
1037-
return false;
1038-
}
1039-
match (self.dim, other.dim) {
1036+
match (self.dim, shader.dim) {
10401037
(NumericDimension::Scalar, NumericDimension::Scalar) => true,
10411038
(NumericDimension::Scalar, NumericDimension::Vector(_)) => true,
10421039
(NumericDimension::Vector(s0), NumericDimension::Vector(s1)) => s0 <= s1,
@@ -1054,7 +1051,7 @@ pub fn check_color_attachment_compatibility(
10541051
output_ty: NumericType,
10551052
) -> Result<(), ColorStateError> {
10561053
let pipeline_ty = NumericType::from_texture_format(state.format);
1057-
if !pipeline_ty.is_subtype_of(output_ty) {
1054+
if !pipeline_ty.compatible_with_shader_output(output_ty) {
10581055
return Err(ColorStateError::IncompatibleFormat {
10591056
pipeline: pipeline_ty,
10601057
shader: output_ty,
@@ -1647,7 +1644,7 @@ impl Interface {
16471644
));
16481645
}
16491646
(
1650-
iv.ty.is_subtype_of(provided.ty),
1647+
iv.ty == provided.ty,
16511648
iv.per_primitive == provided.per_primitive,
16521649
)
16531650
}

0 commit comments

Comments
 (0)