MSL: Fix duplicate gl_Position outputs when gl_Position defined but unused.
When gl_Position is defined by SPIR-V, but neither used nor initialized,
it appeared twice in the MSL output, as gl_Position and glPosition_1.
The existing tests for whether an output is active check only that it is
used by an op, or initialized. Adding the implicit gl_Position also marked
the existing gl_Position as active, duplicating the output variable.
Fix is that when checking for the need to add an implicit gl_Position
output, also check if the var is already defined in the shader,
and just needs to be marked as active.
Add test shader.
diff --git a/reference/opt/shaders-msl/vert/unused-position.vert b/reference/opt/shaders-msl/vert/unused-position.vert
new file mode 100644
index 0000000..7dc4672
--- /dev/null
+++ b/reference/opt/shaders-msl/vert/unused-position.vert
@@ -0,0 +1,18 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_out
+{
+ float4 gl_Position [[position]];
+ float gl_PointSize [[point_size]];
+};
+
+vertex main0_out main0()
+{
+ main0_out out = {};
+ out.gl_PointSize = 1.0;
+ return out;
+}
+
diff --git a/reference/shaders-msl/vert/unused-position.vert b/reference/shaders-msl/vert/unused-position.vert
new file mode 100644
index 0000000..7dc4672
--- /dev/null
+++ b/reference/shaders-msl/vert/unused-position.vert
@@ -0,0 +1,18 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_out
+{
+ float4 gl_Position [[position]];
+ float gl_PointSize [[point_size]];
+};
+
+vertex main0_out main0()
+{
+ main0_out out = {};
+ out.gl_PointSize = 1.0;
+ return out;
+}
+
diff --git a/shaders-msl/vert/unused-position.vert b/shaders-msl/vert/unused-position.vert
new file mode 100644
index 0000000..61e30b4
--- /dev/null
+++ b/shaders-msl/vert/unused-position.vert
@@ -0,0 +1,13 @@
+#version 450
+
+out gl_PerVertex
+{
+ vec4 gl_Position;
+ float gl_PointSize;
+ float gl_ClipDistance[1];
+};
+
+void main()
+{
+ gl_PointSize = 1.0;
+}
diff --git a/spirv_msl.cpp b/spirv_msl.cpp
index 76c5565..8fbe7e3 100644
--- a/spirv_msl.cpp
+++ b/spirv_msl.cpp
@@ -877,12 +877,36 @@
if (need_position)
{
// If we can get away with returning void from entry point, we don't need to care.
- // If there is at least one other stage output, we need to return [[position]].
- need_position = false;
+ // If there is at least one other stage output, we need to return [[position]],
+ // so we need to create one if it doesn't appear in the SPIR-V. Before adding the
+ // implicit variable, check if it actually exists already, but just has not been used
+ // or initialized, and if so, mark it as active, and do not create the implicit variable.
+ bool has_output = false;
ir.for_each_typed_id<SPIRVariable>([&](uint32_t, SPIRVariable &var) {
if (var.storage == StorageClassOutput && interface_variable_exists_in_entry_point(var.self))
- need_position = true;
+ {
+ has_output = true;
+
+ // Check if the var is the Position builtin
+ if (has_decoration(var.self, DecorationBuiltIn) && get_decoration(var.self, DecorationBuiltIn) == BuiltInPosition)
+ active_output_builtins.set(BuiltInPosition);
+
+ // If the var is a struct, check if any members is the Position builtin
+ auto &var_type = get_variable_element_type(var);
+ if (var_type.basetype == SPIRType::Struct)
+ {
+ auto mbr_cnt = var_type.member_types.size();
+ for (uint32_t mbr_idx = 0; mbr_idx < mbr_cnt; mbr_idx++)
+ {
+ auto builtin = BuiltInMax;
+ bool is_builtin = is_member_builtin(var_type, mbr_idx, &builtin);
+ if (is_builtin && builtin == BuiltInPosition)
+ active_output_builtins.set(BuiltInPosition);
+ }
+ }
+ }
});
+ need_position = has_output && !active_output_builtins.get(BuiltInPosition);
}
if (need_position)
