[Mesa-dev] [PATCH 1/2 v2] meta: use _mesa_prepare_mipmap_levels()

[Brian Paul]

On 03/29/2016 05:44 PM, Ian Romanick wrote:
> From: Brian Paul <brianp at vmware.com>
>
> The prepare_mipmap_level() wrapper for _mesa_prepare_mipmap_level() is
> not needed.  It only served to undo the GL_TEXTURE_1D_ARRAY height/depth
> change was was made before the call to prepare_mipmap_level()
>
> Said another way, regardless of how the meta code manipulates the height/
> depth dims for GL_TEXTURE_1D_ARRAY, the gl_texture_image dimensions are
> correctly set up by _mesa_prepare_mipmap_levels().
>
> Tested by plugging _mesa_meta_GenerateMipmap() into the swrast driver
> and testing with piglit.
>
> v2 (idr): Early out of the mipmap generation loop with dstImage is NULL.
> This can occur for immutable textures that have a limited range of
> levels or in the presense of memory allocation failures.  Fixes
> arb_texture_view-mipgen on Intel platforms.
> ---
>   src/mesa/drivers/common/meta_generate_mipmap.c | 32 +++++++-------------------
>   1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c b/src/mesa/drivers/common/meta_generate_mipmap.c
> index d4b7539..b81e179 100644
> --- a/src/mesa/drivers/common/meta_generate_mipmap.c
> +++ b/src/mesa/drivers/common/meta_generate_mipmap.c
> @@ -137,21 +137,6 @@ _mesa_meta_glsl_generate_mipmap_cleanup(struct gl_context *ctx,
>      _mesa_meta_blit_shader_table_cleanup(ctx, &mipmap->shaders);
>   }
>
> -static GLboolean
> -prepare_mipmap_level(struct gl_context *ctx,
> -                     struct gl_texture_object *texObj, GLuint level,
> -                     GLsizei width, GLsizei height, GLsizei depth,
> -                     GLenum intFormat, mesa_format format)
> -{
> -   if (texObj->Target == GL_TEXTURE_1D_ARRAY) {
> -      /* Work around Mesa expecting the number of array slices in "height". */
> -      height = depth;
> -      depth = 1;
> -   }
> -
> -   return _mesa_prepare_mipmap_level(ctx, texObj, level, width, height, depth,
> -                                     0, intFormat, format);
> -}
>
>   /**
>    * Called via ctx->Driver.GenerateMipmap()
> @@ -270,6 +255,8 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
>      /* texture is already locked, unlock now */
>      _mesa_unlock_texture(ctx, texObj);
>
> +   _mesa_prepare_mipmap_levels(ctx, texObj, baseLevel, maxLevel);
> +
>      for (dstLevel = baseLevel + 1; dstLevel <= maxLevel; dstLevel++) {
>         const struct gl_texture_image *srcImage;
>         struct gl_texture_image *dstImage;
> @@ -309,17 +296,14 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
>         _mesa_texture_parameteriv(ctx, texObj, GL_TEXTURE_MAX_LEVEL,
>                                   (GLint *) &dstLevel, false);
>
> -      if (!prepare_mipmap_level(ctx, texObj, dstLevel,
> -                                dstWidth, dstHeight, dstDepth,
> -                                srcImage->InternalFormat,
> -                                srcImage->TexFormat)) {
> -         /* All done.  We either ran out of memory or we would go beyond the
> -          * last valid level of an immutable texture if we continued.
> -          */
> -         break;
> -      }
>         dstImage = _mesa_select_tex_image(texObj, faceTarget, dstLevel);
>
> +      /* All done.  We either ran out of memory or we would go beyond the last
> +       * valid level of an immutable texture if we continued.
> +       */
> +      if (dstImage == NULL)
> +         break;
> +
>         /* limit minification to src level */
>         _mesa_texture_parameteriv(ctx, texObj, GL_TEXTURE_MAX_LEVEL,
>                                   (GLint *) &srcLevel, false);
>

LGTM.  Thanks for testing & fixing.

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

I'll insert this into my series and push it soon.