github/wgpu
1
1
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2026-04-22 03:02:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

Skip gl_PerVertex unused builtins in the SPIR-V frontend (#2272)

Browse Source 
Co-authored-by: Jim Blandy <jimb@red-bean.com>
This commit is contained in:
Teodor Tanasoaia
2023-03-15 18:46:25 +01:00
committed by GitHub
parent 7f829c6ac6
commit 63e91faecb
11 changed files with 125 additions and 214 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/arena.rs
View File

@@ -590,6 +590,19 @@ impl<T: Eq + hash::Hash> UniqueArena<T> {
Handle::from_usize(index)
}
/// Replace an old value with a new value.
///
/// # Panics
///
/// - if the old value is not in the arena
/// - if the new value already exists in the arena
pub fn replace(&mut self, old: Handle<T>, new: T) {
let (index, added) = self.set.insert_full(new);
assert!(added && index == self.set.len() - 1);
self.set.swap_remove_index(old.index()).unwrap();
}
/// Return this arena's handle for `value`, if present.
///
/// If this arena already contains an element equal to `value`,

8
src/back/glsl/features.rs
View File

@@ -99,9 +99,8 @@ impl FeaturesManager {
check_feature!(CONSERVATIVE_DEPTH, 130, 300);
check_feature!(NOPERSPECTIVE_QUALIFIER, 130);
check_feature!(SAMPLE_QUALIFIER, 400, 320);
// gl_ClipDistance is supported by core versions > 1.3 and aren't supported by an es versions without extensions
check_feature!(CLIP_DISTANCE, 130, 300);
check_feature!(CULL_DISTANCE, 450, 300);
check_feature!(CLIP_DISTANCE, 130, 300 /* with extension */);
check_feature!(CULL_DISTANCE, 450, 300 /* with extension */);
check_feature!(SAMPLE_VARIABLES, 400, 300);
check_feature!(DYNAMIC_ARRAY_SIZE, 430, 310);
match version {
@@ -197,9 +196,8 @@ impl FeaturesManager {
if (self.0.contains(Features::CLIP_DISTANCE) || self.0.contains(Features::CULL_DISTANCE))
&& version.is_es()
{
// TODO: handle gl_ClipDistance and gl_CullDistance usage in better way
// https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_clip_cull_distance.txt
// writeln!(out, "#extension GL_EXT_clip_cull_distance : require")?;
writeln!(out, "#extension GL_EXT_clip_cull_distance : require")?;
}
if self.0.contains(Features::SAMPLE_VARIABLES) && version.is_es() {

24
src/back/glsl/mod.rs
View File

@@ -2029,27 +2029,11 @@ impl<'a, W: Write> Writer<'a, W> {
};
for (index, member) in members.iter().enumerate() {
// TODO: handle builtin in better way
if let Some(crate::Binding::BuiltIn(builtin)) =
member.binding
if let Some(crate::Binding::BuiltIn(
crate::BuiltIn::PointSize,
)) = member.binding
{
has_point_size |= builtin == crate::BuiltIn::PointSize;
match builtin {
crate::BuiltIn::ClipDistance
| crate::BuiltIn::CullDistance => {
if self.options.version.is_es() {
// Note that gl_ClipDistance and gl_CullDistance are listed in the GLSL ES 3.2 spec but shouldn't
// See https://github.com/KhronosGroup/GLSL/issues/132#issuecomment-685818465
log::warn!(
"{:?} is not part of GLSL ES",
builtin
);
continue;
}
}
_ => {}
}
has_point_size = true;
}
let varying_name = VaryingName {

5
src/back/hlsl/conv.rs
View File

@@ -155,9 +155,6 @@ impl crate::BuiltIn {
Self::ClipDistance => "SV_ClipDistance",
Self::CullDistance => "SV_CullDistance",
Self::InstanceIndex => "SV_InstanceID",
// based on this page https://docs.microsoft.com/en-us/windows/uwp/gaming/glsl-to-hlsl-reference#comparing-opengl-es-20-with-direct3d-11
// No meaning unless you target Direct3D 9
Self::PointSize => "PSIZE",
Self::VertexIndex => "SV_VertexID",
// fragment
Self::FragDepth => "SV_Depth",
@@ -177,7 +174,7 @@ impl crate::BuiltIn {
Self::BaseInstance | Self::BaseVertex | Self::WorkGroupSize => {
return Err(Error::Unimplemented(format!("builtin {self:?}")))
}
Self::ViewIndex | Self::PointCoord => {
Self::PointSize | Self::ViewIndex | Self::PointCoord => {
return Err(Error::Custom(format!("Unsupported builtin {self:?}")))
}
})

34
src/back/msl/writer.rs
View File

@@ -2233,18 +2233,13 @@ impl<W: Write> Writer<W> {
let mut is_first = true;
for (index, member) in members.iter().enumerate() {
match member.binding {
Some(crate::Binding::BuiltIn(crate::BuiltIn::PointSize)) => {
has_point_size = true;
if !context.pipeline_options.allow_point_size {
continue;
}
}
Some(crate::Binding::BuiltIn(crate::BuiltIn::CullDistance)) => {
log::warn!("Ignoring CullDistance built-in");
if let Some(crate::Binding::BuiltIn(crate::BuiltIn::PointSize)) =
member.binding
{
has_point_size = true;
if !context.pipeline_options.allow_point_size {
continue;
}
_ => {}
}
let comma = if is_first { "" } else { "," };
@@ -3563,24 +3558,11 @@ impl<W: Write> Writer<W> {
};
let binding = binding.ok_or(Error::Validation)?;
match *binding {
// Point size is only supported in VS of pipelines with
// point primitive topology.
crate::Binding::BuiltIn(crate::BuiltIn::PointSize) => {
has_point_size = true;
if !pipeline_options.allow_point_size {
continue;
}
}
// Cull Distance is not supported in Metal.
// But we can't return UnsupportedBuiltIn error to user.
// Because otherwise we can't generate msl shader from any glslang SPIR-V shaders.
// glslang generates gl_PerVertex struct with gl_CullDistance builtin inside by default.
crate::Binding::BuiltIn(crate::BuiltIn::CullDistance) => {
log::warn!("Ignoring CullDistance BuiltIn");
if let crate::Binding::BuiltIn(crate::BuiltIn::PointSize) = *binding {
has_point_size = true;
if !pipeline_options.allow_point_size {
continue;
}
_ => {}
}
let array_len = match module.types[ty].inner {

164
src/back/wgsl/writer.rs
View File

@@ -335,11 +335,8 @@ impl<W: Write> Writer<W> {
match *attribute {
Attribute::Location(id) => write!(self.out, "@location({id}) ")?,
Attribute::BuiltIn(builtin_attrib) => {
if let Some(builtin) = builtin_str(builtin_attrib) {
write!(self.out, "@builtin({builtin}) ")?;
} else {
log::warn!("Unsupported builtin attribute: {:?}", builtin_attrib);
}
let builtin = builtin_str(builtin_attrib)?;
write!(self.out, "@builtin({builtin}) ")?;
}
Attribute::Stage(shader_stage) => {
let stage_str = match shader_stage {
@@ -401,14 +398,6 @@ impl<W: Write> Writer<W> {
write!(self.out, " {{")?;
writeln!(self.out)?;
for (index, member) in members.iter().enumerate() {
// Skip struct member with unsupported built in
if let Some(crate::Binding::BuiltIn(built_in)) = member.binding {
if builtin_str(built_in).is_none() {
log::warn!("Skip member with unsupported builtin {:?}", built_in);
continue;
}
}
// The indentation is only for readability
write!(self.out, "{}", back::INDENT)?;
if let Some(ref binding) = member.binding {
@@ -669,22 +658,6 @@ impl<W: Write> Writer<W> {
_ => false,
};
if min_ref_count <= info.ref_count || required_baking_expr {
// If expression contains unsupported builtin we should skip it
if let Expression::Load { pointer } = func_ctx.expressions[handle] {
if let Expression::AccessIndex { base, index } =
func_ctx.expressions[pointer]
{
if access_to_unsupported_builtin(
base,
index,
module,
func_ctx.info,
) {
return Ok(());
}
}
}
Some(format!("{}{}", back::BAKE_PREFIX, handle.index()))
} else {
None
@@ -746,14 +719,6 @@ impl<W: Write> Writer<W> {
writeln!(self.out, "discard;")?
}
Statement::Store { pointer, value } => {
// WGSL does not support all SPIR-V builtins and we should skip it in generated shaders.
// We already skip them when we generate struct type.
// Now we need to find expression that used struct with ignored builtins
if let Expression::AccessIndex { base, index } = func_ctx.expressions[pointer] {
if access_to_unsupported_builtin(base, index, module, func_ctx.info) {
return Ok(());
}
}
write!(self.out, "{level}")?;
let is_atomic = match *func_ctx.info[pointer].ty.inner_with(&module.types) {
@@ -1150,42 +1115,10 @@ impl<W: Write> Writer<W> {
Expression::Compose { ty, ref components } => {
self.write_type(module, ty)?;
write!(self.out, "(")?;
// !spv-in specific notes!
// WGSL does not support all SPIR-V builtins and we should skip it in generated shaders.
// We already skip them when we generate struct type.
// Now we need to find components that used struct with ignored builtins.
// So, why we can't just return the error to a user?
// We can, but otherwise, we can't generate WGSL shader from any glslang SPIR-V shaders.
// glslang generates gl_PerVertex struct with gl_CullDistance, gl_ClipDistance and gl_PointSize builtin inside by default.
// All of them are not supported by WGSL.
// We need to copy components to another vec because we don't know which of them we should write.
let mut components_to_write = Vec::with_capacity(components.len());
for component in components {
let mut skip_component = false;
if let Expression::Load { pointer } = func_ctx.expressions[*component] {
if let Expression::AccessIndex { base, index } =
func_ctx.expressions[pointer]
{
if access_to_unsupported_builtin(base, index, module, func_ctx.info) {
skip_component = true;
}
}
}
if skip_component {
continue;
} else {
components_to_write.push(*component);
}
}
// non spv-in specific notes!
// Real `Expression::Compose` logic generates here.
for (index, component) in components_to_write.iter().enumerate() {
for (index, component) in components.iter().enumerate() {
self.write_expr(module, *component, func_ctx)?;
// Only write a comma if isn't the last element
if index != components_to_write.len().saturating_sub(1) {
if index != components.len().saturating_sub(1) {
// The leading space is for readability only
write!(self.out, ", ")?;
}
@@ -1764,25 +1697,8 @@ impl<W: Write> Writer<W> {
self.write_type(module, ty)?;
write!(self.out, "(")?;
let members = match module.types[ty].inner {
TypeInner::Struct { ref members, .. } => Some(members),
_ => None,
};
// Write the comma separated constants
for (index, constant) in components.iter().enumerate() {
if let Some(&crate::Binding::BuiltIn(built_in)) =
members.and_then(|members| members.get(index)?.binding.as_ref())
{
if builtin_str(built_in).is_none() {
log::warn!(
"Skip constant for struct member with unsupported builtin {:?}",
built_in
);
continue;
}
}
self.write_constant(module, *constant)?;
// Only write a comma if isn't the last element
if index != components.len().saturating_sub(1) {
@@ -1869,26 +1785,34 @@ impl<W: Write> Writer<W> {
}
}
const fn builtin_str(built_in: crate::BuiltIn) -> Option<&'static str> {
fn builtin_str(built_in: crate::BuiltIn) -> Result<&'static str, Error> {
use crate::BuiltIn as Bi;
match built_in {
Bi::VertexIndex => Some("vertex_index"),
Bi::InstanceIndex => Some("instance_index"),
Bi::Position { .. } => Some("position"),
Bi::FrontFacing => Some("front_facing"),
Bi::FragDepth => Some("frag_depth"),
Bi::LocalInvocationId => Some("local_invocation_id"),
Bi::LocalInvocationIndex => Some("local_invocation_index"),
Bi::GlobalInvocationId => Some("global_invocation_id"),
Bi::WorkGroupId => Some("workgroup_id"),
Bi::NumWorkGroups => Some("num_workgroups"),
Bi::SampleIndex => Some("sample_index"),
Bi::SampleMask => Some("sample_mask"),
Bi::PrimitiveIndex => Some("primitive_index"),
Bi::ViewIndex => Some("view_index"),
_ => None,
}
Ok(match built_in {
Bi::VertexIndex => "vertex_index",
Bi::InstanceIndex => "instance_index",
Bi::Position { .. } => "position",
Bi::FrontFacing => "front_facing",
Bi::FragDepth => "frag_depth",
Bi::LocalInvocationId => "local_invocation_id",
Bi::LocalInvocationIndex => "local_invocation_index",
Bi::GlobalInvocationId => "global_invocation_id",
Bi::WorkGroupId => "workgroup_id",
Bi::NumWorkGroups => "num_workgroups",
Bi::SampleIndex => "sample_index",
Bi::SampleMask => "sample_mask",
Bi::PrimitiveIndex => "primitive_index",
Bi::ViewIndex => "view_index",
Bi::BaseInstance
| Bi::BaseVertex
| Bi::ClipDistance
| Bi::CullDistance
| Bi::PointSize
| Bi::PointCoord
| Bi::WorkGroupSize => {
return Err(Error::Custom(format!("Unsupported builtin {built_in:?}")))
}
})
}
const fn image_dimension_str(dim: crate::ImageDimension) -> &'static str {
@@ -2032,31 +1956,3 @@ fn map_binding_to_attribute(
},
}
}
/// Helper function that check that expression don't access to structure member with unsupported builtin.
fn access_to_unsupported_builtin(
expr: Handle<crate::Expression>,
index: u32,
module: &Module,
info: &valid::FunctionInfo,
) -> bool {
let base_ty_res = &info[expr].ty;
let resolved = base_ty_res.inner_with(&module.types);
if let TypeInner::Pointer {
base: pointer_base_handle,
..
} = *resolved
{
// Let's check that we try to access a struct member with unsupported built-in and skip it.
if let TypeInner::Struct { ref members, .. } = module.types[pointer_base_handle].inner {
if let Some(crate::Binding::BuiltIn(built_in)) = members[index as usize].binding {
if builtin_str(built_in).is_none() {
log::warn!("Skip component with unsupported builtin {:?}", built_in);
return true;
}
}
}
}
false
}

4
src/front/glsl/variables.rs
View File

@@ -200,8 +200,8 @@ impl Frontend {
stride: 4,
},
builtin: match name {
"gl_ClipDistance" => BuiltIn::PointSize,
"gl_CullDistance" => BuiltIn::FragDepth,
"gl_ClipDistance" => BuiltIn::ClipDistance,
"gl_CullDistance" => BuiltIn::CullDistance,
_ => unreachable!(),
},
mutable: self.meta.stage == ShaderStage::Vertex,

48
src/front/spv/function.rs
View File

@@ -370,22 +370,50 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
let expr_handle = function
.expressions
.append(crate::Expression::GlobalVariable(lvar.handle), span);
// Cull problematic builtins of gl_PerVertex.
// See the docs for `Frontend::gl_per_vertex_builtin_access`.
{
let ty = &module.types[result.ty];
match ty.inner {
crate::TypeInner::Struct {
members: ref original_members,
span,
} if ty.name.as_deref() == Some("gl_PerVertex") => {
let mut new_members = original_members.clone();
for member in &mut new_members {
if let Some(crate::Binding::BuiltIn(built_in)) = member.binding
{
if !self.gl_per_vertex_builtin_access.contains(&built_in) {
member.binding = None
}
}
}
if &new_members != original_members {
module.types.replace(
result.ty,
crate::Type {
name: ty.name.clone(),
inner: crate::TypeInner::Struct {
members: new_members,
span,
},
},
);
}
}
_ => {}
}
}
match module.types[result.ty].inner {
crate::TypeInner::Struct {
members: ref sub_members,
..
} => {
for (index, sm) in sub_members.iter().enumerate() {
match sm.binding {
Some(crate::Binding::BuiltIn(built_in)) => {
// Cull unused builtins to preserve performances
if !self.builtin_usage.contains(&built_in) {
continue;
}
}
// unrecognized binding, skip
None => continue,
_ => {}
if sm.binding.is_none() {
continue;
}
let mut sm = sm.clone();

27
src/front/spv/mod.rs
View File

@@ -560,9 +560,15 @@ pub struct Frontend<I> {
std::hash::BuildHasherDefault<rustc_hash::FxHasher>,
>,
/// Tracks usage of builtins, used to cull unused builtins since they can
/// have serious performance implications.
builtin_usage: FastHashSet<crate::BuiltIn>,
/// Tracks access to gl_PerVertex's builtins, it is used to cull unused builtins since initializing those can
/// affect performance and the mere presence of some of these builtins might cause backends to error since they
/// might be unsupported.
///
/// The problematic builtins are: PointSize, ClipDistance and CullDistance.
///
/// glslang declares those by default even though they are never written to
/// (see <https://github.com/KhronosGroup/glslang/issues/1868>)
gl_per_vertex_builtin_access: FastHashSet<crate::BuiltIn>,
}
impl<I: Iterator<Item = u32>> Frontend<I> {
@@ -596,7 +602,7 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
index_constants: Vec::new(),
index_constant_expressions: Vec::new(),
switch_cases: indexmap::IndexMap::default(),
builtin_usage: FastHashSet::default(),
gl_per_vertex_builtin_access: FastHashSet::default(),
}
}
@@ -1480,7 +1486,8 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
log::trace!("\t\t\tlooking up type {:?}", acex.type_id);
let type_lookup = self.lookup_type.lookup(acex.type_id)?;
acex = match ctx.type_arena[type_lookup.handle].inner {
let ty = &ctx.type_arena[type_lookup.handle];
acex = match ty.inner {
// can only index a struct with a constant
crate::TypeInner::Struct { ref members, .. } => {
let index = index_maybe
@@ -1498,10 +1505,12 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
span,
);
if let Some(crate::Binding::BuiltIn(built_in)) =
members[index as usize].binding
{
self.builtin_usage.insert(built_in);
if ty.name.as_deref() == Some("gl_PerVertex") {
if let Some(crate::Binding::BuiltIn(built_in)) =
members[index as usize].binding
{
self.gl_per_vertex_builtin_access.insert(built_in);
}
}
AccessExpression {

7
tests/out/hlsl/quad-vert.hlsl
View File

@@ -1,9 +1,10 @@
struct gl_PerVertex {
float4 gl_Position : SV_Position;
float gl_PointSize : PSIZE;
float gl_ClipDistance[1] : SV_ClipDistance;
float gl_CullDistance[1] : SV_CullDistance;
float gl_PointSize;
float gl_ClipDistance[1];
float gl_CullDistance[1];
int _end_pad_0;
};
struct type_9 {

5
tests/out/wgsl/quad-vert.wgsl
View File

@@ -1,5 +1,8 @@
struct gl_PerVertex {
@builtin(position) gl_Position: vec4<f32>,
gl_PointSize: f32,
gl_ClipDistance: array<f32,1u>,
gl_CullDistance: array<f32,1u>,
}
struct VertexOutput {
@@ -9,7 +12,7 @@ struct VertexOutput {
var<private> v_uv: vec2<f32>;
var<private> a_uv_1: vec2<f32>;
var<private> perVertexStruct: gl_PerVertex = gl_PerVertex(vec4<f32>(0.0, 0.0, 0.0, 1.0), );
var<private> perVertexStruct: gl_PerVertex = gl_PerVertex(vec4<f32>(0.0, 0.0, 0.0, 1.0), 1.0, array<f32,1u>(0.0), array<f32,1u>(0.0));
var<private> a_pos_1: vec2<f32>;
fn main_1() {