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

[Brian Paul]

On 03/28/2016 02:23 PM, Brian Paul wrote:
> 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...

The special GL_TEXTURE_1D_ARRAY code in meta is orthogonal to what 
_mesa_prepare_mipmap_levels() is doing.  I've updated the meta code to 
use _mesa_prepare_mipmap_levels() and will post a patch.  See comments 
there.

I also have a v2 of this patch with your above suggestions.

-Brian