On 4 April 2013 12:13, Kenneth Graunke <[email protected]> wrote:

> On 04/04/2013 08:13 AM, Paul Berry wrote:
>
>> On 2 April 2013 23:33, Kenneth Graunke <[email protected]> wrote:
>>
> [snip]
>
>      diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
>>     index ce084b4..13dfdd3 100644
>>     --- a/src/glsl/main.cpp
>>     +++ b/src/glsl/main.cpp
>>     @@ -176,7 +176,7 @@ compile_shader(struct gl_context *ctx, struct
>>     gl_shader *shader)
>>          if (!state->error && !shader->ir->is_empty()) {
>>             bool progress;
>>             do {
>>     -        progress = do_common_optimization(shader-**>ir, false,
>> false,
>>     32);
>>     +        progress = do_common_optimization(shader-**>ir, false,
>> false,
>>     32, false);
>>
>>
>> What's the reason for passing false in this case?  It seems like we
>> ought to pass ctx->mvp_with_dp4 in all cases.
>>
>
> Fair enough.  For the standalone compiler, I just picked something rather
> arbitrarily.  ctx->mvp_with_dp4 is false for now.


Ah! I got confused and didn't realize that this call site was in the
standalone compiler.  Makes sense.


>
>
>  For that matter, I'm curious why we don't just check the value of
>> ctx->mvp_with_dp4 from inside do_common_optimization()--it seems like
>> that would be easier to maintain.
>>
>
> It doesn't currently have access to gl_context.  I could instead pass
> that...or move this flag inside ctx->ShaderCompilerOptions and pass a const
> pointer to that instead.  Preferences?


I like the idea of moving the flag inside ctx->ShaderCompilerOptions and
passing a const pointer to that.  Assuming it's not too much trouble.


>
>
>  With that question addressed, this series is:
>>
>> Reviewed-by: Paul Berry <[email protected]>
>>
>
> Thanks Paul!
>
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to