<div dir="ltr">On 17 December 2013 10:51, Brian Paul <<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</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"> <div class=""><div class="h5">On 12/17/2013 11:23 AM, Paul Berry wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> We already have a function for converting a shader type index to a string: _mesa_glsl_shader_target_name(). --- src/glsl/link_atomics.cpp | 8 +++----- src/glsl/linker.cpp | 16 ++++++---------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp index 33903ad..cac5615 100644 --- a/src/glsl/link_atomics.cpp +++ b/src/glsl/link_atomics.cpp @@ -21,6 +21,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include "glsl_parser_extras.h" #include "ir.h" #include "ir_uniform.h" #include "linker.h" @@ -214,9 +215,6 @@ link_check_atomic_counter_resources(struct gl_context *ctx, struct gl_shader_program *prog) { STATIC_ASSERT(MESA_SHADER_TYPES == 3); - static const char *shader_names[MESA_SHADER_TYPES] = { - "vertex", "geometry", "fragment" - }; const unsigned max_atomic_counters[MESA_SHADER_TYPES] = { ctx->Const.VertexProgram.MaxAtomicCounters, ctx->Const.GeometryProgram.MaxAtomicCounters, @@ -260,11 +258,11 @@ link_check_atomic_counter_resources(struct gl_context *ctx, for (unsigned i = 0; i < MESA_SHADER_TYPES; i++) { if (atomic_counters[i] > max_atomic_counters[i]) linker_error(prog, "Too many %s shader atomic counters", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); if (atomic_buffers[i] > max_atomic_buffers[i]) linker_error(prog, "Too many %s shader atomic counter buffers", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); } if (total_atomic_counters > ctx->Const.MaxCombinedAtomicCounters) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index a6133ea..1406f13 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1893,10 +1893,6 @@ store_fragdepth_layout(struct gl_shader_program *prog) static void check_resources(struct gl_context *ctx, struct gl_shader_program *prog) { - static const char *const shader_names[MESA_SHADER_TYPES] = { - "vertex", "geometry", "fragment" - }; - const unsigned max_samplers[MESA_SHADER_TYPES] = { ctx->Const.VertexProgram.MaxTextureImageUnits, ctx->Const.GeometryProgram.MaxTextureImageUnits, @@ -1929,7 +1925,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) if (sh->num_samplers > max_samplers[i]) { linker_error(prog, "Too many %s shader texture samplers", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); } if (sh->num_uniform_components > max_default_uniform_components[i]) { @@ -1938,11 +1934,11 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) "components, but the driver will try to optimize " "them out; this is non-portable out-of-spec " "behavior\n", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); } else { linker_error(prog, "Too many %s shader default uniform block " "components", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); } } @@ -1952,10 +1948,10 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) linker_warning(prog, "Too many %s shader uniform components, " "but the driver will try to optimize them out; " "this is non-portable out-of-spec behavior\n", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); } else { linker_error(prog, "Too many %s shader uniform components", - shader_names[i]); + _mesa_glsl_shader_target_name((gl_shader_type) i)); } } } @@ -1979,7 +1975,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) for (unsigned i = 0; i < MESA_SHADER_TYPES; i++) { if (blocks[i] > max_uniform_blocks[i]) { linker_error(prog, "Too many %s uniform blocks (%d/%d)", - shader_names[i], + _mesa_glsl_shader_target_name((gl_shader_type) i), blocks[i], max_uniform_blocks[i]); break; </blockquote> </div></div> Could the variable 'i' be declared as gl_shader_type to avoid all the casting? -Brian </blockquote><div> </div><div>I'm not thrilled about the casting either. I'm worried that some time in the future someone will try removing one of the casts, see that there is no compiler error, and come to the wrong conclusion that the cast does nothing. (In reality, the cast is necessary because it's selecting between two overloads of _mesa_glsl_shader_target_name(), one which takes a GLenum and one which takes a gl_shader_type). </div><div> Unfortunatly, declaring i as gl_shader_type isn't a great fix because it forces casting to be added elsewhere. The top of the "for" loop would have to change to something like: for (gl_shader_type i = (gl_shader_type) 0; i < MESA_SHADER_TYPES; i = (gl_shader_type) (i + 1)) { </div><div>which seems pretty ugly. </div><div>How about this idea instead: rather than use function overloading to distinguish between the two meanings of _mesa_glsl_shader_target_name(), have two functions with separate names, e.g.: </div><div>const char *_mesa_gl_shader_type_to_string(unsigned): converts MESA_SHADER_VERTEX -> "vertex", etc. </div><div>const char *_mesa_shader_target_enum_to_string(GLenum): converts GL_VERTEX_SHADER -> "vertex", etc. </div><div> </div><div>That would eliminate the need for typecasts, and it would reduce the danger of someone calling the wrong function by mistake.</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"> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">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>