diff --git a/src/back/glsl/mod.rs b/src/back/glsl/mod.rs index f4e59f1796..c7c5a194ec 100644 --- a/src/back/glsl/mod.rs +++ b/src/back/glsl/mod.rs @@ -539,17 +539,17 @@ impl<'a, W: Write> Writer<'a, W> { self.collect_reflection_info() } - /// Helper method used to write non image/sampler types + /// Helper method used to write value types /// /// # Notes /// Adds no trailing or leading whitespace /// /// # Panics - /// - If type is either a image or sampler + /// - If type is either a image, a sampler, a pointer, or a struct /// - If it's an Array with a [`ArraySize::Constant`](crate::ArraySize::Constant) with a /// constant that isn't [`Uint`](crate::ConstantInner::Uint) - fn write_type(&mut self, ty: Handle) -> BackendResult { - match self.module.types[ty].inner { + fn write_value_type(&mut self, inner: &TypeInner) -> BackendResult { + match *inner { // Scalars are simple we just get the full name from `glsl_scalar` TypeInner::Scalar { kind, width } | TypeInner::ValuePointer { @@ -587,8 +587,6 @@ impl<'a, W: Write> Writer<'a, W> { columns as u8, rows as u8 )?, - // glsl has no pointer types so just write types as normal and loads are skipped - TypeInner::Pointer { base, .. } => self.write_type(base)?, // GLSL arrays are written as `type name[size]` // Current code is written arrays only as `[size]` // Base `type` and `name` should be written outside @@ -613,24 +611,45 @@ impl<'a, W: Write> Writer<'a, W> { write!(self.out, "]")? } + // Panic if either Image, Sampler, Pointer, or a Struct is being written + // + // Write all variants instead of `_` so that if new variants are added a + // no exhaustiveness error is thrown + TypeInner::Pointer { .. } + | TypeInner::Struct { .. } + | TypeInner::Image { .. } + | TypeInner::Sampler { .. } => unreachable!(), + } + + Ok(()) + } + + /// Helper method used to write non image/sampler types + /// + /// # Notes + /// Adds no trailing or leading whitespace + /// + /// # Panics + /// - If type is either a image or sampler + /// - If it's an Array with a [`ArraySize::Constant`](crate::ArraySize::Constant) with a + /// constant that isn't [`Uint`](crate::ConstantInner::Uint) + fn write_type(&mut self, ty: Handle) -> BackendResult { + match self.module.types[ty].inner { + // glsl has no pointer types so just write types as normal and loads are skipped + TypeInner::Pointer { base, .. } => self.write_type(base), TypeInner::Struct { block: true, ref members, - } => self.write_struct(true, ty, members)?, + } => self.write_struct(true, ty, members), // glsl structs are written as just the struct name if it isn't a block TypeInner::Struct { block: false, .. } => { // Get the struct name let name = &self.names[&NameKey::Type(ty)]; - write!(self.out, "{}", name)? + write!(self.out, "{}", name)?; + Ok(()) } - // Panic if either Image or Sampler is being written - // - // Write all variants instead of `_` so that if new variants are added a - // no exhaustiveness error is thrown - TypeInner::Image { .. } | TypeInner::Sampler { .. } => unreachable!(), + ref other => self.write_value_type(other), } - - Ok(()) } /// Helper method to write a image type @@ -794,6 +813,8 @@ impl<'a, W: Write> Writer<'a, W> { typifier: &typifier, }; + self.cached_expressions.clear(); + // Write the function header // // glsl headers are the same as in c: @@ -1041,11 +1062,11 @@ impl<'a, W: Write> Writer<'a, W> { // This is where we can generate intermediate constants for some expression types. Statement::Emit(ref range) => { for handle in range.clone() { - if let Ok(ty_handle) = ctx.typifier.get_handle(handle) { - let min_ref_count = ctx.expressions[handle].bake_ref_count(); - if min_ref_count <= ctx.info[handle].ref_count { - write!(self.out, "{}", INDENT.repeat(indent))?; - match self.module.types[ty_handle].inner { + let min_ref_count = ctx.expressions[handle].bake_ref_count(); + if min_ref_count <= ctx.info[handle].ref_count { + write!(self.out, "{}", INDENT.repeat(indent))?; + match ctx.typifier.get_handle(handle) { + Ok(ty_handle) => match self.module.types[ty_handle].inner { TypeInner::Struct { .. } => { let ty_name = &self.names[&NameKey::Type(ty_handle)]; write!(self.out, "{}", ty_name)?; @@ -1053,13 +1074,16 @@ impl<'a, W: Write> Writer<'a, W> { _ => { self.write_type(ty_handle)?; } - }; - let name = format!("_expr{}", handle.index()); - write!(self.out, " {} = ", name)?; - self.write_expr(handle, ctx)?; - writeln!(self.out, ";")?; - self.cached_expressions.insert(handle, name); + }, + Err(inner) => { + self.write_value_type(inner)?; + } } + let name = format!("_expr{}", handle.index()); + write!(self.out, " {} = ", name)?; + self.write_expr(handle, ctx)?; + writeln!(self.out, ";")?; + self.cached_expressions.insert(handle, name); } } } diff --git a/src/back/mod.rs b/src/back/mod.rs index 1b0d826e29..c7d513a66d 100644 --- a/src/back/mod.rs +++ b/src/back/mod.rs @@ -12,13 +12,18 @@ pub mod spv; impl crate::Expression { /// Returns the ref count, upon reaching which this expression /// should be considered for baking. + /// + /// Note: we have to cache any expressions that depend on the control flow, + /// or otherwise they may be moved into a non-uniform contol flow, accidentally. #[allow(dead_code)] fn bake_ref_count(&self) -> usize { match *self { // accesses are never cached, only loads are crate::Expression::Access { .. } | crate::Expression::AccessIndex { .. } => !0, - // image operations look better when isolated + // sampling may use the control flow, and image ops look better by themselves crate::Expression::ImageSample { .. } | crate::Expression::ImageLoad { .. } => 1, + // derivatives use the control flow + crate::Expression::Derivative { .. } => 1, // cache expressions that are referenced multiple times _ => 2, } diff --git a/tests/out/quad-Fragment.glsl.snap b/tests/out/quad-Fragment.glsl.snap index b764907389..7b875449f5 100644 --- a/tests/out/quad-Fragment.glsl.snap +++ b/tests/out/quad-Fragment.glsl.snap @@ -13,10 +13,11 @@ uniform highp sampler2D _group_0_binding_0; out vec4 _location_0; void main() { - if((texture(_group_0_binding_0, vec2(_location_0_vs))[3] == 0.0)) { + vec4 _expr9 = texture(_group_0_binding_0, vec2(_location_0_vs)); + if((_expr9[3] == 0.0)) { discard; } - _location_0 = (texture(_group_0_binding_0, vec2(_location_0_vs))[3] * texture(_group_0_binding_0, vec2(_location_0_vs))); + _location_0 = (_expr9[3] * _expr9); return; } diff --git a/tests/out/shadow-Fragment.glsl.snap b/tests/out/shadow-Fragment.glsl.snap index 6624e149e7..df8300b46a 100644 --- a/tests/out/shadow-Fragment.glsl.snap +++ b/tests/out/shadow-Fragment.glsl.snap @@ -32,7 +32,9 @@ float fetch_shadow(uint light_id, vec4 homogeneous_coords) { if((homogeneous_coords[3] <= 0.0)) { return 1.0; } - return textureGrad(_group_0_binding_2, vec4((((vec2(homogeneous_coords[0], homogeneous_coords[1]) * vec2(0.5, -0.5)) * (1.0 / homogeneous_coords[3])) + vec2(0.5, 0.5)), int(light_id), (homogeneous_coords[2] * (1.0 / homogeneous_coords[3]))), vec2(0, 0), vec2(0,0)); + float _expr15 = (1.0 / homogeneous_coords[3]); + float _expr28 = textureGrad(_group_0_binding_2, vec4((((vec2(homogeneous_coords[0], homogeneous_coords[1]) * vec2(0.5, -0.5)) * _expr15) + vec2(0.5, 0.5)), int(light_id), (homogeneous_coords[2] * _expr15)), vec2(0, 0), vec2(0,0)); + return _expr28; } void main() { @@ -51,3 +53,5 @@ void main() { _location_0 = vec4(color1, 1.0); return; } + + diff --git a/tests/out/skybox-Fragment.glsl.snap b/tests/out/skybox-Fragment.glsl.snap index da1ebab576..6d949cd3cc 100644 --- a/tests/out/skybox-Fragment.glsl.snap +++ b/tests/out/skybox-Fragment.glsl.snap @@ -13,6 +13,9 @@ in vec3 _location_0_vs; out vec4 _location_0; void main() { - _location_0 = texture(_group_0_binding_1, vec3(_location_0_vs)); + vec4 _expr9 = texture(_group_0_binding_1, vec3(_location_0_vs)); + _location_0 = _expr9; return; } + +