From 0ff3fa32153590f9b4c4476ee1343db6bc248ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= Date: Fri, 18 Mar 2022 22:10:35 +0000 Subject: [PATCH] glsl-in: Allow nested accesses in lhs positions Also fixes access to runtime sized arrays behind named blocks --- src/front/glsl/ast.rs | 2 + src/front/glsl/context.rs | 78 ++++++++++++++++++--------- src/front/glsl/variables.rs | 38 +++++++++---- tests/in/glsl/buffer.frag | 22 ++++++++ tests/out/wgsl/bevy-pbr-frag.wgsl | 26 ++++----- tests/out/wgsl/buffer-frag.wgsl | 37 +++++++++++++ tests/out/wgsl/declarations-vert.wgsl | 12 ++--- tests/out/wgsl/fma-frag.wgsl | 18 +++---- 8 files changed, 168 insertions(+), 65 deletions(-) create mode 100644 tests/in/glsl/buffer.frag create mode 100644 tests/out/wgsl/buffer-frag.wgsl diff --git a/src/front/glsl/ast.rs b/src/front/glsl/ast.rs index 0d5041306c..95faed6e7c 100644 --- a/src/front/glsl/ast.rs +++ b/src/front/glsl/ast.rs @@ -97,7 +97,9 @@ pub struct EntryArg { #[derive(Debug, Clone)] pub struct VariableReference { pub expr: Handle, + /// Wether the variable is of a pointer type (and needs loading) or not pub load: bool, + /// Wether the value of the variable can be changed or not pub mutable: bool, pub constant: Option<(Handle, Handle)>, pub entry_arg: Option, diff --git a/src/front/glsl/context.rs b/src/front/glsl/context.rs index 781c9c7677..827890b037 100644 --- a/src/front/glsl/context.rs +++ b/src/front/glsl/context.rs @@ -16,7 +16,7 @@ use crate::{ use std::{convert::TryFrom, ops::Index}; /// The position at which an expression is, used while lowering -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum ExprPos { /// The expression is in the left hand side of an assignment Lhs, @@ -24,18 +24,18 @@ pub enum ExprPos { Rhs, /// The expression is an array being indexed, needed to allow constant /// arrays to be dinamically indexed - ArrayBase { + AccessBase { /// The index is a constant constant_index: bool, }, } impl ExprPos { - /// Returns an lhs position if the current position is lhs otherwise ArrayBase - fn maybe_array_base(&self, constant_index: bool) -> Self { + /// Returns an lhs position if the current position is lhs otherwise AccessBase + fn maybe_access_base(&self, constant_index: bool) -> Self { match *self { ExprPos::Lhs => *self, - _ => ExprPos::ArrayBase { constant_index }, + _ => ExprPos::AccessBase { constant_index }, } } } @@ -492,6 +492,8 @@ impl Context { ) -> Result<(Option>, Span)> { let HirExpr { ref kind, meta } = stmt.hir_exprs[expr]; + log::debug!("Lowering {:?}", expr); + let handle = match *kind { HirExprKind::Access { base, index } => { let (index, index_meta) = @@ -508,7 +510,7 @@ impl Context { stmt, parser, base, - pos.maybe_array_base(maybe_constant_index.is_some()), + pos.maybe_access_base(maybe_constant_index.is_some()), body, )? .0; @@ -551,18 +553,20 @@ impl Context { pointer } HirExprKind::Select { base, ref field } => { - let base = self.lower_expect_inner(stmt, parser, base, pos, body)?.0; + let base = self + .lower_expect_inner(stmt, parser, base, pos.maybe_access_base(true), body)? + .0; - parser.field_selection(self, ExprPos::Lhs == pos, body, base, field, meta)? + parser.field_selection(self, pos, body, base, field, meta)? } HirExprKind::Constant(constant) if pos != ExprPos::Lhs => { self.add_expression(Expression::Constant(constant), meta, body) } HirExprKind::Binary { left, op, right } if pos != ExprPos::Lhs => { let (mut left, left_meta) = - self.lower_expect_inner(stmt, parser, left, pos, body)?; + self.lower_expect_inner(stmt, parser, left, ExprPos::Rhs, body)?; let (mut right, right_meta) = - self.lower_expect_inner(stmt, parser, right, pos, body)?; + self.lower_expect_inner(stmt, parser, right, ExprPos::Rhs, body)?; match op { BinaryOperator::ShiftLeft | BinaryOperator::ShiftRight => self @@ -1003,7 +1007,9 @@ impl Context { } } HirExprKind::Unary { op, expr } if pos != ExprPos::Lhs => { - let expr = self.lower_expect_inner(stmt, parser, expr, pos, body)?.0; + let expr = self + .lower_expect_inner(stmt, parser, expr, ExprPos::Rhs, body)? + .0; self.add_expression(Expression::Unary { op, expr }, meta, body) } @@ -1020,20 +1026,29 @@ impl Context { var.expr } - ExprPos::ArrayBase { - constant_index: false, - } => { - if let Some((constant, ty)) = var.constant { - let local = self.locals.append( - LocalVariable { - name: None, - ty, - init: Some(constant), - }, - Span::default(), - ); + ExprPos::AccessBase { constant_index } => { + // If the index isn't constant all accesses backed by a constant base need + // to be done trough a proxy local variable, since constants have a non + // pointer type which is required for dynamic indexing + if !constant_index { + if let Some((constant, ty)) = var.constant { + let local = self.locals.append( + LocalVariable { + name: None, + ty, + init: Some(constant), + }, + Span::default(), + ); - self.add_expression(Expression::LocalVariable(local), Span::default(), body) + self.add_expression( + Expression::LocalVariable(local), + Span::default(), + body, + ) + } else { + var.expr + } } else { var.expr } @@ -1091,9 +1106,12 @@ impl Context { let (mut value, value_meta) = self.lower_expect_inner(stmt, parser, value, ExprPos::Rhs, body)?; - let scalar_components = self.expr_scalar_components(parser, pointer, ptr_meta)?; + let ty = match *parser.resolve_type(self, pointer, ptr_meta)? { + TypeInner::Pointer { base, .. } => &parser.module.types[base].inner, + ref ty => ty, + }; - if let Some((kind, width)) = scalar_components { + if let Some((kind, width)) = scalar_components(ty) { self.implicit_conversion(parser, &mut value, value_meta, kind, width)?; } @@ -1216,6 +1234,14 @@ impl Context { } }; + log::trace!( + "Lowered {:?}\n\tKind = {:?}\n\tPos = {:?}\n\tResult = {:?}", + expr, + kind, + pos, + handle + ); + Ok((Some(handle), meta)) } diff --git a/src/front/glsl/variables.rs b/src/front/glsl/variables.rs index fb5f3b3625..4b69abc05d 100644 --- a/src/front/glsl/variables.rs +++ b/src/front/glsl/variables.rs @@ -1,6 +1,6 @@ use super::{ ast::*, - context::Context, + context::{Context, ExprPos}, error::{Error, ErrorKind}, Parser, Result, Span, }; @@ -231,7 +231,7 @@ impl Parser { pub(crate) fn field_selection( &mut self, ctx: &mut Context, - lhs: bool, + pos: ExprPos, body: &mut Block, expression: Handle, name: &str, @@ -250,14 +250,21 @@ impl Parser { kind: ErrorKind::UnknownField(name.into()), meta, })?; - Ok(ctx.add_expression( + let pointer = ctx.add_expression( Expression::AccessIndex { base: expression, index: index as u32, }, meta, body, - )) + ); + + Ok(match pos { + ExprPos::Rhs if is_pointer => { + ctx.add_expression(Expression::Load { pointer }, meta, body) + } + _ => pointer, + }) } // swizzles (xyzw, rgba, stpq) TypeInner::Vector { size, .. } => { @@ -277,7 +284,7 @@ impl Parser { .or_else(|| check_swizzle_components("stpq")); if let Some(components) = components { - if lhs { + if let ExprPos::Lhs = pos { let not_unique = (1..components.len()) .any(|i| components[i..].contains(&components[i - 1])); if not_unique { @@ -315,14 +322,23 @@ impl Parser { } let size = match components.len() { + // Swizzles with just one component are accesses and not swizzles 1 => { - // only single element swizzle, like pos.y, just return that component. - if lhs { - // Because of possible nested swizzles, like pos.xy.x, we have to unwrap the potential load expr. - if let Expression::Load { ref pointer } = ctx[expression] { - expression = *pointer; + match pos { + // If the position is in the right hand side and the base + // vector is a pointer, load it, otherwise the swizzle would + // produce a pointer + ExprPos::Rhs if is_pointer => { + expression = ctx.add_expression( + Expression::Load { + pointer: expression, + }, + meta, + body, + ); } - } + _ => {} + }; return Ok(ctx.add_expression( Expression::AccessIndex { base: expression, diff --git a/tests/in/glsl/buffer.frag b/tests/in/glsl/buffer.frag new file mode 100644 index 0000000000..dc50aae2ed --- /dev/null +++ b/tests/in/glsl/buffer.frag @@ -0,0 +1,22 @@ +#version 450 + +layout(set = 0, binding = 0) buffer testBufferBlock { + uint[] data; +} testBuffer; + +layout(set = 0, binding = 1) writeonly buffer testBufferWriteOnlyBlock { + uint[] data; +} testBufferWriteOnly; + +layout(set = 0, binding = 2) readonly buffer testBufferReadOnlyBlock { + uint[] data; +} testBufferReadOnly; + +void main() { + uint a = testBuffer.data[0]; + testBuffer.data[1] = 2; + + testBufferWriteOnly.data[1] = 2; + + uint b = testBufferReadOnly.data[0]; +} diff --git a/tests/out/wgsl/bevy-pbr-frag.wgsl b/tests/out/wgsl/bevy-pbr-frag.wgsl index 5d04450756..a1f49dd16b 100644 --- a/tests/out/wgsl/bevy-pbr-frag.wgsl +++ b/tests/out/wgsl/bevy-pbr-frag.wgsl @@ -498,21 +498,21 @@ fn point_light(light: PointLight, roughness_8: f32, NdotV: f32, N: vec3, V_ R_1 = R; F0_1 = F0_; diffuseColor_1 = diffuseColor; - let _e56 = light_1; + let _e57 = light_1.pos; let _e59 = v_WorldPosition_1; - light_to_frag = (_e56.pos.xyz - _e59.xyz); + light_to_frag = (_e57.xyz - _e59.xyz); let _e65 = light_to_frag; let _e66 = light_to_frag; distance_square = dot(_e65, _e66); - let _e70 = light_1; + let _e71 = light_1.lightParams; let _e73 = distance_square; - let _e74 = light_1; - let _e77 = getDistanceAttenuation(_e73, _e74.lightParams.x); + let _e75 = light_1.lightParams; + let _e77 = getDistanceAttenuation(_e73, _e75.x); rangeAttenuation = _e77; let _e79 = roughness_9; a_1 = _e79; - let _e81 = light_1; - radius = _e81.lightParams.y; + let _e82 = light_1.lightParams; + radius = _e82.y; let _e87 = light_to_frag; let _e88 = R_1; let _e90 = R_1; @@ -611,10 +611,10 @@ fn point_light(light: PointLight, roughness_8: f32, NdotV: f32, N: vec3, V_ diffuse = (_e302 * _e311); let _e314 = diffuse; let _e315 = specular_1; - let _e317 = light_1; + let _e318 = light_1.color; let _e321 = rangeAttenuation; let _e322 = NoL_6; - return (((_e314 + _e315) * _e317.color.xyz) * (_e321 * _e322)); + return (((_e314 + _e315) * _e318.xyz) * (_e321 * _e322)); } fn dir_light(light_2: DirectionalLight, roughness_10: f32, NdotV_2: f32, normal: vec3, view: vec3, R_2: vec3, F0_2: vec3, diffuseColor_2: vec3) -> vec3 { @@ -643,8 +643,8 @@ fn dir_light(light_2: DirectionalLight, roughness_10: f32, NdotV_2: f32, normal: R_3 = R_2; F0_3 = F0_2; diffuseColor_3 = diffuseColor_2; - let _e56 = light_3; - incident_light = _e56.direction.xyz; + let _e57 = light_3.direction; + incident_light = _e57.xyz; let _e60 = incident_light; let _e61 = view_1; let _e63 = incident_light; @@ -684,9 +684,9 @@ fn dir_light(light_2: DirectionalLight, roughness_10: f32, NdotV_2: f32, normal: specular_2 = _e146; let _e148 = specular_2; let _e149 = diffuse_1; - let _e151 = light_3; + let _e152 = light_3.color; let _e155 = NoL_7; - return (((_e148 + _e149) * _e151.color.xyz) * _e155); + return (((_e148 + _e149) * _e152.xyz) * _e155); } fn main_1() { diff --git a/tests/out/wgsl/buffer-frag.wgsl b/tests/out/wgsl/buffer-frag.wgsl new file mode 100644 index 0000000000..455fbb7af9 --- /dev/null +++ b/tests/out/wgsl/buffer-frag.wgsl @@ -0,0 +1,37 @@ +struct testBufferBlock { + data: array, +} + +struct testBufferWriteOnlyBlock { + data: array, +} + +struct testBufferReadOnlyBlock { + data: array, +} + +@group(0) @binding(0) +var testBuffer: testBufferBlock; +@group(0) @binding(1) +var testBufferWriteOnly: testBufferWriteOnlyBlock; +@group(0) @binding(2) +var testBufferReadOnly: testBufferReadOnlyBlock; + +fn main_1() { + var a: u32; + var b: u32; + + let _e12 = testBuffer.data[0]; + a = _e12; + testBuffer.data[1] = u32(2); + testBufferWriteOnly.data[1] = u32(2); + let _e27 = testBufferReadOnly.data[0]; + b = _e27; + return; +} + +@stage(fragment) +fn main() { + main_1(); + return; +} diff --git a/tests/out/wgsl/declarations-vert.wgsl b/tests/out/wgsl/declarations-vert.wgsl index cfd2528f39..0c5feecb64 100644 --- a/tests/out/wgsl/declarations-vert.wgsl +++ b/tests/out/wgsl/declarations-vert.wgsl @@ -34,12 +34,12 @@ fn main_1() { var a_1: f32; var b: f32; - let _e34 = in_array_2; - from_input_array = _e34[1]; - let _e39 = array_2d; - a_1 = _e39[0][0]; - let _e50 = array_toomanyd; - b = _e50[0][0][0][0][0][0][0]; + let _e35 = in_array_2[1]; + from_input_array = _e35; + let _e41 = array_2d[0][0]; + a_1 = _e41; + let _e57 = array_toomanyd[0][0][0][0][0][0][0]; + b = _e57; out_array[0] = vec4(2.0); return; } diff --git a/tests/out/wgsl/fma-frag.wgsl b/tests/out/wgsl/fma-frag.wgsl index 3c2233abfe..2019e48ad1 100644 --- a/tests/out/wgsl/fma-frag.wgsl +++ b/tests/out/wgsl/fma-frag.wgsl @@ -16,18 +16,18 @@ fn Fma(d: ptr, m: Mat4x3_, s: f32) { m_1 = m; s_1 = s; - let _e6 = (*d); - let _e8 = m_1; + let _e7 = (*d).mx; + let _e9 = m_1.mx; let _e10 = s_1; - (*d).mx = (_e6.mx + (_e8.mx * _e10)); - let _e14 = (*d); - let _e16 = m_1; + (*d).mx = (_e7 + (_e9 * _e10)); + let _e15 = (*d).my; + let _e17 = m_1.my; let _e18 = s_1; - (*d).my = (_e14.my + (_e16.my * _e18)); - let _e22 = (*d); - let _e24 = m_1; + (*d).my = (_e15 + (_e17 * _e18)); + let _e23 = (*d).mz; + let _e25 = m_1.mz; let _e26 = s_1; - (*d).mz = (_e22.mz + (_e24.mz * _e26)); + (*d).mz = (_e23 + (_e25 * _e26)); return; }