Merge pull request #246 from KhronosGroup/fix-241

Unify vertex clip-coord fixups.

README.md

@@ -241,6 +241,20 @@ but older GLSL relies on symbol names to perform the linking. When emitting shad
 so it is important that the API user ensures that the names of I/O variables are sanitized so that linking will work properly.
 The reflection API can rename variables, struct types and struct members to deal with these scenarios using `Compiler::set_name` and friends.
 
+#### Clip-space conventions
+
+SPIRV-Cross can perform some common clip space conversions on gl_Position/SV_Position by enabling `CompilerGLSL::Options.vertex.fixup_clipspace`.
+While this can be convenient, it is recommended to modify the projection matrices instead as that can achieve the same result.
+
+For GLSL targets, enabling this will convert a shader which assumes `[0, w]` depth range (Vulkan / D3D / Metal) into `[-w, w]` range.
+For MSL and HLSL targets, enabling this will convert a shader in `[-w, w]` depth range (OpenGL) to `[0, w]` depth range.
+
+By default, the CLI will not enable `fixup_clipspace`, but in the API you might want to set an explicit value using `CompilerGLSL::set_options()`.
+
+Y-flipping of gl_Position and similar is also supported.
+The use of this is discouraged, because relying on vertex shader Y-flipping tends to get quite messy.
+To enable this, set `CompilerGLSL::Options.vertex.flip_vert_y` or `--flip-vert-y` in CLI.
+
 ## Contributing
 
 Contributions to SPIRV-Cross are welcome. See Testing and Licensing sections for details.

main.cpp

@@ -17,6 +17,7 @@
 #include "spirv_cpp.hpp"
 #include "spirv_hlsl.hpp"
 #include "spirv_msl.hpp"
+#include "spirv_glsl.hpp"
 #include <algorithm>
 #include <cstdio>
 #include <cstring>
@@ -439,6 +440,7 @@ struct CLIArguments
 	bool force_temporary = false;
 	bool flatten_ubo = false;
 	bool fixup = false;
+	bool yflip = false;
 	bool sso = false;
 	vector<PLSArg> pls_in;
 	vector<PLSArg> pls_out;
@@ -463,7 +465,7 @@ static void print_help()
 {
 	fprintf(stderr, "Usage: spirv-cross [--output <output path>] [SPIR-V file] [--es] [--no-es] [--no-cfg-analysis] "
 	                "[--version <GLSL version>] [--dump-resources] [--help] [--force-temporary] "
-	                "[--vulkan-semantics] [--flatten-ubo] [--fixup-clipspace] [--iterations iter] "
+	                "[--vulkan-semantics] [--flatten-ubo] [--fixup-clipspace] [--flip-vert-y] [--iterations iter] "
 	                "[--cpp] [--cpp-interface-name <name>] "
 	                "[--msl] "
 	                "[--hlsl] [--shader-model] [--hlsl-enable-compat] "
@@ -603,6 +605,7 @@ int main(int argc, char *argv[])
 	cbs.add("--force-temporary", [&args](CLIParser &) { args.force_temporary = true; });
 	cbs.add("--flatten-ubo", [&args](CLIParser &) { args.flatten_ubo = true; });
 	cbs.add("--fixup-clipspace", [&args](CLIParser &) { args.fixup = true; });
+	cbs.add("--flip-vert-y", [&args](CLIParser &) { args.yflip = true; });
 	cbs.add("--iterations", [&args](CLIParser &parser) { args.iterations = parser.next_uint(); });
 	cbs.add("--cpp", [&args](CLIParser &) { args.cpp = true; });
 	cbs.add("--cpp-interface-name", [&args](CLIParser &parser) { args.cpp_interface_name = parser.next_string(); });
@@ -735,6 +738,7 @@ int main(int argc, char *argv[])
 	opts.flatten_multidimensional_arrays = args.flatten_multidimensional_arrays;
 	opts.vulkan_semantics = args.vulkan_semantics;
 	opts.vertex.fixup_clipspace = args.fixup;
+	opts.vertex.flip_vert_y = args.yflip;
 	opts.cfg_analysis = args.cfg_analysis;
 	compiler->set_options(opts);
 

spirv_glsl.cpp

@@ -6670,10 +6670,16 @@ void CompilerGLSL::emit_function(SPIRFunction &func, uint64_t return_flags)
 void CompilerGLSL::emit_fixup()
 {
 	auto &execution = get_entry_point();
-	if (execution.model == ExecutionModelVertex && options.vertex.fixup_clipspace)
+	if (execution.model == ExecutionModelVertex)
 	{
-		const char *suffix = backend.float_literal_suffix ? "f" : "";
+		if (options.vertex.fixup_clipspace)
-		statement("gl_Position.z = 2.0", suffix, " * gl_Position.z - gl_Position.w;");
+		{
+			const char *suffix = backend.float_literal_suffix ? "f" : "";
+			statement("gl_Position.z = 2.0", suffix, " * gl_Position.z - gl_Position.w;");
+		}
+
+		if (options.vertex.flip_vert_y)
+			statement("gl_Position.y = -gl_Position.y;");
 	}
 }
 

spirv_glsl.hpp

@@ -89,8 +89,13 @@ public:
 
 		struct
 		{
-			// In vertex shaders, rewrite [0, w] depth (Vulkan/D3D style) to [-w, w] depth (GL style).
+			// GLSL: In vertex shaders, rewrite [0, w] depth (Vulkan/D3D style) to [-w, w] depth (GL style).
-			bool fixup_clipspace = true;
+			// MSL: In vertex shaders, rewrite [-w, w] depth (GL style) to [0, w] depth.
+			// HLSL: In vertex shaders, rewrite [-w, w] depth (GL style) to [0, w] depth.
+			bool fixup_clipspace = false;
+
+			// Inverts gl_Position.y or equivalent.
+			bool flip_vert_y = false;
 		} vertex;
 
 		struct

spirv_hlsl.cpp

@@ -1228,33 +1228,29 @@ void CompilerHLSL::emit_hlsl_entry_point()
 			}
 		}
 
