[Mesa-dev] [PATCH 2/6] meta: Rearrange shader creation in setup_glsl_generate_mipmap

[Kenneth Graunke]

On 10/07/2012 08:33 PM, Ian Romanick wrote:
> On 10/05/2012 05:36 PM, Kenneth Graunke wrote:
>> On 10/05/2012 03:56 PM, Ian Romanick wrote:
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>
>>> The diff looks weird, but this moves the code from the first 'if
>>> (ctx->Const.GLSLVersion < 130)' block down into the second block.  It
>>> also moves some variable decalarations closer to their use.
>>>
>>> NOTE: This is a candidate for the 9.0 branch.
>>>
>>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>>> ---
>>>   src/mesa/drivers/common/meta.c | 85
>>> +++++++++++++++++++++---------------------
>>>   1 file changed, 43 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/common/meta.c
>>> b/src/mesa/drivers/common/meta.c
>>> index 0c9ee59..10dc495 100644
>>> --- a/src/mesa/drivers/common/meta.c
>>> +++ b/src/mesa/drivers/common/meta.c
>>> @@ -3047,7 +3047,6 @@ setup_glsl_generate_mipmap(struct gl_context *ctx,
>>>      };
>>>      struct glsl_sampler *sampler;
>>>      const char *vs_source;
>>> -   const char *fs_template;
>>>
>>>      static const char *vs_int_source =
>>>         "#version 130\n"
>>> @@ -3070,11 +3069,41 @@ setup_glsl_generate_mipmap(struct gl_context
>>> *ctx,
>>>         "   out_color = texture(tex2d, texCoords.xy);\n"
>>>         "}\n";
>>>      char *fs_source;
>>> -   const char *extension_mode;
>>>      GLuint vs, fs;
>>>      void *mem_ctx;
>>>
>>> +   /* Check if already initialized */
>>> +   if (mipmap->ArrayObj == 0) {
>>> +
>>> +      /* create vertex array object */
>>> +      _mesa_GenVertexArrays(1, &mipmap->ArrayObj);
>>> +      _mesa_BindVertexArray(mipmap->ArrayObj);
>>> +
>>> +      /* create vertex array buffer */
>>> +      _mesa_GenBuffersARB(1, &mipmap->VBO);
>>> +      _mesa_BindBufferARB(GL_ARRAY_BUFFER_ARB, mipmap->VBO);
>>> +
>>> +      /* setup vertex arrays */
>>> +      _mesa_VertexAttribPointerARB(0, 2, GL_FLOAT, GL_FALSE,
>>> +                                   sizeof(struct vertex), OFFSET(x));
>>> +      _mesa_VertexAttribPointerARB(1, 3, GL_FLOAT, GL_FALSE,
>>> +                                   sizeof(struct vertex), OFFSET(tex));
>>> +   }
>>> +
>>> +   /* Generate a fragment shader program appropriate for the texture
>>> target */
>>> +   sampler = setup_texture_sampler(target, mipmap);
>>> +   assert(sampler != NULL);
>>> +   if (sampler->shader_prog != 0) {
>>> +      mipmap->ShaderProg = sampler->shader_prog;
>>> +      return;
>>> +   }
>>> +
>>> +   mem_ctx = ralloc_context(NULL);
>>> +
>>
>> I would combine this with the declaration:
>>
>>     void *mem_ctx = ralloc_context(NULL);
>
> There's an early return that would have to change of the ralloc_context
> was moved to the top, and the declaration can't be moved down because
> this is C.  Having them split, while annoying, seemed the safest route.
>   Right?

You're right...didn't notice that.  Definitely leave it as-is.