[Mesa-dev] [PATCH 7/9] glcpp: pass gl_context to preprocess()

[nobled]

On Sun, Apr 15, 2012 at 6:08 AM, nobled <nobled at dreamwidth.org> wrote:
> On Fri, Apr 13, 2012 at 2:04 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 04/13/2012 08:52 AM, nobled wrote:
>>>
>>> ---
>>>  src/glsl/glcpp/glcpp.c        |    2 +-
>>>  src/glsl/glcpp/glcpp.h        |    4 +++-
>>>  src/glsl/glcpp/pp.c           |    3 ++-
>>>  src/glsl/glsl_parser_extras.h |    2 +-
>>>  src/glsl/linker.cpp           |    2 --
>>>  src/glsl/main.cpp             |    2 +-
>>>  src/glsl/test_optpass.cpp     |    2 +-
>>>  7 files changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
>>> index e461a65..bd6344b 100644
>>> --- a/src/glsl/glcpp/glcpp.c
>>> +++ b/src/glsl/glcpp/glcpp.c
>>> @@ -111,7 +111,7 @@ main (int argc, char *argv[])
>>>        if (shader == NULL)
>>>           return 1;
>>>
>>> -       ret = preprocess(ctx,&shader,&info_log, NULL, API_OPENGL);
>>> +       ret = preprocess(ctx,&shader,&info_log, API_OPENGL, NULL);
>>>
>>>
>>>        printf("%s", shader);
>>>        fprintf(stderr, "%s", info_log);
>>> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
>>> index 2d7cad2..0c52077 100644
>>> --- a/src/glsl/glcpp/glcpp.h
>>> +++ b/src/glsl/glcpp/glcpp.h
>>> @@ -160,6 +160,8 @@ typedef struct active_list {
>>>        struct active_list *next;
>>>  } active_list_t;
>>>
>>> +struct gl_context;
>>> +
>>>  struct glcpp_parser {
>>>        yyscan_t scanner;
>>>        struct hash_table *defines;
>>> @@ -192,7 +194,7 @@ glcpp_parser_destroy (glcpp_parser_t *parser);
>>>
>>>  int
>>>  preprocess(void *ralloc_ctx, const char **shader, char **info_log,
>>> -          const struct gl_extensions *extensions, int api);
>>> +          int api, struct gl_context *glctx);
>>>
>>>  /* Functions for writing to the info log */
>>>
>>> diff --git a/src/glsl/glcpp/pp.c b/src/glsl/glcpp/pp.c
>>> index b0afed4..7218eeb 100644
>>> --- a/src/glsl/glcpp/pp.c
>>> +++ b/src/glsl/glcpp/pp.c
>>> @@ -146,9 +146,10 @@ remove_line_continuations(glcpp_parser_t *ctx,
>>> const char *shader)
>>>
>>>  int
>>>  preprocess(void *ralloc_ctx, const char **shader, char **info_log,
>>> -          const struct gl_extensions *extensions, int api)
>>> +          int api, struct gl_context *glctx)
>>>  {
>>>        int errors;
>>> +       struct gl_extensions *extensions = glctx ?&glctx->Extensions :
>>> NULL;
>>
>>
>> It looks like the only place that can call preprocess with a NULL extension
>> pointer is the standalone preprocessor.  I think it would be better to
>> modify that code to pass an empty gl_context with the gl_extensions field
>> cleared to zeros.  I think that would simplify this code a bit, and it would
>> eliminate the need to have a separate api parameter (preprocess could just
>> look at the gl_context).
> Yeah, I wanted to do that at first, but...
>>
>> Some helpers like in standalone_scaffolding.cpp might be in order.  This
>> would match the way the standalone GLSL compiler works.
> Yeah, it looked like it required duplicating
> initialize_context_to_defaults() and initialize_context() from
> main.cpp and standalone_scaffolding.cpp. Can we just move these down a
> layer into a new file, glcpp_standalone.cpp? The standalone
> preprocessor is already duplicating the _mesa_reference_shader(), and
> now the _mesa_shader_error(), standalone functions.
ping. Good idea or bad idea?

>
>>
>> As a side comment, we should probably get the glcpp unit tests wired into
>> 'make check'.
>>
>>
>>>        glcpp_parser_t *parser = glcpp_parser_create (extensions, api);
>>>        *shader = remove_line_continuations(parser, *shader);
>>>
>>> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
>>> index 1a909c6..10402c9 100644
>>> --- a/src/glsl/glsl_parser_extras.h
>>> +++ b/src/glsl/glsl_parser_extras.h
>>> @@ -297,7 +297,7 @@ extern "C" {
>>>  #endif
>>>
>>>  extern int preprocess(void *ctx, const char **shader, char **info_log,
>>> -                      const struct gl_extensions *extensions, int api);
>>> +                      int api, struct gl_context *glctx);
>>>
>>>  extern void _mesa_destroy_shader_compiler(void);
>>>  extern void _mesa_destroy_shader_compiler_caches(void);
>>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>>> index e2a5ef9..e43ec3a 100644
>>> --- a/src/glsl/linker.cpp
>>> +++ b/src/glsl/linker.cpp
>>> @@ -175,10 +175,8 @@ private:
>>>  static void
>>>  linker_msg(gl_shader_program *prog, bool error, const char *fmt, va_list
>>> ap)
>>>  {
>>> -
>>>     ralloc_strcat(&prog->InfoLog, error ? "error: " : "warning: ");
>>>     ralloc_vasprintf_append(&prog->InfoLog, fmt, ap);
>>> -
>>
>>
>> Eh... maybe just don't introduce these whitespace errors in the earlier
>> patch (6/9)? :)
> Whoops, heh. Should not use `git add -u`.
>>
>>>  }
>>>
>>>
>>> diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
>>> index d43bf1a..1d509eb 100644
>>> --- a/src/glsl/main.cpp
>>> +++ b/src/glsl/main.cpp
>>> @@ -136,7 +136,7 @@ compile_shader(struct gl_context *ctx, struct
>>> gl_shader *shader)
>>>
>>>     const char *source = shader->Source;
>>>     state->error = preprocess(state,&source,&state->info_log,
>>>
>>> -                            state->extensions, ctx->API) != 0;
>>> +                            ctx->API, ctx) != 0;
>>>
>>>     if (!state->error) {
>>>        _mesa_glsl_lexer_ctor(state, source);
>>> diff --git a/src/glsl/test_optpass.cpp b/src/glsl/test_optpass.cpp
>>> index 6abafb5..0844706 100644
>>> --- a/src/glsl/test_optpass.cpp
>>> +++ b/src/glsl/test_optpass.cpp
>>> @@ -219,7 +219,7 @@ int test_optpass(int argc, char **argv)
>>>        shader->Source = input.c_str();
>>>        const char *source = shader->Source;
>>>        state->error = preprocess(state,&source,&state->info_log,
>>>
>>> -                                state->extensions, ctx->API) != 0;
>>> +                                ctx->API, ctx) != 0;
>>>
>>>        if (!state->error) {
>>>           _mesa_glsl_lexer_ctor(state, source);
>>
>>