[Mesa-dev] [PATCH 1/4] glsl: Refactor a bunch of the code out of cross_validate_outputs_to_inputs
Paul Berry
stereotype441 at gmail.com
Tue Sep 3 09:41:16 PDT 2013
On 30 August 2013 16:07, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> The new function, cross_validate_types_and_qualifiers, will have
> multiple callers from this file in future commits.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
> 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)
>
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.
However, it's extra bookkeeping work to do that, so I'm ambivalent about
it. Either way,
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> +{
> + /* 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130903/81de9692/attachment-0001.html>
More information about the mesa-dev
mailing list