[Mesa-dev] [PATCH 1/2] mesa: new _mesa_prepare_mipmap_levels() function for mipmap generation

[Brian Paul]

On 03/28/2016 12:39 PM, Ian Romanick wrote:
> On 03/26/2016 10:21 AM, Brian Paul wrote:
>> Simplifies the loops in generate_mipmap_uncompressed() and
>> generate_mipmap_compressed().  Will be used in the state tracker too.
>> Could probably be used in the meta code.  If so, some additional
>> clean-ups can be done after that.
>
> I'm in favor of just about anything that can lead to clean ups in meta.
>   I have a couple comments below.
>
>> ---
>>   src/mesa/main/mipmap.c | 89 ++++++++++++++++++++++++++++++++------------------
>>   src/mesa/main/mipmap.h |  5 +++
>>   2 files changed, 63 insertions(+), 31 deletions(-)
>>
>> diff --git a/src/mesa/main/mipmap.c b/src/mesa/main/mipmap.c
>> index 5a02780..02d236d 100644
>> --- a/src/mesa/main/mipmap.c
>> +++ b/src/mesa/main/mipmap.c
>> @@ -1872,6 +1872,50 @@ _mesa_prepare_mipmap_level(struct gl_context *ctx,
>>   }
>>
>>
>> +/**
>> + * Prepare all mipmap levels beyond 'baseLevel' for mipmap generation.
>> + * When finished, all the gl_texture_image structures for the smaller
>> + * mipmap levels will be consistent with the base level (in terms of
>> + * dimensions, format, etc).
>> + */
>> +void
>> +_mesa_prepare_mipmap_levels(struct gl_context *ctx,
>> +                            struct gl_texture_object *texObj,
>> +                            GLuint baseLevel, GLuint maxLevel)
>
> Can these be plain unsigned?

Sure.


