[Mesa-dev] [PATCH 1/2] i965/miptree: Don't shrink textures when augmenting for more levels

[Jason Ekstrand]

On Tue, Nov 22, 2016 at 9:09 AM, Topi Pohjolainen <
topi.pohjolainen at gmail.com> wrote:

> This was detected when examining CCS_E failures with piglit test:
> "fbo-generatemipmap-formats". Test creates a 2D texture with
> dimensions 293x277. It manually loops over all levels and calls
> glTexImage2D(). Level one triggers creation of full miptree:
> intel_alloc_texture_image_buffer() realizes that there is only one
> level in the miptree and calls intel_miptree_create_for_teximage()
> to re-allocate the miptree with all 9 levels. However, the end result
> is a miptree with level zero dimensions of 292x276.
>

Man... Who decided that you should be able to resize a texture by uploading
something to miplevel 2... It was a bad idea...

Both are

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> Related, and possibly calling for treatment of its own is mip-map
> generation:
> After calling glTexImage2D() against every level test continues by
> replacing content for levels one to eight with data derived from level
> zero by calling glGenerateMipmapEXT(). This results into the miptree
> being allocated anew for every level:
> Mip-map generation goes thru meta which ends up validating the texture
> (brw_validate_textures()->intel_finalize_mipmap_tree()->
> intel_miptree_match_image()) where one finds texture with base level
> size 292:276. This results into new miptree being created for the npot
> size 293:277. Only here intel_finalize_mipmap_tree() is asked for only
> one level, and therefore such is created. Generation for level one in
> turn finds right base level size but only one level when two is needed.
> And the same goes on for all eight levels.
>
> This patch prevents the shrink maintaining the NPOT size of 293x277.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> CC: Jason Ekstrand <jason at jlekstrand.net>
> CC: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/intel_tex_image.c | 29
> +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 4454e53..cbff58a 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -29,6 +29,22 @@
>
>  #define FILE_DEBUG_FLAG DEBUG_TEXTURE
>
> +/* Make sure one doesn't end up shrinking base level zero unnecessarily.
> + * Determining the base level dimension by shifting higher level dimension
> + * ends up in off-by-one value in case base level has NPOT size (for
> example,
> + * 293 != 146 << 1).
> + * Choose the original base level dimension when shifted dimensions agree.
> + * Otherwise assume real resize is intended and use the new shifted value.
> + */
> +static unsigned
> +get_base_dim(unsigned old_base_dim, unsigned new_level_dim, unsigned
> level)
> +{
> +   const unsigned old_level_dim = old_base_dim >> level;
> +   const unsigned new_base_dim = new_level_dim << level;
> +
> +   return old_level_dim == new_level_dim ? old_base_dim : new_base_dim;
>
+}
> +
>  /* Work back from the specified level of the image to the baselevel and
> create a
>   * miptree of that size.
>   */
> @@ -40,6 +56,8 @@ intel_miptree_create_for_teximage(struct brw_context
> *brw,
>  {
>     GLuint lastLevel;
>     int width, height, depth;
> +   const struct intel_mipmap_tree *old_mt = intelObj->mt;
> +   const unsigned level = intelImage->base.Base.Level;
>
>     intel_get_image_dims(&intelImage->base.Base, &width, &height, &depth);
>
> @@ -51,20 +69,23 @@ intel_miptree_create_for_teximage(struct brw_context
> *brw,
>     case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>     case GL_TEXTURE_RECTANGLE:
>     case GL_TEXTURE_EXTERNAL_OES:
> -      assert(intelImage->base.Base.Level == 0);
> +      assert(level == 0);
>        break;
>     case GL_TEXTURE_3D:
> -      depth <<= intelImage->base.Base.Level;
> +      depth = old_mt ? get_base_dim(old_mt->logical_depth0, depth,
> level) :
> +                       depth << level;
>        /* Fall through */
>     case GL_TEXTURE_2D:
>     case GL_TEXTURE_2D_ARRAY:
>     case GL_TEXTURE_CUBE_MAP:
>     case GL_TEXTURE_CUBE_MAP_ARRAY:
> -      height <<= intelImage->base.Base.Level;
> +      height = old_mt ? get_base_dim(old_mt->logical_height0, height,
> level) :
> +                        height << level;
>        /* Fall through */
>     case GL_TEXTURE_1D:
>     case GL_TEXTURE_1D_ARRAY:
> -      width <<= intelImage->base.Base.Level;
> +      width = old_mt ? get_base_dim(old_mt->logical_width0, width,
> level) :
> +                       width << level;
>        break;
>     default:
>        unreachable("Unexpected target");
> --
> 2.5.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161122/b7ce2d18/attachment-0001.html>