Skip to content

Commit 853e129

Browse files
ErichDonGublerErichDonGubler
committed
WIP: fix(core): add max_inter_stage_shader_variables validation
- add `StageError::VertexOutputLocationTooLarge` - add `StageError::TooManyVertexOutputs` - add `MaxInterStageShaderVariablesDeductions`
1 parent e84e1c7 commit 853e129

13 files changed

Lines changed: 145 additions & 7 deletions

File tree

‎cts_runner/test.lst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ webgpu:api,operation,vertex_state,correctness:setVertexBuffer_offset_and_attribu
3333
webgpu:api,validation,buffer,create:*
3434
webgpu:api,validation,buffer,destroy:*
3535
fails-if(dx12) webgpu:api,validation,capability_checks,limits,maxBindGroups:setBindGroup,*
36+
webgpu:api,validation,capability_checks,limits,maxInterStageShaderVariables:*
3637
webgpu:api,validation,createBindGroup:buffer_offset_and_size_for_bind_groups_match:*
3738
webgpu:api,validation,createBindGroup:buffer,effective_buffer_binding_size:*
3839
webgpu:api,validation,createBindGroup:buffer,resource_binding_size:*
@@ -135,6 +136,7 @@ webgpu:api,validation,render_pass,render_pass_descriptor:attachments,*
135136
webgpu:api,validation,render_pass,render_pass_descriptor:color_attachments,*
136137
webgpu:api,validation,render_pass,render_pass_descriptor:resolveTarget,*
137138
webgpu:api,validation,render_pass,resolve:resolve_attachment:*
139+
webgpu:api,validation,render_pipeline,inter_stage:max_shader_variable_location:*
138140
webgpu:api,validation,resource_usages,buffer,in_pass_encoder:*
139141
// FAIL: 2 other cases in resource_usages,texture,in_pass_encoder. https://github.com/gfx-rs/wgpu/issues/3126
140142
webgpu:api,validation,resource_usages,texture,in_pass_encoder:scope,*

