On Fri, Apr 13, 2012 at 2:04 PM, Ian Romanick <i...@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.
>
> 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);
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
