[Mesa-dev] [PATCH] meta: Don't modify GL_GENERATE_MIPMAP state when it doesn't exist

[Ian Romanick]

On 08/23/2012 07:56 AM, Brian Paul wrote:
> On 08/22/2012 07:58 PM, Ian Romanick wrote:
>> From: Ian Romanick<ian.d.romanick at intel.com>
>>
>> This is a bit of a hack.  _mesa_meta_GenerateMipmap shouldn't even be
>> used in contexts where GL_GENERATE_MIPMAP doesn't exist (i.e., core
>> profile and ES2) because it uses fixed-function, and fixed-function
>> doesn't exist there either!
>>
>> A GLSL-based _mesa_meta_GenerateMipmap should be available soon.
>>
>> Signed-off-by: Ian Romanick<ian.d.romanick at intel.com>
>> Cc: Kenneth Graunke<kenneth at whitecape.org>
>> ---
>> Without this patch, the in-reply-to patch breaks glGenerateMipmaps in
>> OpenGL ES 2.0 contexts.  With a Reviewed-by, I'll commit this patch
>> before the other 18 patch series.
>>
>>   src/mesa/drivers/common/meta.c |    8 ++++++--
>>   1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta.c
>> b/src/mesa/drivers/common/meta.c
>> index db49d90..5d219c7 100644
>> --- a/src/mesa/drivers/common/meta.c
>> +++ b/src/mesa/drivers/common/meta.c
>> @@ -3010,7 +3010,10 @@ _mesa_meta_GenerateMipmap(struct gl_context
>> *ctx, GLenum target,
>>
>>      _mesa_BindFramebufferEXT(GL_FRAMEBUFFER_EXT, mipmap->FBO);
>>
>> -   _mesa_TexParameteri(target, GL_GENERATE_MIPMAP, GL_FALSE);
>> +   if (ctx->API != API_OPENGLES2&&  ctx->API != API_OPENGL_CORE)
>> +      _mesa_TexParameteri(target, GL_GENERATE_MIPMAP, GL_FALSE);
>> +   else
>> +      assert(!genMipmapSave);
>
> I don't think the GL_GENERATE_MIPMAP tex parameter is supported in GLES
> 3 so wouldn't it be more future-proof to write this as:

Since GLES 3.0 is backwards compatible with GLES 2.0, I've been using 
the same API enum for both.  The difference is in gl_context::Version. 
See also the _mesa_is_gles3 function in context.h.

That said, I don't think there's any harm in changing this condition 
from a negative test to a positive test.

>     if (ctx->API == API_OPENGLES || ctx->API == API_OPENGL)
>        _mesa_TexParameteri(target, GL_GENERATE_MIPMAP, GL_FALSE);
>     else
>        assert(!genMipmapSave);
>
>>
>>      if (ctx->Extensions.EXT_framebuffer_sRGB) {
>>         _mesa_set_enable(ctx, GL_FRAMEBUFFER_SRGB_EXT, GL_FALSE);
>> @@ -3149,7 +3152,8 @@ _mesa_meta_GenerateMipmap(struct gl_context
>> *ctx, GLenum target,
>>      _mesa_meta_end(ctx);
>>
>>      _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, maxLevelSave);
>> -   _mesa_TexParameteri(target, GL_GENERATE_MIPMAP, genMipmapSave);
>> +   if (genMipmapSave)
>> +      _mesa_TexParameteri(target, GL_GENERATE_MIPMAP, genMipmapSave);
>>
>>      _mesa_BindFramebufferEXT(GL_FRAMEBUFFER_EXT, fboSave);
>>   }
>
> Looks good otherwise.
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
>