-		if (execution.model == ExecutionModelVertex)
-		{
-			// Do various mangling on the gl_Position.
-			if (options.shader_model <= 30)
-			{
-				statement("stage_output.gl_Position.x = stage_output.gl_Position.x - gl_HalfPixel.x * "
-				          "stage_output.gl_Position.w;");
-				statement("stage_output.gl_Position.y = stage_output.gl_Position.y + gl_HalfPixel.y * "
-				          "stage_output.gl_Position.w;");
-			}
-			if (options.flip_vert_y)
-			{
-				statement("stage_output.gl_Position.y = -stage_output.gl_Position.y;");
-			}
-			if (options.fixup_clipspace)
-			{
-				statement(
-				    "stage_output.gl_Position.z = (stage_output.gl_Position.z + stage_output.gl_Position.w) * 0.5;");
-			}
-		}
-
 		statement("return stage_output;");
 	}
 
 	end_scope();
 }
 
+void CompilerHLSL::emit_fixup()
+{
+	// Do various mangling on the gl_Position.
+	if (options.shader_model <= 30)
+	{
+		statement("gl_Position.x = gl_Position.x - gl_HalfPixel.x * "
+				          "gl_Position.w;");
+		statement("gl_Position.y = gl_Position.y + gl_HalfPixel.y * "
+				          "gl_Position.w;");
+	}
+
+	if (CompilerGLSL::options.vertex.flip_vert_y)
+		statement("gl_Position.y = -gl_Position.y;");
+	if (CompilerGLSL::options.vertex.fixup_clipspace)
+		statement("gl_Position.z = (gl_Position.z + gl_Position.w) * 0.5;");
+}
+
 void CompilerHLSL::emit_texture_op(const Instruction &i)
 {
 	auto ops = stream(i);

spirv_hlsl.hpp

@@ -29,8 +29,6 @@ public:
 	struct Options
 	{
 		uint32_t shader_model = 30; // TODO: map ps_4_0_level_9_0,... somehow
-		bool fixup_clipspace = false;
-		bool flip_vert_y = false;
 
 		// Allows the PointSize builtin, and ignores it, as PointSize is not supported in HLSL.
 		bool point_size_compat = false;
@@ -81,6 +79,7 @@ private:
 	void emit_modern_uniform(const SPIRVariable &var);
 	void emit_legacy_uniform(const SPIRVariable &var);
 	void emit_specialization_constants();
+	void emit_fixup() override;
 	std::string layout_for_member(const SPIRType &type, uint32_t index) override;
 	std::string to_interpolation_qualifiers(uint64_t flags) override;
 	std::string bitcast_glsl_op(const SPIRType &result_type, const SPIRType &argument_type) override;

spirv_msl.cpp

@@ -106,7 +106,6 @@ string CompilerMSL::compile()
 	CompilerGLSL::options.vulkan_semantics = true;
 	CompilerGLSL::options.es = false;
 	CompilerGLSL::options.version = 120;
-	CompilerGLSL::options.vertex.fixup_clipspace = false;
 	backend.float_literal_suffix = false;
 	backend.uint32_t_literal_suffix = true;
 	backend.basic_int_type = "int";
@@ -1954,7 +1953,7 @@ void CompilerMSL::emit_fixup()
 			          ".w) * 0.5;       // Adjust clip-space for Metal");
 		}
 
-		if (options.flip_vert_y)
+		if (CompilerGLSL::options.vertex.flip_vert_y)
 			statement(qual_pos_var_name, ".y = -(", qual_pos_var_name, ".y);", "    // Invert Y-axis for Metal");
 	}
 }

spirv_msl.hpp

@@ -76,7 +76,6 @@ public:
 	// Options for compiling to Metal Shading Language
 	struct Options
 	{
-		bool flip_vert_y = false;
 		bool enable_point_size_builtin = true;
 		std::string entry_point_name;
 	};