[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
Fri Dec 12 07:26:12 PST 2014
Alternatively, I can search the format string and reject it if it
contains '%' not followed by 's'.
Marek
On Wed, Dec 10, 2014 at 9:37 PM, Marek Olšák <maraeo at gmail.com> wrote:
> 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