<div dir="ltr">On 30 August 2013 16:07, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> The new function, cross_validate_types_and_qualifiers, will have multiple callers from this file in future commits. Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> --- src/glsl/link_varyings.cpp | 171 +++++++++++++++++++++++++-------------------- 1 file changed, 94 insertions(+), 77 deletions(-) diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index 4ceb1d3..a1899f7 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -41,6 +41,97 @@ /** + * Validate the types and qualifiers of an output from one stage against the + * matching input to another stage. + */ +static void +cross_validate_types_and_qualifiers(struct gl_shader_program *prog, + const ir_variable *input, + const ir_variable *output, + GLenum consumer_type, + const char *consumer_stage, + const char *producer_stage) </blockquote><div> </div><div>It seems redundant to pass both consumer_type and consumer_stage as arguments, since the latter is just _mesa_glsl_shader_target_name(consumer_type). You might want to just pass consumer_type and producer_type, and use _mesa_glsl_shader_target_name() to convert them to strings in the event of an error. </div><div>However, it's extra bookkeeping work to do that, so I'm ambivalent about it. Either way, </div><div> </div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +{ + /* Check that the types match between stages. + */ + const glsl_type *type_to_match = input->type; + if (consumer_type == GL_GEOMETRY_SHADER) { + assert(type_to_match->is_array()); /* Enforced by ast_to_hir */ + type_to_match = type_to_match->element_type(); + } + if (type_to_match != output->type) { + /* There is a bit of a special case for gl_TexCoord. This + * built-in is unsized by default. Applications that variable + * access it must redeclare it with a size. There is some + * language in the GLSL spec that implies the fragment shader + * and vertex shader do not have to agree on this size. Other + * driver behave this way, and one or two applications seem to + * rely on it. + * + * Neither declaration needs to be modified here because the array + * sizes are fixed later when update_array_sizes is called. + * + * From page 48 (page 54 of the PDF) of the GLSL 1.10 spec: + * + * "Unlike user-defined varying variables, the built-in + * varying variables don't have a strict one-to-one + * correspondence between the vertex language and the + * fragment language." + */ + if (!output->type->is_array() + || (strncmp("gl_", output->name, 3) != 0)) { + linker_error(prog, + "%s shader output `%s' declared as type `%s', " + "but %s shader input declared as type `%s'\n", + producer_stage, output->name, + output->type->name, + consumer_stage, input->type->name); + return; + } + } + + /* Check that all of the qualifiers match between stages. + */ + if (input->centroid != output->centroid) { + linker_error(prog, + "%s shader output `%s' %s centroid qualifier, " + "but %s shader input %s centroid qualifier\n", + producer_stage, + output->name, + (output->centroid) ? "has" : "lacks", + consumer_stage, + (input->centroid) ? "has" : "lacks"); + return; + } + + if (input->invariant != output->invariant) { + linker_error(prog, + "%s shader output `%s' %s invariant qualifier, " + "but %s shader input %s invariant qualifier\n", + producer_stage, + output->name, + (output->invariant) ? "has" : "lacks", + consumer_stage, + (input->invariant) ? "has" : "lacks"); + return; + } + + if (input->interpolation != output->interpolation) { + linker_error(prog, + "%s shader output `%s' specifies %s " + "interpolation qualifier, " + "but %s shader input specifies %s " + "interpolation qualifier\n", + producer_stage, + output->name, + output->interpolation_string(), + consumer_stage, + input->interpolation_string()); + return; + } +} + +/** * Validate that outputs from one stage match inputs of another */ void @@ -81,83 +172,9 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog, ir_variable *const output = parameters.get_variable(input->name); if (output != NULL) { - /* Check that the types match between stages. - */ - const glsl_type *type_to_match = input->type; - if (consumer->Type == GL_GEOMETRY_SHADER) { - assert(type_to_match->is_array()); /* Enforced by ast_to_hir */ - type_to_match = type_to_match->element_type(); - } - if (type_to_match != output->type) { - /* There is a bit of a special case for gl_TexCoord. This - * built-in is unsized by default. Applications that variable - * access it must redeclare it with a size. There is some - * language in the GLSL spec that implies the fragment shader - * and vertex shader do not have to agree on this size. Other - * driver behave this way, and one or two applications seem to - * rely on it. - * - * Neither declaration needs to be modified here because the array - * sizes are fixed later when update_array_sizes is called. - * - * From page 48 (page 54 of the PDF) of the GLSL 1.10 spec: - * - * "Unlike user-defined varying variables, the built-in - * varying variables don't have a strict one-to-one - * correspondence between the vertex language and the - * fragment language." - */ - if (!output->type->is_array() - || (strncmp("gl_", output->name, 3) != 0)) { - linker_error(prog, - "%s shader output `%s' declared as type `%s', " - "but %s shader input declared as type `%s'\n", - producer_stage, output->name, - output->type->name, - consumer_stage, input->type->name); - return; - } - } - - /* Check that all of the qualifiers match between stages. - */ - if (input->centroid != output->centroid) { - linker_error(prog, - "%s shader output `%s' %s centroid qualifier, " - "but %s shader input %s centroid qualifier\n", - producer_stage, - output->name, - (output->centroid) ? "has" : "lacks", - consumer_stage, - (input->centroid) ? "has" : "lacks"); - return; - } - - if (input->invariant != output->invariant) { - linker_error(prog, - "%s shader output `%s' %s invariant qualifier, " - "but %s shader input %s invariant qualifier\n", - producer_stage, - output->name, - (output->invariant) ? "has" : "lacks", - consumer_stage, - (input->invariant) ? "has" : "lacks"); - return; - } - - if (input->interpolation != output->interpolation) { - linker_error(prog, - "%s shader output `%s' specifies %s " - "interpolation qualifier, " - "but %s shader input specifies %s " - "interpolation qualifier\n", - producer_stage, - output->name, - output->interpolation_string(), - consumer_stage, - input->interpolation_string()); - return; - } + cross_validate_types_and_qualifiers(prog, input, output, + consumer->Type, consumer_stage, + producer_stage); } } } -- 1.8.1.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>