From 0e0633ca49425dbc869521cede6a82d2d91c8042 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 14 Jul 2021 17:29:46 +1000 Subject: [PATCH] glsl: relax rule on varying matching for shaders older than 4.20 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This expands on commit c54c42321ea5. See the code comment for full justifications. At the time of the previous commit Ian wanted to limit the relaxing of the rule to GLSL 3.30 as that was the highest version of shaders seen in the wild that were having trouble with the stricter rules. However since then I've found that the long standing issue with tess shaders failing to compile in the game 'Layers Of Fear' is due to this same issue. The game uses 4.10 shaders and also makes use of explicit varying locations, so here we relax the rule to 4.20 and make sure to apply the restriction to shaders using varyings with explicit locations also. Fixes: c54c42321ea5 ("glsl: relax rule on varying matching for shaders older than 4.00") Reviewed-by: Samuel Iglesias Gonsálvez Reviewed-by: Matt Turner Part-of: --- src/compiler/glsl/link_varyings.cpp | 71 +++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index 51b1f43..9954a73 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -740,6 +740,44 @@ validate_first_and_last_interface_explicit_locations(struct gl_context *ctx, } /** + * Check if we should force input / output matching between shader + * interfaces. + * + * Section 4.3.4 (Inputs) of the GLSL 4.10 specifications say: + * + * "Only the input variables that are actually read need to be + * written by the previous stage; it is allowed to have + * superfluous declarations of input variables." + * + * However it's not defined anywhere as to how we should handle + * inputs that are not written in the previous stage and it's not + * clear what "actually read" means. + * + * The GLSL 4.20 spec however is much clearer: + * + * "Only the input variables that are statically read need to + * be written by the previous stage; it is allowed to have + * superfluous declarations of input variables." + * + * It also has a table that states it is an error to statically + * read an input that is not defined in the previous stage. While + * it is not an error to not statically write to the output (it + * just needs to be defined to not be an error). + * + * The text in the GLSL 4.20 spec was an attempt to clarify the + * previous spec iterations. However given the difference in spec + * and that some applications seem to depend on not erroring when + * the input is not actually read in control flow we only apply + * this rule to GLSL 4.20 and higher. GLSL 4.10 shaders have been + * seen in the wild that depend on the less strict interpretation. + */ +static bool +static_input_output_matching(struct gl_shader_program *prog) +{ + return prog->data->Version >= (prog->IsES ? 0 : 420); +} + +/** * Validate that outputs from one stage match inputs of another */ void @@ -847,7 +885,7 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx, * output declaration and there is Static Use of the * declared input. */ - if (input->data.used) { + if (input->data.used && static_input_output_matching(prog)) { linker_error(prog, "%s shader input `%s' with explicit location " "has no matching output\n", @@ -882,40 +920,11 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx, /* Check for input vars with unmatched output vars in prev stage * taking into account that interface blocks could have a matching * output but with different name, so we ignore them. - * - * Section 4.3.4 (Inputs) of the GLSL 4.10 specifications say: - * - * "Only the input variables that are actually read need to be - * written by the previous stage; it is allowed to have - * superfluous declarations of input variables." - * - * However it's not defined anywhere as to how we should handle - * inputs that are not written in the previous stage and it's not - * clear what "actually read" means. - * - * The GLSL 4.20 spec however is much clearer: - * - * "Only the input variables that are statically read need to - * be written by the previous stage; it is allowed to have - * superfluous declarations of input variables." - * - * It also has a table that states it is an error to statically - * read an input that is not defined in the previous stage. While - * it is not an error to not statically write to the output (it - * just needs to be defined to not be an error). - * - * The text in the GLSL 4.20 spec was an attempt to clarify the - * previous spec iterations. However given the difference in spec - * and that some applications seem to depend on not erroring when - * the input is not actually read in control flow we only apply - * this rule to GLSL 4.00 and higher. GLSL 4.00 was chosen as - * a 3.30 shader is the highest version of GLSL we have seen in - * the wild dependant on the less strict interpretation. */ assert(!input->data.assigned); if (input->data.used && !input->get_interface_type() && !input->data.explicit_location && - (prog->data->Version >= (prog->IsES ? 0 : 400))) + static_input_output_matching(prog)) linker_error(prog, "%s shader input `%s' " "has no matching output in the previous stage\n", -- 2.7.4