[Mesa-dev] [PATCH] intel: Kill intel_mipmap_level::nr_images

Kenneth Graunke kenneth at whitecape.org
Sat Oct 22 17:25:38 PDT 2011


On 10/21/2011 08:45 PM, Chad Versace wrote:
> For all texture targets except GL_TEXTURE_CUBE_MAP, the 'nr_images' and
> 'depth' fields of intel_mipmap_level were identical.  In the exceptional
> case, nr_images == 6 and depth == 1.
> 
> It is simple to determine if a texture is a cube or not, so the presence
> of two fields here was not helpful. Worse, it was confusing.
> 
> This patch removes 'nr_images' and assigns to 'depth' a consistent
> meaning: depth is the number of two-dimensional slices at each 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 each miplevel. It's
>      value, like width and height, varies with miplevel.
>    - For other texture types, depth is 1.

I'm not sure I like this.  What about Cube Arrays?
http://www.opengl.org/registry/specs/ARB/texture_cube_map_array.txt

Admittedly it's not core until 4.0 (AFAICT), but there's nothing
preventing us from doing it earlier...

> As a consequence, parameters were removed from the following function
> signatures:
>     intel_miptree_get_image_offset
>         Remove 'face'.
> 
>     intel_miptree_set_level_info
>         Remove 'nr_images'.
> 
>     i945_miptree_layout
>     brw_miptree_layout_texture
>     brw_miptree_layout_texture_array
>         Remove 'slices'.
> 
> No regressions found on gen6.
> 
> CC: Eric Anholt <eric at anholt.net>
> Signed-off-by: Chad Versace <chad at chad-versace.us>
> ---
>  src/mesa/drivers/dri/i915/i830_texstate.c       |    2 +-
>  src/mesa/drivers/dri/i965/brw_tex_layout.c      |   17 +++----
>  src/mesa/drivers/dri/intel/intel_blit.c         |    1 -
>  src/mesa/drivers/dri/intel/intel_fbo.c          |   11 ++++-
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.c  |   55 ++++++++++++-----------
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.h  |   20 ++++++---
>  src/mesa/drivers/dri/intel/intel_tex.c          |   13 +++++-
>  src/mesa/drivers/dri/intel/intel_tex_copy.c     |    1 -
>  src/mesa/drivers/dri/intel/intel_tex_image.c    |    2 +-
>  src/mesa/drivers/dri/intel/intel_tex_layout.c   |    4 +-
>  src/mesa/drivers/dri/intel/intel_tex_layout.h   |    3 +-
>  src/mesa/drivers/dri/intel/intel_tex_subimage.c |    2 +-
>  src/mesa/drivers/dri/intel/intel_tex_validate.c |    5 +-
>  13 files changed, 78 insertions(+), 58 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i915/i830_texstate.c b/src/mesa/drivers/dri/i915/i830_texstate.c
> index 5f32b92..d44b4e0 100644
> --- a/src/mesa/drivers/dri/i915/i830_texstate.c
> +++ b/src/mesa/drivers/dri/i915/i830_texstate.c
> @@ -145,7 +145,7 @@ i830_update_tex_unit(struct intel_context *intel, GLuint unit, GLuint ss3)
>      */
>     firstImage = tObj->Image[0][tObj->BaseLevel];
>  
> -   intel_miptree_get_image_offset(intelObj->mt, tObj->BaseLevel, 0, 0,
> +   intel_miptree_get_image_offset(intelObj->mt, tObj->BaseLevel, 0,
>  				  &dst_x, &dst_y);
>  
>     drm_intel_bo_reference(intelObj->mt->region->bo);
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index d77bf4d..ac6ade6 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -41,8 +41,7 @@
>  
>  static void
>  brw_miptree_layout_texture_array(struct intel_context *intel,
> -				 struct intel_mipmap_tree *mt,
> -				 int slices)
> +				 struct intel_mipmap_tree *mt)
>  {
>     GLuint align_w;
>     GLuint align_h;
> @@ -58,14 +57,14 @@ brw_miptree_layout_texture_array(struct intel_context *intel,
>     if (mt->compressed)
>        qpitch /= 4;
>  
> -   i945_miptree_layout_2d(mt, slices);
> +   i945_miptree_layout_2d(mt);
>  
>     for (level = mt->first_level; level <= mt->last_level; level++) {
> -      for (q = 0; q < slices; q++) {
> +      for (q = 0; q < mt->depth0; q++) {
>  	 intel_miptree_set_image_offset(mt, level, q, 0, q * qpitch);
>        }
>     }
> -   mt->total_height = qpitch * slices;
> +   mt->total_height = qpitch * mt->depth0;
>  }
>  
>  void
> @@ -82,7 +81,7 @@ brw_miptree_layout(struct intel_context *intel, struct intel_mipmap_tree *mt)
>  	  * pitch of qpitch rows, where qpitch is defined by the equation given
>  	  * in Volume 1 of the BSpec.
>  	  */
> -	 brw_miptree_layout_texture_array(intel, mt, 6);
> +	 brw_miptree_layout_texture_array(intel, mt);
>  	 break;
>        }
>        /* FALLTHROUGH */
> @@ -117,7 +116,7 @@ brw_miptree_layout(struct intel_context *intel, struct intel_mipmap_tree *mt)
>  	 GLint y = 0;
>  	 GLint q, j;
>  
> -	 intel_miptree_set_level_info(mt, level, nr_images,
> +	 intel_miptree_set_level_info(mt, level,
>  				      0, mt->total_height,
>  				      width, height, depth);
>  
> @@ -170,11 +169,11 @@ brw_miptree_layout(struct intel_context *intel, struct intel_mipmap_tree *mt)
>  
>     case GL_TEXTURE_2D_ARRAY:
>     case GL_TEXTURE_1D_ARRAY:
> -      brw_miptree_layout_texture_array(intel, mt, mt->depth0);
> +      brw_miptree_layout_texture_array(intel, mt);
>        break;
>  
>     default:
> -      i945_miptree_layout_2d(mt, 1);
> +      i945_miptree_layout_2d(mt);
>        break;
>     }
>     DBG("%s: %dx%dx%d\n", __FUNCTION__,
> diff --git a/src/mesa/drivers/dri/intel/intel_blit.c b/src/mesa/drivers/dri/intel/intel_blit.c
> index def226c..f6fed00 100644
> --- a/src/mesa/drivers/dri/intel/intel_blit.c
> +++ b/src/mesa/drivers/dri/intel/intel_blit.c
> @@ -538,7 +538,6 @@ intel_set_teximage_alpha_to_one(struct gl_context *ctx,
>     intel_miptree_get_image_offset(intel_image->mt,
>  				  intel_image->base.Base.Level,
>  				  intel_image->base.Base.Face,
> -				  0,
>  				  &image_x, &image_y);
>  
>     x1 = image_x;
> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c
> index 4537f1f..e4b0718 100644
> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> @@ -606,12 +606,19 @@ intel_renderbuffer_set_draw_offset(struct intel_renderbuffer *irb,
>  				   int zoffset)
>  {
>     unsigned int dst_x, dst_y;
> +   unsigned int depth;
> +
> +   if (intel_image->base.Base.TexObject->Target == GL_TEXTURE_CUBE_MAP) {
> +      assert(zoffset == 0);
> +      depth = intel_image->base.Base.Face;
> +   } else {
> +      depth = zoffset;
> +   }
>  
>     /* compute offset of the particular 2D image within the texture region */
>     intel_miptree_get_image_offset(intel_image->mt,
>  				  intel_image->base.Base.Level,
> -				  intel_image->base.Base.Face,
> -				  zoffset,
> +				  depth,
>  				  &dst_x, &dst_y);
>  
>     irb->draw_x = dst_x;
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index bf6a0f6..f427aa6 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -87,6 +87,13 @@ intel_miptree_create_internal(struct intel_context *intel,
>     mt->compressed = compress_byte ? 1 : 0;
>     mt->refcount = 1; 
>  
> +   if (target == GL_TEXTURE_CUBE_MAP) {
> +      assert(depth0 == 1);
> +      mt->depth0 = 6;
> +   } else {
> +      mt->depth0 = depth0;
> +   }
> +
>  #ifdef I915
>     (void) intel;
>     if (intel->is_945)
> @@ -271,7 +278,6 @@ intel_miptree_match_image(struct intel_mipmap_tree *mt,
>  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)
>  {
> @@ -280,17 +286,15 @@ intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
>     mt->level[level].depth = d;
>     mt->level[level].level_x = x;
>     mt->level[level].level_y = y;
> -   mt->level[level].nr_images = nr_images;
>  
>     DBG("%s level %d size: %d,%d,%d offset %d,%d\n", __FUNCTION__,
>         level, w, h, d, x, y);
>  
> -   assert(nr_images);
>     assert(!mt->level[level].x_offset);
>  
> -   mt->level[level].x_offset = malloc(nr_images * sizeof(GLuint));
> +   mt->level[level].x_offset = malloc(d * sizeof(GLuint));
>     mt->level[level].x_offset[0] = mt->level[level].level_x;
> -   mt->level[level].y_offset = malloc(nr_images * sizeof(GLuint));
> +   mt->level[level].y_offset = malloc(d * sizeof(GLuint));
>     mt->level[level].y_offset[0] = mt->level[level].level_y;
>  }
>  
> @@ -303,7 +307,7 @@ intel_miptree_set_image_offset(struct intel_mipmap_tree *mt,
>     if (img == 0 && level == 0)
>        assert(x == 0 && y == 0);
>  
> -   assert(img < mt->level[level].nr_images);
> +   assert(img < mt->level[level].depth);
>  
>     mt->level[level].x_offset[img] = mt->level[level].level_x + x;
>     mt->level[level].y_offset[img] = mt->level[level].level_y + y;
> @@ -316,26 +320,12 @@ intel_miptree_set_image_offset(struct intel_mipmap_tree *mt,
>  
>  void
>  intel_miptree_get_image_offset(struct intel_mipmap_tree *mt,
> -			       GLuint level, GLuint face, GLuint depth,
> +			       GLuint level, GLuint depth,
>  			       GLuint *x, GLuint *y)
>  {
> -   switch (mt->target) {
> -   case GL_TEXTURE_CUBE_MAP_ARB:
> -      *x = mt->level[level].x_offset[face];
> -      *y = mt->level[level].y_offset[face];
> -      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].x_offset[depth];
> -      *y = mt->level[level].y_offset[depth];
> -      break;
> -   default:
> -      *x = mt->level[level].x_offset[0];
> -      *y = mt->level[level].y_offset[0];
> -      break;
> -   }
> +   assert(depth < mt->level[level].depth);
> +   *x = mt->level[level].x_offset[depth];
> +   *y = mt->level[level].y_offset[depth];
>  }
>  
>  /**
> @@ -354,6 +344,8 @@ intel_miptree_copy_teximage(struct intel_context *intel,
>     GLuint height = src_mt->level[level].height;
>     GLuint depth = src_mt->level[level].depth;
>     int slice;
> +   int num_slices;
> +   int slice_begin;
>     void *src, *dst;
>  
>     if (dst_mt->compressed) {
> @@ -365,16 +357,25 @@ intel_miptree_copy_teximage(struct intel_context *intel,
>        width = ALIGN(width, align_w);
>     }
>  
> -   for (slice = 0; slice < depth; slice++) {
> +   if (intelImage->base.Base.TexObject->Target == GL_TEXTURE_CUBE_MAP) {
> +      assert(depth == 6);
> +      slice_begin = face;
> +      num_slices = 1;
> +   } else {
> +      slice_begin = 0;
> +      num_slices = depth;
> +   }
> +
> +   for (slice = slice_begin; slice < slice_begin + num_slices; slice++) {
>        unsigned int dst_x, dst_y, src_x, src_y;
>  
> -      intel_miptree_get_image_offset(dst_mt, level, face, slice,
> +      intel_miptree_get_image_offset(dst_mt, level, slice,
>  				     &dst_x, &dst_y);
>  
>        if (src_mt) {
>  	 /* Copy potentially with the blitter:
>  	  */
> -	 intel_miptree_get_image_offset(src_mt, level, face, slice,
> +	 intel_miptree_get_image_offset(src_mt, level, slice,
>  					&src_x, &src_y);
>  
>  	 DBG("validate blit mt %p %d,%d/%d -> mt %p %d,%d/%d (%dx%d)\n",
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> index e29b943..6d075c6 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> @@ -69,14 +69,23 @@ 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 two-dimensional 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. It's
> +    *      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;
>  
>     /** @{
>      * offsets from level_[xy] to the image for each cube face or depth
> -    * level.
> +    * level. The array's length is \c depth.
>      *
>      * Pretty much have to accept that hardware formats
>      * are going to be so diverse that there is no unified way to
> @@ -168,7 +177,7 @@ bool intel_miptree_match_image(struct intel_mipmap_tree *mt,
>  
>  void
>  intel_miptree_get_image_offset(struct intel_mipmap_tree *mt,
> -			       GLuint level, GLuint face, GLuint depth,
> +			       GLuint level, GLuint depth,
>  			       GLuint *x, GLuint *y);
>  
>  void
> @@ -177,7 +186,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.c b/src/mesa/drivers/dri/intel/intel_tex.c
> index 054ae42..80cd7a5 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex.c
> @@ -168,8 +168,17 @@ intel_map_texture_image(struct gl_context *ctx,
>        void *base = intel_region_map(intel, mt->region, mode);
>        unsigned int image_x, image_y;
>  
> -      intel_miptree_get_image_offset(mt, tex_image->Level, tex_image->Face,
> -				     slice, &image_x, &image_y);
> +      unsigned int depth;
> +      if (tex_image->TexObject->Target == GL_TEXTURE_CUBE_MAP) {
> +	 assert(slice == 0);
> +	 depth = tex_image->Face;
> +      } else {
> +	 assert(tex_image->Face == 0);
> +	 depth = slice;
> +      }
> +
> +      intel_miptree_get_image_offset(mt, tex_image->Level, depth,
> +				     &image_x, &image_y);
>        x += image_x;
>        y += image_y;
>  
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_copy.c b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> index 2df4ef6..f7cab87 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_copy.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> @@ -116,7 +116,6 @@ intel_copy_texsubimage(struct intel_context *intel,
>        intel_miptree_get_image_offset(intelImage->mt,
>  				     intelImage->base.Base.Level,
>  				     intelImage->base.Base.Face,
> -				     0,
>  				     &image_x, &image_y);
>  
>        /* The blitter can't handle Y-tiled buffers. */
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_image.c b/src/mesa/drivers/dri/intel/intel_tex_image.c
> index 4710bf6..d9c9dfd 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_image.c
> @@ -195,7 +195,7 @@ try_pbo_upload(struct gl_context *ctx,
>        src_stride = width;
>  
>     intel_miptree_get_image_offset(intelImage->mt, intelImage->base.Base.Level,
> -				  intelImage->base.Base.Face, 0,
> +				  intelImage->base.Base.Face,
>  				  &dst_x, &dst_y);
>  
>     dst_stride = intelImage->mt->region->pitch;
> 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_subimage.c b/src/mesa/drivers/dri/intel/intel_tex_subimage.c
> index 6629588..1a015cc 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_subimage.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_subimage.c
> @@ -113,7 +113,7 @@ intel_blit_texsubimage(struct gl_context * ctx,
>     dstRowStride = pitch;
>  
>     intel_miptree_get_image_offset(intelImage->mt, level,
> -				  intelImage->base.Base.Face, 0,
> +				  intelImage->base.Base.Face,
>  				  &blit_x, &blit_y);
>     blit_x += xoffset;
>     blit_y += yoffset;
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_validate.c b/src/mesa/drivers/dri/intel/intel_tex_validate.c
> index 4012400..68dc76e 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_validate.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_validate.c
> @@ -161,7 +161,7 @@ intel_tex_map_image_for_swrast(struct intel_context *intel,
>         * share code with the normal path.
>         */
>        for (i = 0; i < mt->level[level].depth; i++) {
> -	 intel_miptree_get_image_offset(mt, level, face, i, &x, &y);
> +	 intel_miptree_get_image_offset(mt, level, i, &x, &y);
>  	 intel_image->base.Base.ImageOffsets[i] = x + y * mt->region->pitch;
>        }
>  
> @@ -169,8 +169,7 @@ intel_tex_map_image_for_swrast(struct intel_context *intel,
>  
>        intel_image->base.Base.Data = intel_region_map(intel, mt->region, mode);
>     } else {
> -      assert(mt->level[level].depth == 1);
> -      intel_miptree_get_image_offset(mt, level, face, 0, &x, &y);
> +      intel_miptree_get_image_offset(mt, level, face, &x, &y);
>        intel_image->base.Base.ImageOffsets[0] = 0;
>  
>        DBG("%s: (%d,%d) -> (%d, %d)/%d\n",



More information about the mesa-dev mailing list