[Mesa-dev] [PATCH] mesa: fix mipmap generation for immutable, compressed textures

[Brian Paul]

On 09/23/2015 04:03 PM, Roland Scheidegger wrote:
> Ping?
>
> Am 20.09.2015 um 04:33 schrieb sroland at vmware.com:
>> From: Roland Scheidegger <sroland at vmware.com>
>>
>> If the immutable compressed texture didn't have the full mip pyramid,
>> this didn't work, because it tried to generate mip levels for non-existing
>> levels. _mesa_prepare_mipmap_level() would correctly handle this by returning
>> FALSE if the mip level didn't exist, however we actually created the
>> non-existing mip level right before that because we used _mesa_get_tex_image()
>> before calling _mesa_prepare_mipmap_level(). It would then proceed to crash
>> (we allocated the mip level, which is a bad idea on an immutable texture,
>> but didn't initialize the values, leading to assertion failures or segfaults).
>> Fix this by using _mesa_select_tex_image() instead and call it after
>> _mesa_prepare_mipmap_level(), as that function will allocate missing mip levels
>> for non-immutable textures already.
>> This fixes a (2 year old) crash with astromenace which was hack-fixed in ubuntu
>> packages instead: http://bugs.debian.org/718680 (I guess most apps do full mip
>> chains - I believe this app not doing it is actually unintentional, always one
>> level less than full mip chain...).
>>
>> Cc: "10.6 11.0" <mesa-stable at lists.freedesktop.org>
>> ---
>>   src/mesa/main/mipmap.c | 36 +++++++++++++++---------------------
>>   1 file changed, 15 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/mesa/main/mipmap.c b/src/mesa/main/mipmap.c
>> index 2bf5902..ab16c28 100644
>> --- a/src/mesa/main/mipmap.c
>> +++ b/src/mesa/main/mipmap.c
>> @@ -1922,11 +1922,8 @@ generate_mipmap_uncompressed(struct gl_context *ctx, GLenum target,
>>         }
>>
>>         /* get dest gl_texture_image */
>> -      dstImage = _mesa_get_tex_image(ctx, texObj, target, level + 1);
>> -      if (!dstImage) {
>> -         _mesa_error(ctx, GL_OUT_OF_MEMORY, "generating mipmaps");
>> -         return;
>> -      }
>> +      dstImage = _mesa_select_tex_image(texObj, target, level + 1);
>> +      assert(dstImage);
>>
>>         if (target == GL_TEXTURE_1D_ARRAY) {
>>   	 srcDepth = srcHeight;
>> @@ -2110,7 +2107,19 @@ generate_mipmap_compressed(struct gl_context *ctx, GLenum target,
>>                                            srcWidth, srcHeight, srcDepth,
>>                                            &dstWidth, &dstHeight, &dstDepth);
>>         if (!nextLevel)
>> -	 break;
>> +	 goto end;
>> +
>> +      if (!_mesa_prepare_mipmap_level(ctx, texObj, level + 1,
>> +                                      dstWidth, dstHeight, dstDepth,
>> +                                      border, srcImage->InternalFormat,
>> +                                      srcImage->TexFormat)) {
>> +         /* all done */
>> +         goto end;
>> +      }
>> +
>> +      /* get dest gl_texture_image */
>> +      dstImage = _mesa_select_tex_image(texObj, target, level + 1);
>> +      assert(dstImage);
>>
>>         /* Compute dst image strides and alloc memory on first iteration */
>>         temp_dst_row_stride = _mesa_format_row_stride(temp_format, dstWidth);
>> @@ -2124,13 +2133,6 @@ generate_mipmap_compressed(struct gl_context *ctx, GLenum target,
>>   	 }
>>         }
>>
>> -      /* get dest gl_texture_image */
>> -      dstImage = _mesa_get_tex_image(ctx, texObj, target, level + 1);
>> -      if (!dstImage) {
>> -         _mesa_error(ctx, GL_OUT_OF_MEMORY, "generating mipmaps");
>> -         goto end;
>> -      }
>> -
>>         /* for 2D arrays, setup array[depth] of slice pointers */
>>         for (i = 0; i < srcDepth; i++) {
>>            temp_src_slices[i] = temp_src + temp_src_img_stride * i;
>> @@ -2149,14 +2151,6 @@ generate_mipmap_compressed(struct gl_context *ctx, GLenum target,
>>                                     dstWidth, dstHeight, dstDepth,
>>                                     temp_dst_slices, temp_dst_row_stride);
>>
>> -      if (!_mesa_prepare_mipmap_level(ctx, texObj, level + 1,
>> -                                      dstWidth, dstHeight, dstDepth,
>> -                                      border, srcImage->InternalFormat,
>> -                                      srcImage->TexFormat)) {
>> -         /* all done */
>> -         goto end;
>> -      }
>> -
>>         /* The image space was allocated above so use glTexSubImage now */
>>         ctx->Driver.TexSubImage(ctx, 2, dstImage,
>>                                 0, 0, 0, dstWidth, dstHeight, dstDepth,
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

Looks good.

Reviewed-by: Brian Paul <brianp at vmware.com>