From ead052b7734b767ef358b2f1d7e573968039cd13 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Fri, 2 Apr 2021 00:16:11 -0400 Subject: [PATCH] [spv-in] add a flag to adjust the coordinate space --- bin/convert.rs | 3 +++ src/front/spv/function.rs | 51 +++++++++++++++++++----------------- src/front/spv/mod.rs | 15 ++++++++++- tests/out/quad-vert.msl.snap | 1 - tests/snapshots.rs | 11 +++++--- 5 files changed, 51 insertions(+), 30 deletions(-) diff --git a/bin/convert.rs b/bin/convert.rs index 402b21e342..d52468110b 100644 --- a/bin/convert.rs +++ b/bin/convert.rs @@ -5,6 +5,8 @@ use std::{env, error::Error, path::Path}; #[derive(Default)] struct Parameters { + #[cfg(feature = "spv-in")] + spv_adjust_coordinate_space: bool, #[cfg(feature = "spv-in")] spv_flow_dump_prefix: Option, #[cfg(feature = "spv-out")] @@ -102,6 +104,7 @@ fn main() { #[cfg(feature = "spv-in")] "spv" => { let options = naga::front::spv::Options { + adjust_coordinate_space: params.spv_adjust_coordinate_space, flow_graph_dump_prefix: params.spv_flow_dump_prefix.map(std::path::PathBuf::from), }; let input = fs::read(input_path).unwrap(); diff --git a/src/front/spv/function.rs b/src/front/spv/function.rs index 8a71b4037a..ba38cea69d 100644 --- a/src/front/spv/function.rs +++ b/src/front/spv/function.rs @@ -279,31 +279,34 @@ impl> super::Parser { } } - let position_index = members.iter().position(|member| match member.binding { - Some(crate::Binding::BuiltIn(crate::BuiltIn::Position)) => true, - _ => false, - }); - if let Some(component_index) = position_index { - let old_len = function.expressions.len(); - let global_expr = components[component_index]; - let access_expr = function.expressions.append(crate::Expression::AccessIndex { - base: global_expr, - index: 1, - }); - let load_expr = function.expressions.append(crate::Expression::Load { - pointer: access_expr, - }); - let neg_expr = function.expressions.append(crate::Expression::Unary { - op: crate::UnaryOperator::Negate, - expr: load_expr, - }); - function.body.push(crate::Statement::Emit( - function.expressions.range_from(old_len), - )); - function.body.push(crate::Statement::Store { - pointer: access_expr, - value: neg_expr, + if self.options.adjust_coordinate_space { + let position_index = members.iter().position(|member| match member.binding { + Some(crate::Binding::BuiltIn(crate::BuiltIn::Position)) => true, + _ => false, }); + if let Some(component_index) = position_index { + // The IR is Y-up, while SPIR-V is Y-down. + let old_len = function.expressions.len(); + let global_expr = components[component_index]; + let access_expr = function.expressions.append(crate::Expression::AccessIndex { + base: global_expr, + index: 1, + }); + let load_expr = function.expressions.append(crate::Expression::Load { + pointer: access_expr, + }); + let neg_expr = function.expressions.append(crate::Expression::Unary { + op: crate::UnaryOperator::Negate, + expr: load_expr, + }); + function.body.push(crate::Statement::Emit( + function.expressions.range_from(old_len), + )); + function.body.push(crate::Statement::Store { + pointer: access_expr, + value: neg_expr, + }); + } } let old_len = function.expressions.len(); diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index 1fe4e51daa..0d13d9714d 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -312,11 +312,24 @@ enum ExtendedClass { Output, } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct Options { + /// The IR coordinate space matches all the APIs except SPIR-V, + /// so by default we flip the Y coordinate of the `BuiltIn::Position`. + /// This flag can be used to avoid this. + pub adjust_coordinate_space: bool, pub flow_graph_dump_prefix: Option, } +impl Default for Options { + fn default() -> Self { + Options { + adjust_coordinate_space: true, + flow_graph_dump_prefix: None, + } + } +} + pub struct Parser { data: I, state: ModuleState, diff --git a/tests/out/quad-vert.msl.snap b/tests/out/quad-vert.msl.snap index 79d91a3cd4..16b3529771 100644 --- a/tests/out/quad-vert.msl.snap +++ b/tests/out/quad-vert.msl.snap @@ -69,7 +69,6 @@ vertex main2Output main2( a_uv = a_uv1; a_pos = a_pos1; main1(v_uv, a_uv, _, a_pos); - _.gl_Position.y = -_.gl_Position.y; const auto _tmp = type10 {v_uv, _.gl_Position, _.gl_PointSize, {_.gl_ClipDistance[0]}}; return main2Output { _tmp.member, _tmp.gl_Position1, _tmp.gl_PointSize1, {_tmp.gl_ClipDistance1[0]} }; } diff --git a/tests/snapshots.rs b/tests/snapshots.rs index d6bd5e8870..c4c86398b9 100644 --- a/tests/snapshots.rs +++ b/tests/snapshots.rs @@ -279,10 +279,13 @@ fn convert_wgsl_texture_array() { } #[cfg(feature = "spv-in")] -fn convert_spv(name: &str, targets: Targets) { +fn convert_spv(name: &str, adjust_coordinate_space: bool, targets: Targets) { let module = naga::front::spv::parse_u8_slice( &std::fs::read(format!("tests/in/{}{}", name, ".spv")).expect("Couldn't find spv file"), - &Default::default(), + &naga::front::spv::Options { + adjust_coordinate_space, + flow_graph_dump_prefix: None, + }, ) .unwrap(); check_targets(&module, name, targets); @@ -294,13 +297,13 @@ fn convert_spv(name: &str, targets: Targets) { #[cfg(feature = "spv-in")] #[test] fn convert_spv_quad_vert() { - convert_spv("quad-vert", Targets::METAL); + convert_spv("quad-vert", false, Targets::METAL); } #[cfg(feature = "spv-in")] #[test] fn convert_spv_shadow() { - convert_spv("shadow", Targets::IR | Targets::ANALYSIS); + convert_spv("shadow", true, Targets::IR | Targets::ANALYSIS); } #[cfg(feature = "glsl-in")]