>> +{
>> +   const struct gl_texture_image *baseImage =
>> +      _mesa_select_tex_image(texObj, texObj->Target, baseLevel);
>> +   const GLint border = 0;
>> +   GLint width = baseImage->Width;
>> +   GLint height = baseImage->Height;
>> +   GLint depth = baseImage->Depth;
>> +   const GLenum intFormat = baseImage->InternalFormat;
>> +   const mesa_format texFormat = baseImage->TexFormat;
>> +   GLint newWidth, newHeight, newDepth;
>> +   GLuint level;
>> +
>> +   /* Prepare baseLevel + 1, baseLevel + 2, ... */
>> +   for (level = baseLevel + 1; level <= maxLevel; level++) {
>
> Now that we can use more C99, I'm in favor of
>
>     for (unsigned level = baseLevel + 1; level <= maxLevel; level++) {

OK.


>> +      if (!_mesa_next_mipmap_level_size(texObj->Target, border,
>> +                                        width, height, depth,
>> +                                        &newWidth, &newHeight, &newDepth)) {
>
> Meta has some special handling for GL_TEXTURE_1D_ARRAY. I think that
> needs to be done here too.

I'll take a look...

Thanks.

-Brian

>
>> +         /* all done */
>> +         break;
>> +      }
>> +
>> +      if (!_mesa_prepare_mipmap_level(ctx, texObj, level,
>> +                                      newWidth, newHeight, newDepth,
>> +                                      border, intFormat, texFormat)) {
>> +         break;
>> +      }
>> +
>> +      width = newWidth;
>> +      height = newHeight;
>> +      depth = newDepth;
>> +   }
>> +}
>> +
>> +
>>   static void
>>   generate_mipmap_uncompressed(struct gl_context *ctx, GLenum target,
>>   			     struct gl_texture_object *texObj,
>> @@ -1892,7 +1936,6 @@ generate_mipmap_uncompressed(struct gl_context *ctx, GLenum target,
>>         GLint dstWidth, dstHeight, dstDepth;
>>         GLint border;
>>         GLint slice;
>> -      GLboolean nextLevel;
>>         GLubyte **srcMaps, **dstMaps;
>>         GLboolean success = GL_TRUE;
>>
>> @@ -1904,22 +1947,14 @@ generate_mipmap_uncompressed(struct gl_context *ctx, GLenum target,
>>         srcDepth = srcImage->Depth;
>>         border = srcImage->Border;
>>
>> -      nextLevel = _mesa_next_mipmap_level_size(target, border,
>> -                                         srcWidth, srcHeight, srcDepth,
>> -                                         &dstWidth, &dstHeight, &dstDepth);
>> -      if (!nextLevel)
>> -         return;
>> -
>> -      if (!_mesa_prepare_mipmap_level(ctx, texObj, level + 1,
>> -                                      dstWidth, dstHeight, dstDepth,
>> -                                      border, srcImage->InternalFormat,
>> -                                      srcImage->TexFormat)) {
>> -         return;
>> -      }
>> -
>>         /* get dest gl_texture_image */
>>         dstImage = _mesa_select_tex_image(texObj, target, level + 1);
>> -      assert(dstImage);
>> +      if (!dstImage) {
>> +         break;
>> +      }
>> +      dstWidth = dstImage->Width;
>> +      dstHeight = dstImage->Height;
>> +      dstDepth = dstImage->Depth;
>>
>>         if (target == GL_TEXTURE_1D_ARRAY) {
>>   	 srcDepth = srcHeight;
>> @@ -2087,7 +2122,6 @@ generate_mipmap_compressed(struct gl_context *ctx, GLenum target,
>>         GLint srcWidth, srcHeight, srcDepth;
>>         GLint dstWidth, dstHeight, dstDepth;
>>         GLint border;
>> -      GLboolean nextLevel;
>>         GLuint temp_dst_row_stride, temp_dst_img_stride; /* in bytes */
>>         GLint i;
>>
>> @@ -2099,23 +2133,14 @@ generate_mipmap_compressed(struct gl_context *ctx, GLenum target,
>>         srcDepth = srcImage->Depth;
>>         border = srcImage->Border;
>>
>> -      nextLevel = _mesa_next_mipmap_level_size(target, border,
>> -                                         srcWidth, srcHeight, srcDepth,
>> -                                         &dstWidth, &dstHeight, &dstDepth);
>> -      if (!nextLevel)
>> -	 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);
>> +      if (!dstImage) {
>> +         break;
>> +      }
>> +      dstWidth = dstImage->Width;
>> +      dstHeight = dstImage->Height;
>> +      dstDepth = dstImage->Depth;
>>
>>         /* Compute dst image strides and alloc memory on first iteration */
>>         temp_dst_row_stride = _mesa_format_row_stride(temp_format, dstWidth);
>> @@ -2194,6 +2219,8 @@ _mesa_generate_mipmap(struct gl_context *ctx, GLenum target,
>>
>>      maxLevel = MIN2(maxLevel, texObj->MaxLevel);
>>
>> +   _mesa_prepare_mipmap_levels(ctx, texObj, texObj->BaseLevel, maxLevel);
>> +
>>      if (_mesa_is_format_compressed(srcImage->TexFormat)) {
>>         generate_mipmap_compressed(ctx, target, texObj, srcImage, maxLevel);
>>      } else {
>> diff --git a/src/mesa/main/mipmap.h b/src/mesa/main/mipmap.h
>> index c0366d3..0ed7934 100644
>> --- a/src/mesa/main/mipmap.h
>> +++ b/src/mesa/main/mipmap.h
>> @@ -47,6 +47,11 @@ _mesa_prepare_mipmap_level(struct gl_context *ctx,
>>                              GLsizei width, GLsizei height, GLsizei depth,
>>                              GLsizei border, GLenum intFormat, mesa_format format);
>>
>> +void
>> +_mesa_prepare_mipmap_levels(struct gl_context *ctx,
>> +                            struct gl_texture_object *texObj,
>> +                            GLuint baseLevel, GLuint maxLevel);
>> +
>>   extern void
>>   _mesa_generate_mipmap(struct gl_context *ctx, GLenum target,
>>                         struct gl_texture_object *texObj);
>>
>