‎deno_webgpu/01_webgpu.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,7 @@ ObjectDefineProperty(GPUSupportedLimitsPrototype, privateCustomInspect, {
273273
"maxBufferSize",
274274
"maxVertexAttributes",
275275
"maxVertexBufferArrayStride",
276-
// TODO(@crowlKats): support max_inter_stage_shader_variables
277-
// "maxInterStageShaderVariables",
276+
"maxInterStageShaderVariables",
278277
"maxColorAttachments",
279278
"maxColorAttachmentBytesPerSample",
280279
"maxComputeWorkgroupStorageSize",

‎deno_webgpu/adapter.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,10 @@ impl GPUSupportedLimits {
339339
self.0.max_vertex_buffer_array_stride
340340
}
341341

342-
// TODO(@crowlKats): support max_inter_stage_shader_variables
342+
#[getter]
343+
fn maxInterStageShaderVariables(&self) -> u32 {
344+
self.0.max_inter_stage_shader_variables
345+
}
343346

344347
#[getter]
345348
fn maxColorAttachments(&self) -> u32 {

‎wgpu-core/src/device/resource.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4230,6 +4230,7 @@ impl Device {
42304230
vertex_stage = {
42314231
let stage_desc = &vertex.stage;
42324232
let stage = validation::ShaderStageForValidation::Vertex {
4233+
topology: desc.primitive.topology,
42334234
compare_function: desc.depth_stencil.as_ref().map(|d| d.depth_compare),
42344235
};
42354236
let stage_bit = stage.to_wgt_bit();

‎wgpu-core/src/validation.rs‎

Lines changed: 120 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,25 @@ pub enum StageError {
328328
MultipleEntryPointsFound,
329329
#[error(transparent)]
330330
InvalidResource(#[from] InvalidResourceError),
331+
#[error(
332+
"Location[{location}]: {var}'s vertex shader output index exceeds the \
333+
`max_inter_stage_shader_variables` limit ({limit}); note that some " // TODO
334+
)]
335+
VertexOutputLocationTooLarge {
336+
location: u32,
337+
var: InterfaceVar,
338+
limit: u32,
339+
deductions: Vec<MaxInterStageShaderVariablesDeduction>,
340+
},
341+
#[error(
342+
"found {num_found} user-defined vertex shader output variables, which exceeds the \
343+
`max_inter_stage_shader_variables` limit ({limit}); note that some " // TODO
344+
)]
345+
TooManyUserDefinedVertexOutputs {
346+
num_found: u32,
347+
limit: u32,
348+
deductions: Vec<MaxInterStageShaderVariablesDeduction>,
349+
},
331350
#[error(
332351
"Location[{location}] {var}'s index exceeds the `max_color_attachments` limit ({limit})"
333352
)]
@@ -375,6 +394,8 @@ impl WebGpuError for StageError {
375394
| Self::MissingEntryPoint(..)
376395
| Self::NoEntryPointFound
377396
| Self::MultipleEntryPointsFound
397+
| Self::VertexOutputLocationTooLarge { .. }
398+
| Self::TooManyUserDefinedVertexOutputs { .. }
378399
| Self::ColorAttachmentLocationTooLarge { .. }
379400
| Self::TooManyMeshVertices { .. }
380401
| Self::TooManyMeshPrimitives { .. }
@@ -388,6 +409,42 @@ impl WebGpuError for StageError {
388409
}
389410
}
390411

412+
#[derive(Clone, Copy)]
413+
pub struct MaxInterStageShaderVariablesDeduction(MaxInterStageShaderVariablesDeductionImpl);
414+
415+
impl fmt::Debug for MaxInterStageShaderVariablesDeduction {
416+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
417+
let Self(inner) = self;
418+
fmt::Debug::fmt(inner, f)
419+
}
420+
}
421+
422+
#[derive(Clone, Copy, Debug)]
423+
enum MaxInterStageShaderVariablesDeductionImpl {
424+
PointLinePrimitiveTopology,
425+
ClipDistance { count: u32 },
426+
}
427+
428+
impl MaxInterStageShaderVariablesDeductionImpl {
429+
fn variables_from_clip_distance_slot(num_slots: u32) -> u32 {
430+
num_slots.div_ceil(4)
431+
}
432+
433+
pub fn for_variables(self) -> u32 {
434+
match self {
435+
Self::PointLinePrimitiveTopology => 1,
436+
Self::ClipDistance { count } => Self::variables_from_clip_distance_slot(count),
437+
}
438+
}
439+
440+
pub fn for_location(self) -> u32 {
441+
match self {
442+
Self::PointLinePrimitiveTopology => 0,
443+
Self::ClipDistance { count } => Self::variables_from_clip_distance_slot(count),
444+
}
445+
}
446+
}
447+
391448
pub fn map_storage_format_to_naga(format: wgt::TextureFormat) -> Option<naga::StorageFormat> {
392449
use naga::StorageFormat as Sf;
393450
use wgt::TextureFormat as Tf;
@@ -1445,13 +1502,60 @@ impl Interface {
14451502

14461503
match shader_stage {
14471504
ShaderStageForValidation::Vertex {
1505+
topology,
14481506
compare_function,
14491507
} => {
1508+
let mut max_vertex_shader_output_variables =
1509+
self.limits.max_inter_stage_shader_variables;
1510+
let mut max_vertex_shader_output_location = max_vertex_shader_output_variables - 1;
1511+
1512+
let point_list_deduction = if topology == wgt::PrimitiveTopology::PointList {
1513+
Some(MaxInterStageShaderVariablesDeductionImpl::PointLinePrimitiveTopology)
1514+
} else {
1515+
None
1516+
};
1517+
1518+
let clip_distance_deductions = entry_point.outputs.iter().filter_map(|output| {
1519+
if let Varying::BuiltIn(naga::BuiltIn::ClipDistance) = output {
1520+
Some(MaxInterStageShaderVariablesDeductionImpl::ClipDistance {
1521+
// NOTE: `clip_distances`' max array size is currently 8, so at most we deduct 2.
1522+
count: todo!("get size of `clip_distances` binding"),
1523+
})
1524+
} else {
1525+
None
1526+
}
1527+
});
1528+
1529+
let deductions = point_list_deduction
1530+
.into_iter()
1531+
.chain(clip_distance_deductions);
1532+
1533+
for deduction in deductions.clone() {
1534+
// NOTE: We assume that these will never get to 0.
1535+
max_vertex_shader_output_variables -= deduction.for_variables();
1536+
max_vertex_shader_output_location -= deduction.for_location();
1537+
}
1538+
1539+
let mut num_user_defined_outputs = 0;
1540+
14501541
for output in entry_point.outputs.iter() {
1451-
//TODO: count builtins towards the limit?
1452-
inter_stage_components += match *output {
1453-
Varying::Local { ref iv, .. } => iv.ty.dim.num_components(),
1454-
Varying::BuiltIn(_) => 0,
1542+
match *output {
1543+
Varying::Local { ref iv, location } => {
1544+
if location > max_vertex_shader_output_location {
1545+
// TODO: add diagnostics context for limit deductions
1546+
return Err(StageError::VertexOutputLocationTooLarge {
1547+
location,
1548+
var: iv.clone(),
1549+
limit: self.limits.max_inter_stage_shader_variables,
1550+
deductions: deductions
1551+
.map(MaxInterStageShaderVariablesDeduction)
1552+
.collect(),
1553+
});
1554+
}
1555+
num_user_defined_outputs += 1;
1556+
inter_stage_components += iv.ty.dim.num_components()
1557+
}
1558+
Varying::BuiltIn(_) => {}
14551559
};
14561560

14571561
if let Some(
@@ -1478,6 +1582,16 @@ impl Interface {
14781582
}
14791583
}
14801584
}
1585+
1586+
if num_user_defined_outputs > max_vertex_shader_output_variables {
1587+
return Err(StageError::TooManyUserDefinedVertexOutputs {
1588+
num_found: num_user_defined_outputs,
1589+
limit: self.limits.max_inter_stage_shader_variables - 1,
1590+
deductions: deductions
1591+
.map(MaxInterStageShaderVariablesDeduction)
1592+
.collect(),
1593+
});
1594+
}
14811595
}
14821596
ShaderStageForValidation::Fragment => {
14831597
for output in &entry_point.outputs {
@@ -1617,12 +1731,14 @@ pub fn validate_color_attachment_bytes_per_sample(
16171731

16181732
pub enum ShaderStageForValidation {
16191733
Vertex {
1734+
topology: wgt::PrimitiveTopology,
16201735
compare_function: Option<wgt::CompareFunction>,
16211736
},
16221737
Mesh,
16231738
Fragment,
16241739
Compute,
16251740
Task,
1741+
// TODO: preserve ordering?
16261742
}
16271743

16281744
impl ShaderStageForValidation {

‎wgpu-hal/src/dx12/adapter.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,7 @@ impl super::Adapter {
806806
//
807807
// Source: https://learn.microsoft.com/en-us/windows/win32/direct3d12/root-signature-limits#memory-limits-and-costs
808808
max_immediate_size: 128,
809+
max_inter_stage_shader_variables: 16,
809810
min_uniform_buffer_offset_alignment:
810811
Direct3D12::D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT,
811812
min_storage_buffer_offset_alignment: 4,

‎wgpu-hal/src/gles/adapter.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@ impl super::Adapter {
753753
!0
754754
},
755755
max_immediate_size: super::MAX_IMMEDIATES as u32 * 4,
756+
max_inter_stage_shader_variables: 16,
756757
min_uniform_buffer_offset_alignment,
757758
min_storage_buffer_offset_alignment,
758759
max_inter_stage_shader_components: {

‎wgpu-hal/src/metal/adapter.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,7 @@ impl super::PrivateCapabilities {
11731173
max_vertex_attributes: 31,
11741174
max_vertex_buffer_array_stride: base.max_vertex_buffer_array_stride,
11751175
max_immediate_size: 0x1000,
1176+
max_inter_stage_shader_variables: 16,
11761177
min_uniform_buffer_offset_alignment: self.buffer_alignment as u32,
11771178
min_storage_buffer_offset_alignment: self.buffer_alignment as u32,
11781179
max_inter_stage_shader_components: self.max_varying_components,

‎wgpu-hal/src/noop/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ pub const CAPABILITIES: crate::Capabilities = {
181181
max_buffer_size: ALLOC_MAX_U32 as u64,
182182
max_vertex_attributes: ALLOC_MAX_U32,
183183
max_vertex_buffer_array_stride: ALLOC_MAX_U32,
184+
max_inter_stage_shader_variables: ALLOC_MAX_U32,
184185
min_uniform_buffer_offset_alignment: 1,
185186
min_storage_buffer_offset_alignment: 1,
186187
max_inter_stage_shader_components: ALLOC_MAX_U32,

‎wgpu-hal/src/vulkan/adapter.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,6 +1382,7 @@ impl PhysicalDeviceProperties {
13821382
max_vertex_attributes: limits.max_vertex_input_attributes,
13831383
max_vertex_buffer_array_stride: limits.max_vertex_input_binding_stride,
13841384
max_immediate_size: limits.max_push_constants_size,
1385+
max_inter_stage_shader_variables: 16,
13851386
min_uniform_buffer_offset_alignment: limits.min_uniform_buffer_offset_alignment as u32,
13861387
min_storage_buffer_offset_alignment: limits.min_storage_buffer_offset_alignment as u32,
13871388
max_inter_stage_shader_components: limits

0 commit comments

Comments
 (0)