From 0e9ad14ba6421066c43cfdb0a3275a52dbcabd9d Mon Sep 17 00:00:00 2001 From: Chip Davis Date: Wed, 12 Sep 2018 18:00:17 -0500 Subject: [PATCH 1/2] MSL: Handle the ViewportIndex builtin. This requires MSL 2.0+. Also, force `ViewportIndex` and `Layer` to be defined as the correct type, which is always `uint` in MSL. Since Metal doesn't yet have geometry shaders, the vertex shader (or tessellation evaluation shader == "post-tessellation vertex shader" in Metal jargon) is the only kind of shader that can set this output. This currently requires an extension to Vulkan, which causes validation of the SPIR-V binaries for the test cases to fail. Therefore, the test cases are marked "invalid", even though they're actually perfectly valid SPIR-V--they just won't work without the `SPV_EXT_shader_viewport_index_layer` extension. --- .../shaders-msl/vert/layer.msl11.invalid.vert | 24 +++++++++++++++++++ .../vert/viewport-index.msl2.invalid.vert | 24 +++++++++++++++++++ .../shaders-msl/vert/layer.msl11.invalid.vert | 24 +++++++++++++++++++ .../vert/viewport-index.msl2.invalid.vert | 24 +++++++++++++++++++ shaders-msl/vert/layer.msl11.invalid.vert | 10 ++++++++ .../vert/viewport-index.msl2.invalid.vert | 10 ++++++++ spirv_msl.cpp | 19 ++++++++++++++- 7 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 reference/opt/shaders-msl/vert/layer.msl11.invalid.vert create mode 100644 reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert create mode 100644 reference/shaders-msl/vert/layer.msl11.invalid.vert create mode 100644 reference/shaders-msl/vert/viewport-index.msl2.invalid.vert create mode 100644 shaders-msl/vert/layer.msl11.invalid.vert create mode 100644 shaders-msl/vert/viewport-index.msl2.invalid.vert diff --git a/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert b/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert new file mode 100644 index 00000000..1fa64660 --- /dev/null +++ b/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert @@ -0,0 +1,24 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 gl_Position [[position]]; + uint gl_Layer [[render_target_array_index]]; +}; + +struct main0_in +{ + float4 coord [[attribute(0)]]; +}; + +vertex main0_out main0(main0_in in [[stage_in]]) +{ + main0_out out = {}; + out.gl_Position = in.coord; + out.gl_Layer = int(in.coord.z); + return out; +} + diff --git a/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert b/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert new file mode 100644 index 00000000..89d3bb6c --- /dev/null +++ b/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert @@ -0,0 +1,24 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 gl_Position [[position]]; + uint gl_ViewportIndex [[viewport_array_index]]; +}; + +struct main0_in +{ + float4 coord [[attribute(0)]]; +}; + +vertex main0_out main0(main0_in in [[stage_in]]) +{ + main0_out out = {}; + out.gl_Position = in.coord; + out.gl_ViewportIndex = int(in.coord.z); + return out; +} + diff --git a/reference/shaders-msl/vert/layer.msl11.invalid.vert b/reference/shaders-msl/vert/layer.msl11.invalid.vert new file mode 100644 index 00000000..1fa64660 --- /dev/null +++ b/reference/shaders-msl/vert/layer.msl11.invalid.vert @@ -0,0 +1,24 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 gl_Position [[position]]; + uint gl_Layer [[render_target_array_index]]; +}; + +struct main0_in +{ + float4 coord [[attribute(0)]]; +}; + +vertex main0_out main0(main0_in in [[stage_in]]) +{ + main0_out out = {}; + out.gl_Position = in.coord; + out.gl_Layer = int(in.coord.z); + return out; +} + diff --git a/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert b/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert new file mode 100644 index 00000000..89d3bb6c --- /dev/null +++ b/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert @@ -0,0 +1,24 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 gl_Position [[position]]; + uint gl_ViewportIndex [[viewport_array_index]]; +}; + +struct main0_in +{ + float4 coord [[attribute(0)]]; +}; + +vertex main0_out main0(main0_in in [[stage_in]]) +{ + main0_out out = {}; + out.gl_Position = in.coord; + out.gl_ViewportIndex = int(in.coord.z); + return out; +} + diff --git a/shaders-msl/vert/layer.msl11.invalid.vert b/shaders-msl/vert/layer.msl11.invalid.vert new file mode 100644 index 00000000..73b918c8 --- /dev/null +++ b/shaders-msl/vert/layer.msl11.invalid.vert @@ -0,0 +1,10 @@ +#version 450 +#extension GL_ARB_shader_viewport_layer_array : require + +layout(location = 0) in vec4 coord; + +void main() +{ + gl_Position = coord; + gl_Layer = int(coord.z); +} diff --git a/shaders-msl/vert/viewport-index.msl2.invalid.vert b/shaders-msl/vert/viewport-index.msl2.invalid.vert new file mode 100644 index 00000000..c05c1048 --- /dev/null +++ b/shaders-msl/vert/viewport-index.msl2.invalid.vert @@ -0,0 +1,10 @@ +#version 450 +#extension GL_ARB_shader_viewport_layer_array : require + +layout(location = 0) in vec4 coord; + +void main() +{ + gl_Position = coord; + gl_ViewportIndex = int(coord.z); +} diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 6bc2b88c..b8d44956 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -1066,7 +1066,8 @@ uint32_t CompilerMSL::ensure_correct_builtin_type(uint32_t type_id, BuiltIn buil { auto &type = get(type_id); - if (builtin == BuiltInSampleMask && is_array(type)) + if ((builtin == BuiltInSampleMask && is_array(type)) || + ((builtin == BuiltInLayer || builtin == BuiltInViewportIndex) && type.basetype != SPIRType::UInt)) { uint32_t next_id = increase_bound_by(type.pointer ? 2 : 1); uint32_t base_type_id = next_id++; @@ -3228,6 +3229,10 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in // the shader into a render pipeline that uses a non-point topology. return msl_options.enable_point_size_builtin ? (string(" [[") + builtin_qualifier(builtin) + "]]") : ""; + case BuiltInViewportIndex: + if (!msl_options.supports_msl_version(2, 0)) + SPIRV_CROSS_THROW("ViewportIndex requires Metal 2.0."); + /* fallthrough */ case BuiltInPosition: case BuiltInLayer: case BuiltInClipDistance: @@ -4152,6 +4157,10 @@ string CompilerMSL::builtin_to_glsl(BuiltIn builtin, StorageClass storage) // When used in the entry function, output builtins are qualified with output struct name. // Test storage class as NOT Input, as output builtins might be part of generic type. + case BuiltInViewportIndex: + if (!msl_options.supports_msl_version(2, 0)) + SPIRV_CROSS_THROW("ViewportIndex requires Metal 2.0."); + /* fallthrough */ case BuiltInPosition: case BuiltInPointSize: case BuiltInClipDistance: @@ -4203,6 +4212,10 @@ string CompilerMSL::builtin_qualifier(BuiltIn builtin) return "position"; case BuiltInLayer: return "render_target_array_index"; + case BuiltInViewportIndex: + if (!msl_options.supports_msl_version(2, 0)) + SPIRV_CROSS_THROW("ViewportIndex requires Metal 2.0."); + return "viewport_array_index"; // Fragment function in case BuiltInFrontFacing: @@ -4279,6 +4292,10 @@ string CompilerMSL::builtin_type_decl(BuiltIn builtin) return "float4"; case BuiltInLayer: return "uint"; + case BuiltInViewportIndex: + if (!msl_options.supports_msl_version(2, 0)) + SPIRV_CROSS_THROW("ViewportIndex requires Metal 2.0."); + return "uint"; // Fragment function in case BuiltInFrontFacing: From ec857f6778ff9d4bb7484c0442525491f5743ff6 Mon Sep 17 00:00:00 2001 From: Chip Davis Date: Wed, 19 Sep 2018 09:13:30 -0500 Subject: [PATCH 2/2] Cast uses of Layer and ViewportIndex to the expected type. --- .../shaders-msl/vert/layer.msl11.invalid.vert | 2 +- .../vert/viewport-index.msl2.invalid.vert | 2 +- .../shaders-msl/vert/layer.msl11.invalid.vert | 2 +- .../vert/viewport-index.msl2.invalid.vert | 2 +- spirv_msl.cpp | 28 +++++++++++++++++-- 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert b/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert index 1fa64660..b6f39dca 100644 --- a/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert +++ b/reference/opt/shaders-msl/vert/layer.msl11.invalid.vert @@ -18,7 +18,7 @@ vertex main0_out main0(main0_in in [[stage_in]]) { main0_out out = {}; out.gl_Position = in.coord; - out.gl_Layer = int(in.coord.z); + out.gl_Layer = uint(int(in.coord.z)); return out; } diff --git a/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert b/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert index 89d3bb6c..e5316c07 100644 --- a/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert +++ b/reference/opt/shaders-msl/vert/viewport-index.msl2.invalid.vert @@ -18,7 +18,7 @@ vertex main0_out main0(main0_in in [[stage_in]]) { main0_out out = {}; out.gl_Position = in.coord; - out.gl_ViewportIndex = int(in.coord.z); + out.gl_ViewportIndex = uint(int(in.coord.z)); return out; } diff --git a/reference/shaders-msl/vert/layer.msl11.invalid.vert b/reference/shaders-msl/vert/layer.msl11.invalid.vert index 1fa64660..b6f39dca 100644 --- a/reference/shaders-msl/vert/layer.msl11.invalid.vert +++ b/reference/shaders-msl/vert/layer.msl11.invalid.vert @@ -18,7 +18,7 @@ vertex main0_out main0(main0_in in [[stage_in]]) { main0_out out = {}; out.gl_Position = in.coord; - out.gl_Layer = int(in.coord.z); + out.gl_Layer = uint(int(in.coord.z)); return out; } diff --git a/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert b/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert index 89d3bb6c..e5316c07 100644 --- a/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert +++ b/reference/shaders-msl/vert/viewport-index.msl2.invalid.vert @@ -18,7 +18,7 @@ vertex main0_out main0(main0_in in [[stage_in]]) { main0_out out = {}; out.gl_Position = in.coord; - out.gl_ViewportIndex = int(in.coord.z); + out.gl_ViewportIndex = uint(int(in.coord.z)); return out; } diff --git a/spirv_msl.cpp b/spirv_msl.cpp index b8d44956..49db9518 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -4714,6 +4714,8 @@ void CompilerMSL::bitcast_from_builtin_load(uint32_t source_id, std::string &exp case BuiltInLocalInvocationIndex: case BuiltInWorkgroupSize: case BuiltInNumWorkgroups: + case BuiltInLayer: + case BuiltInViewportIndex: expected_type = SPIRType::UInt; break; @@ -4725,9 +4727,31 @@ void CompilerMSL::bitcast_from_builtin_load(uint32_t source_id, std::string &exp expr = bitcast_expression(expr_type, expected_type, expr); } -// MSL always declares output builtins with the SPIR-V type. -void CompilerMSL::bitcast_to_builtin_store(uint32_t, std::string &, const SPIRType &) +void CompilerMSL::bitcast_to_builtin_store(uint32_t target_id, std::string &expr, const SPIRType &expr_type) { + // Only interested in standalone builtin variables. + if (!has_decoration(target_id, DecorationBuiltIn)) + return; + + auto builtin = static_cast(get_decoration(target_id, DecorationBuiltIn)); + auto expected_type = expr_type.basetype; + switch (builtin) + { + case BuiltInLayer: + case BuiltInViewportIndex: + expected_type = SPIRType::UInt; + break; + + default: + break; + } + + if (expected_type != expr_type.basetype) + { + auto type = expr_type; + type.basetype = expected_type; + expr = bitcast_expression(type, expr_type.basetype, expr); + } } std::string CompilerMSL::to_initializer_expression(const SPIRVariable &var)