[Mesa-dev] [PATCH 1/5] gallium/util: add a helper util_create_shader_from_text, cleanup some code
Marek Olšák
maraeo at gmail.com
Wed Dec 10 12:37:32 PST 2014
Indeed, locales can cause problems.
Would "vsnprintf_l" that overrides the current locale or "uselocale"
address your concerns?
vsnprintf_l: http://www.unix.com/man-page/osx/3/printf_l/
uselocale: http://man7.org/linux/man-pages/man3/uselocale.3.html
Marek
On Wed, Dec 10, 2014 at 8:50 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
> On 09/12/14 11:28, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> It's exposed, because I plan to use it elsewhere later.
>
>
> I hesitate in repeating myself, but I can't close my eyes. This is such a
> bad idea, at so many levels.
>
> How will this cope when the application is running on a different locale,
> that adds ',' '.' or ' ' to thousands, or something unexpected like that?
>
> Sure, so far the templates only use %s, but this
> util_create_shader_from_text is just glorifying a bad coding practices,
> being a matter of time until someone adds a %f or %d .
>
> NAK, FWIW.
>
>> ---
>> src/gallium/auxiliary/util/u_simple_shaders.c | 103
>> ++++++++++++--------------
>> src/gallium/auxiliary/util/u_simple_shaders.h | 3 +
>> 2 files changed, 49 insertions(+), 57 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_simple_shaders.c
>> b/src/gallium/auxiliary/util/u_simple_shaders.c
>> index edb3037..abe07ef 100644
>> --- a/src/gallium/auxiliary/util/u_simple_shaders.c
>> +++ b/src/gallium/auxiliary/util/u_simple_shaders.c
>> @@ -48,6 +48,43 @@
>> #include <stdio.h> /* include last */
>>
>>
>> +/**
>> + * Create a shader CSO from a textual TGSI shader representation.
>> + * The text can be formatted like printf.
>> + */
>> +void *
>> +util_create_shader_from_text(struct pipe_context *ctx, const char *text,
>> ...)
>> +{
>> + char final[4096];
>> + struct tgsi_token tokens[1024];
>
>
> 8K. Might be OK, but if it needs to get any bigger we should malloc to avoid
> stack overflow on Windows.
>
>
>> + struct pipe_shader_state state = {tokens};
>> + va_list ap;
>> +
>> + va_start(ap, text);
>> + if (vsnprintf(final, sizeof(final), text, ap) >= sizeof(final)) {
>
>
> vsnprintf -> util_vsnprintf
>
>
>> + va_end(ap);
>> + fprintf(stderr, "Cannot format a shader: %s\n", text);
>
>
> fprintf(stderr) is usually not visible on Windows. Please use _debug_printf.
>
>
>
> Jose
>
>
>> + return NULL;
>> + }
>> + va_end(ap);
>> +
>> + if (!tgsi_text_translate(final, tokens, Elements(tokens))) {
>> + fprintf(stderr, "Cannot translate a shader:\n%s\n", final);
>> + return NULL;
>> + }
>> +
>> + if (!memcmp(final, "VERT", 4))
>> + return ctx->create_vs_state(ctx, &state);
>> + else if (!memcmp(final, "GEOM", 4))
>> + return ctx->create_gs_state(ctx, &state);
>> + else if (!memcmp(final, "FRAG", 4))
>> + return ctx->create_fs_state(ctx, &state);
>> + else {
>> + assert(0);
>> + return NULL;
>> + }
>> +}
>> +
>>
>> /**
>> * Make simple vertex pass-through shader.
>> @@ -120,14 +157,8 @@ void *util_make_layered_clear_vertex_shader(struct
>> pipe_context *pipe)
>> "MOV OUT[1], IN[1]\n"
>> "MOV OUT[2], SV[0]\n"
>> "END\n";
>> - struct tgsi_token tokens[1000];
>> - struct pipe_shader_state state = {tokens};
>>
>> - if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>> - assert(0);
>> - return NULL;
>> - }
>> - return pipe->create_vs_state(pipe, &state);
>> + return util_create_shader_from_text(pipe, text);
>> }
>>
>> /**
>> @@ -436,24 +467,10 @@ util_make_fragment_passthrough_shader(struct
>> pipe_context *pipe,
>> "MOV OUT[0], IN[0]\n"
>> "END\n";
>>
>> - char text[sizeof(shader_templ)+100];
>> - struct tgsi_token tokens[1000];
>> - struct pipe_shader_state state = {tokens};
>> -
>> - sprintf(text, shader_templ,
>> - write_all_cbufs ? "PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1\n" :
>> "",
>> - tgsi_semantic_names[input_semantic],
>> - tgsi_interpolate_names[input_interpolate]);
>> -
>> - if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>> - assert(0);
>> - return NULL;
>> - }
>> -#if 0
>> - tgsi_dump(state.tokens, 0);
>> -#endif
>> -
>> - return pipe->create_fs_state(pipe, &state);
>> + return util_create_shader_from_text(pipe, shader_templ,
>> + write_all_cbufs ? "PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1\n" :
>> "",
>> + tgsi_semantic_names[input_semantic],
>> + tgsi_interpolate_names[input_interpolate]);
>> }
>>
>>
>> @@ -520,26 +537,12 @@ util_make_fs_blit_msaa_gen(struct pipe_context
>> *pipe,
>> "TXF OUT[0]%s, TEMP[0], SAMP[0], %s\n"
>> "END\n";
>>
>> - const char *type = tgsi_texture_names[tgsi_tex];
>> - char text[sizeof(shader_templ)+100];
>> - struct tgsi_token tokens[1000];
>> - struct pipe_shader_state state = {tokens};
>> -
>> assert(tgsi_tex == TGSI_TEXTURE_2D_MSAA ||
>> tgsi_tex == TGSI_TEXTURE_2D_ARRAY_MSAA);
>>
>> - sprintf(text, shader_templ, output_semantic, output_mask, type);
>> -
>> - if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>> - puts(text);
>> - assert(0);
>> - return NULL;
>> - }
>> -#if 0
>> - tgsi_dump(state.tokens, 0);
>> -#endif
>> -
>> - return pipe->create_fs_state(pipe, &state);
>> + return util_create_shader_from_text(pipe, shader_templ,
>> output_semantic,
>> + output_mask,
>> + tgsi_texture_names[tgsi_tex]);
>> }
>>
>>
>> @@ -608,26 +611,12 @@ util_make_fs_blit_msaa_depthstencil(struct
>> pipe_context *pipe,
>> "TXF OUT[0].z, TEMP[0], SAMP[0], %s\n"
>> "TXF OUT[1].y, TEMP[0], SAMP[1], %s\n"
>> "END\n";
>> -
>> const char *type = tgsi_texture_names[tgsi_tex];
>> - char text[sizeof(shader_templ)+100];
>> - struct tgsi_token tokens[1000];
>> - struct pipe_shader_state state = {tokens};
>>
>> assert(tgsi_tex == TGSI_TEXTURE_2D_MSAA ||
>> tgsi_tex == TGSI_TEXTURE_2D_ARRAY_MSAA);
>>
>> - sprintf(text, shader_templ, type, type);
>> -
>> - if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
>> - assert(0);
>> - return NULL;
>> - }
>> -#if 0
>> - tgsi_dump(state.tokens, 0);
>> -#endif
>> -
>> - return pipe->create_fs_state(pipe, &state);
>> + return util_create_shader_from_text(pipe, shader_templ, type, type);
>> }
>>
>>
>> diff --git a/src/gallium/auxiliary/util/u_simple_shaders.h
>> b/src/gallium/auxiliary/util/u_simple_shaders.h
>> index dd282e0..12f50f8 100644
>> --- a/src/gallium/auxiliary/util/u_simple_shaders.h
>> +++ b/src/gallium/auxiliary/util/u_simple_shaders.h
>> @@ -44,6 +44,9 @@ extern "C" {
>>
>>
>> extern void *
>> +util_create_shader_from_text(struct pipe_context *ctx, const char *text,
>> ...);
>> +
>> +extern void *
>> util_make_vertex_passthrough_shader(struct pipe_context *pipe,
>> uint num_attribs,
>> const uint *semantic_names,
>>
>
More information about the mesa-dev
mailing list