[Mesa-dev] [PATCH] mesa: Skip mipmap generation if there is no source image.

[Brian Paul]

On 10/20/2011 01:23 PM, Ian Romanick wrote:
> On 10/17/2011 01:59 PM, Brian Paul wrote:
>> On 10/17/2011 02:45 PM, Eric Anholt wrote:
>>> The spec doesn't explicitly mandate this behavior, but it seems like
>>> the only sensible thing to do (as opposed to assertion failing or
>>> segfaulting).
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40308
>>> ---
>>>
>>> Oops, looks like I never sent this one out for review.
>>>
>>> src/mesa/main/mipmap.c | 4 +++-
>>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/mesa/main/mipmap.c b/src/mesa/main/mipmap.c
>>> index c2f9226..2b6aead 100644
>>> --- a/src/mesa/main/mipmap.c
>>> +++ b/src/mesa/main/mipmap.c
>>> @@ -2036,7 +2036,9 @@ _mesa_generate_mipmap(struct gl_context *ctx,
>>> GLenum target,
>>>
>>> ASSERT(texObj);
>>> srcImage = _mesa_select_tex_image(ctx, texObj, target,
>>> texObj->BaseLevel);
>>> - ASSERT(srcImage);
>>> +
>>> + if (!srcImage)
>>> + return;
>>>
>>> maxLevel = _mesa_max_texture_levels(ctx, texObj->Target) - 1;
>>> ASSERT(maxLevel>= 0); /* bad target */
>>
>> I think this check should come sooner. We typically do error checking
>> and no-op checking before calling the ctx->Driver.Foobar() functions.
>>
>> The above fix might work for the fallback path, but what about if
>> ctx->Driver.GenerateMipmap points to _mesa_meta_GenerateMipmap() or
>> st_generate_mipmap()?
>>
>> I think I'd try to do the check in _mesa_GenerateMipmapEXT().
>
> Will that also catch the GL_GENERATE_MIPMAP case? I think that's why
> Eric originally put the check farther down the execution path. Though,
> you do raise a good point.

I believe the glTexImage()+GL_GENERATE_MIPMAP path is OK since we only 
call check_gen_mipmap() when we know we have a valid texture image.

-Brian