[Mesa-dev] [PATCH 4/6] mesa: Use shared code for converting shader targets to short strings.

[Kenneth Graunke]

On 06/17/2013 04:10 PM, Eric Anholt wrote:
> We were duplicating this code all over the place, and they all would need
> updating for the next set of shader targets.
> ---
>   src/glsl/glsl_parser_extras.cpp            | 35 ++++++++++++++++++++++++++++++
>   src/glsl/glsl_parser_extras.h              |  3 +++
>   src/glsl/link_varyings.cpp                 | 15 ++++++++-----
>   src/glsl/linker.cpp                        |  4 ++--
>   src/mesa/drivers/dri/i965/brw_shader.cpp   |  9 ++++----
>   src/mesa/main/shaderapi.c                  | 17 ++-------------
>   src/mesa/main/uniform_query.cpp            |  9 ++------
>   src/mesa/program/ir_to_mesa.cpp            |  5 +----
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  7 ++----
>   9 files changed, 61 insertions(+), 43 deletions(-)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index 9862714..7b827ba 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -302,6 +302,41 @@ _mesa_glsl_parse_state::process_version_directive(YYLTYPE *locp, int version,
>      }
>   }
>
> +extern "C" {
> +
> +/**
> + * The most common use of _mesa_glsl_shader_target_name(), which is
> + * shared with C code in Mesa core to translate a GLenum to a short
> + * shader stage name in debug printouts.
> + *
> + * It recognizes the PROGRAM variants of the names so it can be used
> + * with a struct gl_program->Target, not just a struct
> + * gl_shader->Type.
> + */
> +const char *
> +_mesa_glsl_shader_target_name(GLenum type)
> +{
> +   switch (type) {
> +   case GL_VERTEX_SHADER:
> +   case GL_VERTEX_PROGRAM_ARB:
> +      return "vertex";
> +   case GL_FRAGMENT_SHADER:
> +   case GL_FRAGMENT_PROGRAM_ARB:
> +      return "fragment";
> +   case GL_GEOMETRY_SHADER:
> +      return "geometry";
> +   default:
> +      assert(!"Should not get here.");
> +      return "unknown";
> +   }
> +}
> +
> +} /* extern "C" */
> +
> +/**
> + * Overloaded C++ variant usable within the compiler for translating
> + * our internal enum into short stage names.
> + */
>   const char *
>   _mesa_glsl_shader_target_name(enum _mesa_glsl_parser_targets target)
>   {

I'm kind of wary of having an overloaded function where the 
distinguisher is GLenum (int) vs. enum (basically int).  I think the C 
vs. C++ enum rules save us here, but still not something I'm crazy about.

Still, this is a really nice cleanup.  I'm glad to see this centralized, 
as well as the vertex/fragment hardcoding going away.

As is,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>