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.roman...@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.roman...@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?
Otherwise, this looks okay. Coalescing the two if-statements seems like a good idea. It does also let you kill both mistakes with a single line in the next patch. For the series: Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> This also replaces Oliver's series.if (ctx->Const.GLSLVersion < 130) { + const char *fs_template; + const char *extension_mode; + vs_source = "attribute vec2 position;\n" "attribute vec3 textureCoords;\n" @@ -3092,7 +3121,18 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, "{\n" " gl_FragColor = %s(texSampler, %s);\n" "}\n"; - } else { + + extension_mode = ((target == GL_TEXTURE_1D_ARRAY) || + (target == GL_TEXTURE_2D_ARRAY)) ? + "require" : "disable"; + + fs_source = ralloc_asprintf(mem_ctx, fs_template, + extension_mode, sampler->type, + sampler->func, sampler->texcoords); + } + else { + const char *fs_template; + vs_source = "#version 130\n" "in vec2 position;\n" @@ -3113,46 +3153,7 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, "{\n" " out_color = texture(texSampler, %s);\n" "}\n"; - } - - /* 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); - - if (ctx->Const.GLSLVersion < 130) { - extension_mode = ((target == GL_TEXTURE_1D_ARRAY) || - (target == GL_TEXTURE_2D_ARRAY)) ? - "require" : "disable"; - - fs_source = ralloc_asprintf(mem_ctx, fs_template, - extension_mode, sampler->type, - sampler->func, sampler->texcoords); - } - else { fs_source = ralloc_asprintf(mem_ctx, fs_template, sampler->type, "vec4", sampler->texcoords);
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
