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
2 changes: 2 additions & 0 deletions cts_runner/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ fails-if(vulkan) webgpu:api,operation,vertex_state,correctness:array_stride_zero
webgpu:api,operation,vertex_state,correctness:setVertexBuffer_offset_and_attribute_offset:*
webgpu:api,validation,buffer,create:*
webgpu:api,validation,buffer,destroy:*
webgpu:api,validation,capability_checks,features,clip_distances:*
fails-if(dx12) webgpu:api,validation,capability_checks,limits,maxBindGroups:setBindGroup,*
// NOTE: Only test some of these `maxInterStageShaderVariables` cases, because of a CTS bug (see
// below). CTS (incorrectly) only deducts once for any set of them being enabled, but it should
Expand Down Expand Up @@ -207,6 +208,7 @@ webgpu:shader,execution,flow_control,return:*
// Many other vertex_buffer_access subtests also passing, but there are too many to enumerate.
// Fails on Metal in CI only, not when running locally.
fails-if(metal) webgpu:shader,execution,robust_access_vertex:vertex_buffer_access:indexed=true;indirect=false;drawCallTestParameter="baseVertex";type="float32x4";additionalBuffers=4;partialLastNumber=false;offsetVertexBuffer=true
webgpu:shader,execution,shader_io,vertex_builtins:outputs,clip_distances:*
webgpu:shader,validation,const_assert,const_assert:*
webgpu:shader,validation,expression,binary,short_circuiting_and_or:array_override:op="%26%26";a_val=1;b_val=1
webgpu:shader,validation,expression,binary,short_circuiting_and_or:invalid_types:*
Expand Down
6 changes: 6 additions & 0 deletions deno_webgpu/webidl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,8 @@ pub enum GPUFeatureName {
Bgra8unormStorage,
#[webidl(rename = "float32-filterable")]
Float32Filterable,
#[webidl(rename = "clip-distances")]
ClipDistances,
#[webidl(rename = "dual-source-blending")]
DualSourceBlending,
#[webidl(rename = "subgroups")]
Expand Down Expand Up @@ -484,6 +486,7 @@ pub fn feature_names_to_features(
GPUFeatureName::Rg11b10ufloatRenderable => Features::RG11B10UFLOAT_RENDERABLE,
GPUFeatureName::Bgra8unormStorage => Features::BGRA8UNORM_STORAGE,
GPUFeatureName::Float32Filterable => Features::FLOAT32_FILTERABLE,
GPUFeatureName::ClipDistances => Features::CLIP_DISTANCES,
GPUFeatureName::DualSourceBlending => Features::DUAL_SOURCE_BLENDING,
GPUFeatureName::Subgroups => Features::SUBGROUP,
GPUFeatureName::TextureFormat16BitNorm => Features::TEXTURE_FORMAT_16BIT_NORM,
Expand Down Expand Up @@ -572,6 +575,9 @@ pub fn features_to_feature_names(
if features.contains(wgpu_types::Features::FLOAT32_FILTERABLE) {
return_features.insert(Float32Filterable);
}
if features.contains(wgpu_types::Features::CLIP_DISTANCES) {
return_features.insert(ClipDistances);
}
if features.contains(wgpu_types::Features::DUAL_SOURCE_BLENDING) {
return_features.insert(DualSourceBlending);
}
Expand Down
182 changes: 170 additions & 12 deletions wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,90 @@ impl fmt::Display for InterfaceVar {
#[derive(Debug)]
enum Varying {
Local { location: u32, iv: InterfaceVar },
BuiltIn(naga::BuiltIn),
BuiltIn(BuiltIn),
}

#[derive(Clone, Debug)]
enum BuiltIn {
Position { invariant: bool },
ViewIndex,
BaseInstance,
BaseVertex,
ClipDistance { array_size: u32 },
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on putting array_size: Option<u32> in the naga type rather then duplicating the whole thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

The Naga type seems like its usage starts for classifying attributes, which only really makes sense as a flat set of unit variants. Naga might want to grow another representation for attributes and all relevant information, but it doesn't currently have that, and that would be strictly more general than what this is trying to accomplish.

CC @jimblandy and/or @teoxoy for thoughts on this, non-blockingly.

CullDistance,
InstanceIndex,
PointSize,
VertexIndex,
DrawID,
FragDepth,
PointCoord,
FrontFacing,
PrimitiveIndex,
Barycentric,
SampleIndex,
SampleMask,
GlobalInvocationId,
LocalInvocationId,
LocalInvocationIndex,
WorkGroupId,
WorkGroupSize,
NumWorkGroups,
NumSubgroups,
SubgroupId,
SubgroupSize,
SubgroupInvocationId,
MeshTaskSize,
CullPrimitive,
PointIndex,
LineIndices,
TriangleIndices,
VertexCount,
Vertices,
PrimitiveCount,
Primitives,
}

impl BuiltIn {
pub fn to_naga(&self) -> naga::BuiltIn {
match self {
&Self::Position { invariant } => naga::BuiltIn::Position { invariant },
Self::ViewIndex => naga::BuiltIn::ViewIndex,
Self::BaseInstance => naga::BuiltIn::BaseInstance,
Self::BaseVertex => naga::BuiltIn::BaseVertex,
Self::ClipDistance { .. } => naga::BuiltIn::ClipDistance,
Self::CullDistance => naga::BuiltIn::CullDistance,
Self::InstanceIndex => naga::BuiltIn::InstanceIndex,
Self::PointSize => naga::BuiltIn::PointSize,
Self::VertexIndex => naga::BuiltIn::VertexIndex,
Self::DrawID => naga::BuiltIn::DrawID,
Self::FragDepth => naga::BuiltIn::FragDepth,
Self::PointCoord => naga::BuiltIn::PointCoord,
Self::FrontFacing => naga::BuiltIn::FrontFacing,
Self::PrimitiveIndex => naga::BuiltIn::PrimitiveIndex,
Self::Barycentric => naga::BuiltIn::Barycentric,
Self::SampleIndex => naga::BuiltIn::SampleIndex,
Self::SampleMask => naga::BuiltIn::SampleMask,
Self::GlobalInvocationId => naga::BuiltIn::GlobalInvocationId,
Self::LocalInvocationId => naga::BuiltIn::LocalInvocationId,
Self::LocalInvocationIndex => naga::BuiltIn::LocalInvocationIndex,
Self::WorkGroupId => naga::BuiltIn::WorkGroupId,
Self::WorkGroupSize => naga::BuiltIn::WorkGroupSize,
Self::NumWorkGroups => naga::BuiltIn::NumWorkGroups,
Self::NumSubgroups => naga::BuiltIn::NumSubgroups,
Self::SubgroupId => naga::BuiltIn::SubgroupId,
Self::SubgroupSize => naga::BuiltIn::SubgroupSize,
Self::SubgroupInvocationId => naga::BuiltIn::SubgroupInvocationId,
Self::MeshTaskSize => naga::BuiltIn::MeshTaskSize,
Self::CullPrimitive => naga::BuiltIn::CullPrimitive,
Self::PointIndex => naga::BuiltIn::PointIndex,
Self::LineIndices => naga::BuiltIn::LineIndices,
Self::TriangleIndices => naga::BuiltIn::TriangleIndices,
Self::VertexCount => naga::BuiltIn::VertexCount,
Self::Vertices => naga::BuiltIn::Vertices,
Self::PrimitiveCount => naga::BuiltIn::PrimitiveCount,
Self::Primitives => naga::BuiltIn::Primitives,
}
}
}

#[allow(unused)]
Expand Down Expand Up @@ -1044,6 +1127,33 @@ impl Interface {
}
return;
}
naga::TypeInner::Array { base, size, stride }
if matches!(
binding,
Some(naga::Binding::BuiltIn(naga::BuiltIn::ClipDistance)),
) =>
{
// NOTE: We should already have validated these in `naga`.
debug_assert_eq!(
&arena[base].inner,
&naga::TypeInner::Scalar(naga::Scalar::F32)
);
debug_assert_eq!(stride, 4);

let naga::ArraySize::Constant(array_size) = size else {
// NOTE: Based on the
// [spec](https://gpuweb.github.io/gpuweb/wgsl/#fixed-footprint-types):
//
// > The only valid use of a fixed-size array with an element count that is an
// > override-expression that is not a const-expression is as a memory view in
// > the workgroup address space.
unreachable!("non-constant array size for `clip_distances`")
};
let array_size = array_size.get();

list.push(Varying::BuiltIn(BuiltIn::ClipDistance { array_size }));
return;
}
ref other => {
//Note: technically this should be at least `log::error`, but
// the reality is - every shader coming from `glslc` outputs an array
Expand Down Expand Up @@ -1071,7 +1181,44 @@ impl Interface {
per_primitive,
},
},
Some(&naga::Binding::BuiltIn(built_in)) => Varying::BuiltIn(built_in),
Some(&naga::Binding::BuiltIn(built_in)) => Varying::BuiltIn(match built_in {
naga::BuiltIn::Position { invariant } => BuiltIn::Position { invariant },
naga::BuiltIn::ViewIndex => BuiltIn::ViewIndex,
naga::BuiltIn::BaseInstance => BuiltIn::BaseInstance,
naga::BuiltIn::BaseVertex => BuiltIn::BaseVertex,
naga::BuiltIn::ClipDistance => unreachable!(),
naga::BuiltIn::CullDistance => BuiltIn::CullDistance,
naga::BuiltIn::InstanceIndex => BuiltIn::InstanceIndex,
naga::BuiltIn::PointSize => BuiltIn::PointSize,
naga::BuiltIn::VertexIndex => BuiltIn::VertexIndex,
naga::BuiltIn::DrawID => BuiltIn::DrawID,
naga::BuiltIn::FragDepth => BuiltIn::FragDepth,
naga::BuiltIn::PointCoord => BuiltIn::PointCoord,
naga::BuiltIn::FrontFacing => BuiltIn::FrontFacing,
naga::BuiltIn::PrimitiveIndex => BuiltIn::PrimitiveIndex,
naga::BuiltIn::Barycentric => BuiltIn::Barycentric,
naga::BuiltIn::SampleIndex => BuiltIn::SampleIndex,
naga::BuiltIn::SampleMask => BuiltIn::SampleMask,
naga::BuiltIn::GlobalInvocationId => BuiltIn::GlobalInvocationId,
naga::BuiltIn::LocalInvocationId => BuiltIn::LocalInvocationId,
naga::BuiltIn::LocalInvocationIndex => BuiltIn::LocalInvocationIndex,
naga::BuiltIn::WorkGroupId => BuiltIn::WorkGroupId,
naga::BuiltIn::WorkGroupSize => BuiltIn::WorkGroupSize,
naga::BuiltIn::NumWorkGroups => BuiltIn::NumWorkGroups,
naga::BuiltIn::NumSubgroups => BuiltIn::NumSubgroups,
naga::BuiltIn::SubgroupId => BuiltIn::SubgroupId,
naga::BuiltIn::SubgroupSize => BuiltIn::SubgroupSize,
naga::BuiltIn::SubgroupInvocationId => BuiltIn::SubgroupInvocationId,
naga::BuiltIn::MeshTaskSize => BuiltIn::MeshTaskSize,
naga::BuiltIn::CullPrimitive => BuiltIn::CullPrimitive,
naga::BuiltIn::PointIndex => BuiltIn::PointIndex,
naga::BuiltIn::LineIndices => BuiltIn::LineIndices,
naga::BuiltIn::TriangleIndices => BuiltIn::TriangleIndices,
naga::BuiltIn::VertexCount => BuiltIn::VertexCount,
naga::BuiltIn::Vertices => BuiltIn::Vertices,
naga::BuiltIn::PrimitiveCount => BuiltIn::PrimitiveCount,
naga::BuiltIn::Primitives => BuiltIn::Primitives,
}),
None => {
log::error!("Missing binding for a varying");
return;
Expand Down Expand Up @@ -1474,10 +1621,10 @@ impl Interface {
});
}
}
Varying::BuiltIn(naga::BuiltIn::PrimitiveIndex) => {
Varying::BuiltIn(BuiltIn::PrimitiveIndex) => {
this_stage_primitive_index = true;
}
Varying::BuiltIn(naga::BuiltIn::DrawID) => {
Varying::BuiltIn(BuiltIn::DrawID) => {
has_draw_id = true;
}
Varying::BuiltIn(_) => {}
Expand All @@ -1499,7 +1646,21 @@ impl Interface {
None
};

let deductions = point_list_deduction.into_iter();
let clip_distance_deductions = entry_point.outputs.iter().filter_map(|output| {
if let &Varying::BuiltIn(BuiltIn::ClipDistance { array_size }) = output {
Some(MaxVertexShaderOutputDeduction::ClipDistance { array_size })
} else {
None
}
});
debug_assert!(
clip_distance_deductions.clone().count() <= 1,
"multiple `clip_distance` outputs found"
);

let deductions = point_list_deduction
.into_iter()
.chain(clip_distance_deductions);

for deduction in deductions.clone() {
// NOTE: Deductions, in the current version of the spec. we implement, do not
Expand Down Expand Up @@ -1534,9 +1695,7 @@ impl Interface {
cmp @ wgt::CompareFunction::Equal | cmp @ wgt::CompareFunction::NotEqual,
) = compare_function
{
if let Varying::BuiltIn(naga::BuiltIn::Position { invariant: false }) =
*output
{
if let Varying::BuiltIn(BuiltIn::Position { invariant: false }) = *output {
log::warn!(
concat!(
"Vertex shader with entry point {} outputs a ",
Expand Down Expand Up @@ -1570,17 +1729,16 @@ impl Interface {
let deductions = entry_point.inputs.iter().filter_map(|output| match output {
Varying::Local { .. } => None,
Varying::BuiltIn(builtin) => {
MaxFragmentShaderInputDeduction::from_inter_stage_builtin(*builtin).or_else(
|| {
MaxFragmentShaderInputDeduction::from_inter_stage_builtin(builtin.to_naga())
.or_else(|| {
unreachable!(
concat!(
"unexpected built-in provided; ",
"{:?} is not used for fragment stage input",
),
builtin
)
},
)
})
}
});

Expand Down
14 changes: 14 additions & 0 deletions wgpu-core/src/validation/shader_io_deductions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,32 @@ pub enum MaxVertexShaderOutputDeduction {
/// When a pipeline's [`crate::pipeline::RenderPipelineDescriptor::primitive`] is set to
/// [`wgt::PrimitiveTopology::PointList`].
PointListPrimitiveTopology,
/// When a clip distances array primitive is used in an output.
ClipDistance { array_size: u32 },
}

impl MaxVertexShaderOutputDeduction {
fn variables_from_clip_distance_slot(num_slots: u32) -> u32 {
num_slots.div_ceil(4)
}
}

impl MaxVertexShaderOutputDeduction {
pub fn for_variables(self) -> u32 {
match self {
Self::PointListPrimitiveTopology => 1,
Self::ClipDistance { array_size } => {
Self::variables_from_clip_distance_slot(array_size)
}
}
}

pub fn for_location(self) -> u32 {
match self {
Self::PointListPrimitiveTopology => 0,
Self::ClipDistance { array_size } => {
Self::variables_from_clip_distance_slot(array_size)
}
}
}
}
Expand Down
Loading