From 17d88ca92872b3cd5a861918605deb55dc4296b1 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Thu, 4 May 2017 10:10:30 +0200 Subject: [PATCH 1/3] Add compatibility option for PointSize in HLSL. If we opt-in to it, PointSize can be ignored to avoid more annoying workarounds. --- main.cpp | 10 +++++++- .../shaders-hlsl/vert/point-size-compat.vert | 20 ++++++++++++++++ shaders-hlsl/vert/point-size-compat.vert | 8 +++++++ spirv_hlsl.cpp | 23 +++++++++++++++++++ spirv_hlsl.hpp | 3 +++ test_shaders.py | 2 +- 6 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 reference/shaders-hlsl/vert/point-size-compat.vert create mode 100644 shaders-hlsl/vert/point-size-compat.vert diff --git a/main.cpp b/main.cpp index ae156d99..a8021417 100644 --- a/main.cpp +++ b/main.cpp @@ -435,6 +435,7 @@ struct CLIArguments bool msl = false; bool msl_pack_ubos = true; bool hlsl = false; + bool hlsl_compat = false; bool vulkan_semantics = false; bool remove_unused = false; bool cfg_analysis = true; @@ -447,7 +448,7 @@ static void print_help() "[--vulkan-semantics] [--flatten-ubo] [--fixup-clipspace] [--iterations iter] " "[--cpp] [--cpp-interface-name ] " "[--msl] [--msl-no-pack-ubos] " - "[--hlsl] [--shader-model] " + "[--hlsl] [--shader-model] [--hlsl-enable-compat] " "[--pls-in format input-name] [--pls-out format output-name] [--remap source_name target_name " "components] [--extension ext] [--entry name] [--remove-unused-variables] " "[--remap-variable-type ]\n"); @@ -571,6 +572,7 @@ int main(int argc, char *argv[]) cbs.add("--msl", [&args](CLIParser &) { args.msl = true; }); cbs.add("--msl-no-pack-ubos", [&args](CLIParser &) { args.msl_pack_ubos = false; }); cbs.add("--hlsl", [&args](CLIParser &) { args.hlsl = true; }); + cbs.add("--hlsl-enable-compat", [&args](CLIParser &) { args.hlsl_compat = true; }); cbs.add("--vulkan-semantics", [&args](CLIParser &) { args.vulkan_semantics = true; }); cbs.add("--extension", [&args](CLIParser &parser) { args.extensions.push_back(parser.next_string()); }); cbs.add("--entry", [&args](CLIParser &parser) { args.entry = parser.next_string(); }); @@ -698,6 +700,12 @@ int main(int argc, char *argv[]) hlsl_opts.shader_model = args.shader_model; } + + if (args.hlsl_compat) + { + // Enable all compat options. + hlsl_opts.point_size_compat = true; + } hlsl->set_options(hlsl_opts); } diff --git a/reference/shaders-hlsl/vert/point-size-compat.vert b/reference/shaders-hlsl/vert/point-size-compat.vert new file mode 100644 index 00000000..3aad537d --- /dev/null +++ b/reference/shaders-hlsl/vert/point-size-compat.vert @@ -0,0 +1,20 @@ +static float4 gl_Position; +static float gl_PointSize; +struct SPIRV_Cross_Output +{ + float4 gl_Position : SV_Position; +}; + +void vert_main() +{ + gl_Position = float4(1.0f, 1.0f, 1.0f, 1.0f); + gl_PointSize = 10.0f; +} + +SPIRV_Cross_Output main() +{ + vert_main(); + SPIRV_Cross_Output stage_output; + stage_output.gl_Position = gl_Position; + return stage_output; +} diff --git a/shaders-hlsl/vert/point-size-compat.vert b/shaders-hlsl/vert/point-size-compat.vert new file mode 100644 index 00000000..64eff363 --- /dev/null +++ b/shaders-hlsl/vert/point-size-compat.vert @@ -0,0 +1,8 @@ +#version 310 es + +void main() +{ + gl_Position = vec4(1.0); + gl_PointSize = 10.0; +} + diff --git a/spirv_hlsl.cpp b/spirv_hlsl.cpp index 75e60313..7920e81a 100644 --- a/spirv_hlsl.cpp +++ b/spirv_hlsl.cpp @@ -270,6 +270,15 @@ void CompilerHLSL::emit_builtin_outputs_in_struct() semantic = legacy ? "DEPTH" : "SV_Depth"; break; + case BuiltInPointSize: + // If point_size_compat is enabled, just ignore PointSize. + // PointSize does not exist in HLSL, but some code bases might want to be able to use these shaders, + // even if it means working around the missing feature. + if (options.point_size_compat) + break; + else + SPIRV_CROSS_THROW("Unsupported builtin in HLSL."); + default: SPIRV_CROSS_THROW("Unsupported builtin in HLSL."); break; @@ -499,6 +508,16 @@ void CompilerHLSL::emit_builtin_variables() type = "int"; break; + case BuiltInPointSize: + if (options.point_size_compat) + { + // Just emit the global variable, it will be ignored. + type = "float"; + break; + } + else + SPIRV_CROSS_THROW(join("Unsupported builtin in HLSL: ", unsigned(builtin))); + default: SPIRV_CROSS_THROW(join("Unsupported builtin in HLSL: ", unsigned(builtin))); break; @@ -1035,6 +1054,10 @@ void CompilerHLSL::emit_hlsl_entry_point() if (!(active_output_builtins & (1ull << i))) continue; + // PointSize doesn't exist in HLSL. + if (i == BuiltInPointSize) + continue; + auto builtin = builtin_to_glsl(static_cast(i)); statement("stage_output.", builtin, " = ", builtin, ";"); } diff --git a/spirv_hlsl.hpp b/spirv_hlsl.hpp index 4a42ed64..3618c017 100644 --- a/spirv_hlsl.hpp +++ b/spirv_hlsl.hpp @@ -31,6 +31,9 @@ public: 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 = true; }; CompilerHLSL(std::vector spirv_) diff --git a/test_shaders.py b/test_shaders.py index e28ddfad..4788691d 100755 --- a/test_shaders.py +++ b/test_shaders.py @@ -107,7 +107,7 @@ def cross_compile_hlsl(shader): os.close(hlsl_f) subprocess.check_call(['glslangValidator', '-V', '-o', spirv_path, shader]) spirv_cross_path = './spirv-cross' - subprocess.check_call([spirv_cross_path, '--entry', 'main', '--output', hlsl_path, spirv_path, '--hlsl', '--shader-model', '50']) + subprocess.check_call([spirv_cross_path, '--entry', 'main', '--output', hlsl_path, spirv_path, '--hlsl-enable-compat', '--hlsl', '--shader-model', '50']) subprocess.check_call(['spirv-val', spirv_path]) validate_shader_hlsl(hlsl_path) From 851acf371225c211464d9b0129eaba375b28fc66 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Thu, 4 May 2017 10:28:30 +0200 Subject: [PATCH 2/3] Avoid boolean mix in HLSL. Update glslang travis checkout as boolean mix support was broken on that commit. --- .travis.yml | 2 +- reference/shaders-hlsl/frag/boolean-mix.frag | 28 ++++++++++++++++++++ reference/shaders-msl/frag/mix.frag | 3 ++- reference/shaders/frag/mix.frag | 2 +- shaders-hlsl/frag/boolean-mix.frag | 10 +++++++ spirv_glsl.cpp | 3 ++- spirv_glsl.hpp | 1 + spirv_hlsl.cpp | 1 + 8 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 reference/shaders-hlsl/frag/boolean-mix.frag create mode 100644 shaders-hlsl/frag/boolean-mix.frag diff --git a/.travis.yml b/.travis.yml index f90439ff..6be89672 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ dist: trusty # We check out glslang at a specific revision to avoid test output mismatches env: - - GLSLANG_REV=19ea56899cdcab0f480d257fcea100ad2160e833 + - GLSLANG_REV=de1cc06c1d1c1eeae31aa5cae686ccf24064730f before_script: - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew install python3; fi diff --git a/reference/shaders-hlsl/frag/boolean-mix.frag b/reference/shaders-hlsl/frag/boolean-mix.frag new file mode 100644 index 00000000..f9201c14 --- /dev/null +++ b/reference/shaders-hlsl/frag/boolean-mix.frag @@ -0,0 +1,28 @@ +static float2 FragColor; +static float2 x0; + +struct SPIRV_Cross_Input +{ + float2 x0 : TEXCOORD0; +}; + +struct SPIRV_Cross_Output +{ + float2 FragColor : SV_Target0; +}; + +void frag_main() +{ + bool _21 = x0.x > x0.y; + bool2 _27 = bool2(_21, _21); + FragColor = float2(_27.x ? float2(1.0f, 0.0f).x : float2(0.0f, 1.0f).x, _27.y ? float2(1.0f, 0.0f).y : float2(0.0f, 1.0f).y); +} + +SPIRV_Cross_Output main(SPIRV_Cross_Input stage_input) +{ + x0 = stage_input.x0; + frag_main(); + SPIRV_Cross_Output stage_output; + stage_output.FragColor = FragColor; + return stage_output; +} diff --git a/reference/shaders-msl/frag/mix.frag b/reference/shaders-msl/frag/mix.frag index 33157811..2d357666 100644 --- a/reference/shaders-msl/frag/mix.frag +++ b/reference/shaders-msl/frag/mix.frag @@ -23,7 +23,8 @@ fragment main0_out main0(main0_in in [[stage_in]]) out.FragColor = float4(l.x ? in.vIn1.x : in.vIn0.x, l.y ? in.vIn1.y : in.vIn0.y, l.z ? in.vIn1.z : in.vIn0.z, l.w ? in.vIn1.w : in.vIn0.w); bool f = true; out.FragColor = float4(f ? in.vIn3 : in.vIn2); - out.FragColor = f ? in.vIn0 : in.vIn1; + bool4 _37 = bool4(f); + out.FragColor = float4(_37.x ? in.vIn0.x : in.vIn1.x, _37.y ? in.vIn0.y : in.vIn1.y, _37.z ? in.vIn0.z : in.vIn1.z, _37.w ? in.vIn0.w : in.vIn1.w); out.FragColor = float4(f ? in.vIn2 : in.vIn3); return out; } diff --git a/reference/shaders/frag/mix.frag b/reference/shaders/frag/mix.frag index 6e7884ff..2b288dff 100644 --- a/reference/shaders/frag/mix.frag +++ b/reference/shaders/frag/mix.frag @@ -14,7 +14,7 @@ void main() FragColor = mix(vIn0, vIn1, l); bool f = true; FragColor = vec4(f ? vIn3 : vIn2); - FragColor = f ? vIn0 : vIn1; + FragColor = mix(vIn1, vIn0, bvec4(f)); FragColor = vec4(f ? vIn2 : vIn3); } diff --git a/shaders-hlsl/frag/boolean-mix.frag b/shaders-hlsl/frag/boolean-mix.frag new file mode 100644 index 00000000..9fd8ab34 --- /dev/null +++ b/shaders-hlsl/frag/boolean-mix.frag @@ -0,0 +1,10 @@ +#version 310 es +precision mediump float; + +layout(location = 0) in vec2 x0; +layout(location = 0) out vec2 FragColor; + +void main() +{ + FragColor = x0.x > x0.y ? vec2(1.0, 0.0) : vec2(0.0, 1.0); +} diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 76010286..0e72df13 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -2591,7 +2591,8 @@ void CompilerGLSL::emit_mix_op(uint32_t result_type, uint32_t id, uint32_t left, auto &restype = get(result_type); string mix_op; - bool has_boolean_mix = (options.es && options.version >= 310) || (!options.es && options.version >= 450); + bool has_boolean_mix = backend.boolean_mix_support && + ((options.es && options.version >= 310) || (!options.es && options.version >= 450)); bool trivial_mix = to_trivial_mix_op(restype, mix_op, left, right, lerp); // Cannot use boolean mix when the lerp argument is just one boolean, diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index 81be16fc..83cc8b45 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -276,6 +276,7 @@ protected: bool use_initializer_list = false; bool native_row_major_matrix = true; bool use_constructor_splatting = true; + bool boolean_mix_support = true; } backend; void emit_struct(SPIRType &type); diff --git a/spirv_hlsl.cpp b/spirv_hlsl.cpp index 7920e81a..6c2787a3 100644 --- a/spirv_hlsl.cpp +++ b/spirv_hlsl.cpp @@ -1842,6 +1842,7 @@ string CompilerHLSL::compile() backend.explicit_struct_type = false; backend.use_initializer_list = true; backend.use_constructor_splatting = false; + backend.boolean_mix_support = false; update_active_builtins(); From 2b4c3db7e36715db2391d91c65cb252f269f9407 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Thu, 4 May 2017 10:32:43 +0200 Subject: [PATCH 3/3] Don't enable point_size_compat by default. --- spirv_hlsl.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spirv_hlsl.hpp b/spirv_hlsl.hpp index 3618c017..5fb1d338 100644 --- a/spirv_hlsl.hpp +++ b/spirv_hlsl.hpp @@ -33,7 +33,7 @@ public: bool flip_vert_y = false; // Allows the PointSize builtin, and ignores it, as PointSize is not supported in HLSL. - bool point_size_compat = true; + bool point_size_compat = false; }; CompilerHLSL(std::vector spirv_)