[Mesa-dev] [PATCH V2 8/8] i965: Rename intel_miptree_get_dimensions_for_image()

[Chad Versace]

On Wed 19 Aug 2015, Anuj Phogat wrote:
> This function isn't specific to miptrees. So, drop the "miptree"
> from function name.
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/drivers/dri/i965/intel_fbo.c          | 2 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c  | 6 +++---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h  | 4 ++--
>  src/mesa/drivers/dri/i965/intel_tex_image.c    | 3 +--
>  src/mesa/drivers/dri/i965/intel_tex_validate.c | 3 +--
>  5 files changed, 8 insertions(+), 10 deletions(-)


> @@ -928,8 +928,8 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>  }
>  
>  void
> -intel_miptree_get_dimensions_for_image(struct gl_texture_image *image,
> -                                       int *width, int *height, int *depth)
> +intel_get_image_dims(struct gl_texture_image *image,
> +                     int *width, int *height, int *depth)
>  {
>     switch (image->TexObject->Target) {
>     case GL_TEXTURE_1D_ARRAY:

True, the function isn't specific to miptrees. But it *is* specific to
Intel's RENDER_SURFACE_STATE, as it translates the image's (width,
height, depth), from the perspective of the OpenGL API, to the needs of
Intel hardware.

Now that 'miptree' is removed from the function name, the function name
looks like a mere getter. In that case, it's not clear why the caller
cannot just access image->width, image->height, and image->depth
directly.

So that we all don't forget why this function exists next year, please copy
into the function the relevant portions of this comment from
intel_miptree_create_layout():

      /* For a 1D Array texture the OpenGL API will treat the height0
       * parameter as the number of array slices. For Intel hardware, we treat
       * the 1D array as a 2D Array with a height of 1.
       *
       * So, when we first come through this path to create a 1D Array
       * texture, height0 stores the number of slices, and depth0 is 1. In
       * this case, we want to swap height0 and depth0.
       *
       * Since some miptrees will be created based on the base miptree, we may
       * come through this path and see height0 as 1 and depth0 being the
       * number of slices. In this case we don't need to do the swap.
       */

With such a comment, I think this patch will be ok.

By the way, the height<->depth adjustment that intel_miptree_create_layout()
performs directly beneath that comment, that adjustment duplicates the
height<-> adjustment done by intel_get_image_dims(). That means you may be able
to eliminate intel_get_image_dims() completely, and rely on
intel_miptree_create_layout() to do the adjustment for you. I say "may" because
I haven't investigated it closely enough to be confident.