[Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images [v2]

[Kenneth Graunke]

On 11/18/2011 01:52 PM, Chad Versace wrote:
[snip]
> @@ -335,23 +338,18 @@ intel_miptree_get_image_offset(struct intel_mipmap_tree *mt,
>  			       GLuint level, GLuint face, GLuint depth,
>  			       GLuint *x, GLuint *y)
>  {
> -   switch (mt->target) {
> -   case GL_TEXTURE_CUBE_MAP_ARB:
> -      *x = mt->level[level].slice[face].x_offset;
> -      *y = mt->level[level].slice[face].y_offset;
> -      break;
> -   case GL_TEXTURE_3D:
> -   case GL_TEXTURE_2D_ARRAY_EXT:
> -   case GL_TEXTURE_1D_ARRAY_EXT:
> -      assert(depth < mt->level[level].nr_images);
> -      *x = mt->level[level].slice[depth].x_offset;
> -      *y = mt->level[level].slice[depth].y_offset;
> -      break;
> -   default:
> -      *x = mt->level[level].slice[0].x_offset;
> -      *y = mt->level[level].slice[0].y_offset;
> -      break;
> +   int slice;
> +
> +   if (face > 0) {
> +      assert(face < 6);
> +      assert(depth == 0);
> +      slice = face;
> +   } else {
> +      slice = depth;
>     }

I find the face > 0 check confusing.  For cube face 0, you're falling
through to the array case and relying the fact that depth == 0 for
cubemaps.  Yes, it works, but...bizarre.

You're also relying on depth == 0 for non-cube/non-array cases, but that
seems entirely reasonable to me.

Perhaps just change the (face > 0) check to (mt->target ==
GL_TEXTURE_CUBE_MAP)?  That seems clear enough.

Technically you could just drop the changes in this function (they're
not necessary), but I do like the cleanup.

> +   *x = mt->level[level].slice[slice].x_offset;
> +   *y = mt->level[level].slice[slice].y_offset;
>  }
>  
>  static void
> @@ -429,7 +427,7 @@ intel_miptree_copy_teximage(struct intel_context *intel,
>     struct intel_mipmap_tree *src_mt = intelImage->mt;
>     int level = intelImage->base.Base.Level;
>     int face = intelImage->base.Base.Face;
> -   GLuint depth = src_mt->level[level].depth;
> +   GLuint depth = intelImage->base.Base.Depth;
>  
>     for (int slice = 0; slice < depth; slice++) {
>        intel_miptree_copy_slice(intel, dst_mt, src_mt, level, face, slice);
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> index 2cad793..8f024f9 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> @@ -69,16 +69,25 @@ struct intel_mipmap_level
>     GLuint level_y;
>     GLuint width;
>     GLuint height;
> -   /** Depth of the mipmap at this level: 1 for 1D/2D/CUBE, n for 3D. */
> +
> +   /**
> +    * \brief Number of 2D slices in this miplevel.
> +    *
> +    * The exact semantics of depth varies according to the texture target:
> +    *    - For GL_TEXTURE_CUBE_MAP, depth is 6.
> +    *    - For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It is
> +    *      identical for all miplevels in the texture.
> +    *    - For GL_TEXTURE_3D, it is the texture's depth at this miplevel. Its
> +    *      value, like width and height, varies with miplevel.
> +    *    - For other texture types, depth is 1.
> +    */
>     GLuint depth;
> -   /** Number of images at this level: 1 for 1D/2D, 6 for CUBE, depth for 3D */
> -   GLuint nr_images;
>  
>     /**
>      * \brief List of 2D images in this mipmap level.
>      *
>      * This may be a list of cube faces, array slices in 2D array texture, or
> -    * layers in a 3D texture. The list's length is \c nr_images.
> +    * layers in a 3D texture. The list's length is \c depth.
>      */
>     struct intel_mipmap_slice {
>        /**
> @@ -205,7 +214,6 @@ intel_miptree_get_dimensions_for_image(struct gl_texture_image *image,
>  
>  void intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
>                                    GLuint level,
> -                                  GLuint nr_images,
>                                    GLuint x, GLuint y,
>                                    GLuint w, GLuint h, GLuint d);
>  
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c b/src/mesa/drivers/dri/intel/intel_tex_layout.c
> index 64f4a70..e6324cf 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_layout.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_layout.c
> @@ -50,7 +50,7 @@ intel_get_texture_alignment_unit(gl_format format,
>     }
>  }
>  
> -void i945_miptree_layout_2d(struct intel_mipmap_tree *mt, int nr_images)
> +void i945_miptree_layout_2d(struct intel_mipmap_tree *mt)
>  {
>     GLuint align_h, align_w;
>     GLuint level;
> @@ -93,7 +93,7 @@ void i945_miptree_layout_2d(struct intel_mipmap_tree *mt, int nr_images)
>     for ( level = mt->first_level ; level <= mt->last_level ; level++ ) {
>        GLuint img_height;
>  
> -      intel_miptree_set_level_info(mt, level, nr_images, x, y, width,
> +      intel_miptree_set_level_info(mt, level, x, y, width,
>  				   height, depth);
>  
>        img_height = ALIGN(height, align_h);
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.h b/src/mesa/drivers/dri/intel/intel_tex_layout.h
> index 257c07c..c6c865d 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_layout.h
> +++ b/src/mesa/drivers/dri/intel/intel_tex_layout.h
> @@ -38,7 +38,6 @@ static INLINE GLuint minify( GLuint d )
>     return MAX2(1, d>>1);
>  }
>  
> -extern void i945_miptree_layout_2d(struct intel_mipmap_tree *mt,
> -				   int nr_images);
> +extern void i945_miptree_layout_2d(struct intel_mipmap_tree *mt);
>  void intel_get_texture_alignment_unit(gl_format format,
>  				      unsigned int *w, unsigned int *h);
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_validate.c b/src/mesa/drivers/dri/intel/intel_tex_validate.c
> index f4c1a68..748fbdc 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_validate.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_validate.c
> @@ -154,7 +154,7 @@ intel_tex_map_image_for_swrast(struct intel_context *intel,
>  
>        intel_image->base.Data = intel_region_map(intel, mt->region, mode);
>     } else {
> -      assert(mt->level[level].depth == 1);
> +      assert(intel_image->base.Base.Depth == 1);
>        intel_miptree_get_image_offset(mt, level, face, 0, &x, &y);
>  
>        DBG("%s: (%d,%d) -> (%d, %d)/%d\n",