[Mesa-dev] [PATCH 1/2] st/mesa: add st_convert_image()

[Nicolai Hähnle]

On 30.03.2017 18:55, Samuel Pitoiset wrote:
> Should be used by the state tracker when glGetImageHandleARB()
> is called in order to create a pipe_image_view template.
>
> v2: - make 'st' const
>     - describe the function
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/mesa/state_tracker/st_atom_image.c | 103 ++++++++++++++++++---------------
>  src/mesa/state_tracker/st_texture.h    |   6 ++
>  2 files changed, 62 insertions(+), 47 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_image.c b/src/mesa/state_tracker/st_atom_image.c
> index 5dd2cd64f9..805014c44a 100644
> --- a/src/mesa/state_tracker/st_atom_image.c
> +++ b/src/mesa/state_tracker/st_atom_image.c
> @@ -44,6 +44,61 @@
>  #include "st_program.h"
>  #include "st_format.h"
>
> +void
> +st_convert_image(const struct st_context *st, const struct gl_image_unit *u,
> +                 struct pipe_image_view *img)
> +{
> +   struct st_texture_object *stObj = st_texture_object(u->TexObj);
> +
> +   img->resource = stObj->pt;
> +   img->format = st_mesa_format_to_pipe_format(st, u->_ActualFormat);
> +
> +   switch (u->Access) {
> +   case GL_READ_ONLY:
> +      img->access = PIPE_IMAGE_ACCESS_READ;
> +      break;
> +   case GL_WRITE_ONLY:
> +      img->access = PIPE_IMAGE_ACCESS_WRITE;
> +      break;
> +   case GL_READ_WRITE:
> +      img->access = PIPE_IMAGE_ACCESS_READ_WRITE;
> +      break;
> +   default:
> +      unreachable("bad gl_image_unit::Access");
> +   }
> +
> +   if (stObj->pt->target == PIPE_BUFFER) {
> +      unsigned base, size;
> +
> +      base = stObj->base.BufferOffset;
> +      assert(base < stObj->pt->width0);
> +      size = MIN2(stObj->pt->width0 - base, (unsigned)stObj->base.BufferSize);
> +
> +      img->u.buf.offset = base;
> +      img->u.buf.size = size;
> +   } else {
> +      img->u.tex.level = u->Level + stObj->base.MinLevel;
> +      if (stObj->pt->target == PIPE_TEXTURE_3D) {
> +         if (u->Layered) {
> +            img->u.tex.first_layer = 0;
> +            img->u.tex.last_layer = u_minify(stObj->pt->depth0, img->u.tex.level) - 1;
> +         } else {
> +            img->u.tex.first_layer = u->_Layer;
> +            img->u.tex.last_layer = u->_Layer;
> +         }
> +      } else {
> +         img->u.tex.first_layer = u->_Layer + stObj->base.MinLayer;
> +         img->u.tex.last_layer = u->_Layer + stObj->base.MinLayer;
> +         if (u->Layered && img->resource->array_size > 1) {
> +            if (stObj->base.Immutable)
> +               img->u.tex.last_layer += stObj->base.NumLayers - 1;
> +            else
> +               img->u.tex.last_layer += img->resource->array_size - 1;
> +         }
> +      }
> +   }
> +}
> +
>  static void
>  st_bind_images(struct st_context *st, struct gl_program *prog,
>                 enum pipe_shader_type shader_type)
> @@ -70,53 +125,7 @@ st_bind_images(struct st_context *st, struct gl_program *prog,
>           continue;
>        }
>
> -      img->resource = stObj->pt;
> -      img->format = st_mesa_format_to_pipe_format(st, u->_ActualFormat);
> -
> -      switch (u->Access) {
> -      case GL_READ_ONLY:
> -         img->access = PIPE_IMAGE_ACCESS_READ;
> -         break;
> -      case GL_WRITE_ONLY:
> -         img->access = PIPE_IMAGE_ACCESS_WRITE;
> -         break;
> -      case GL_READ_WRITE:
> -         img->access = PIPE_IMAGE_ACCESS_READ_WRITE;
> -         break;
> -      default:
> -         unreachable("bad gl_image_unit::Access");
> -      }
> -
> -      if (stObj->pt->target == PIPE_BUFFER) {
> -         unsigned base, size;
> -
> -         base = stObj->base.BufferOffset;
> -         assert(base < stObj->pt->width0);
> -         size = MIN2(stObj->pt->width0 - base, (unsigned)stObj->base.BufferSize);
> -
> -         img->u.buf.offset = base;
> -         img->u.buf.size = size;
> -      } else {
> -         img->u.tex.level = u->Level + stObj->base.MinLevel;
> -         if (stObj->pt->target == PIPE_TEXTURE_3D) {
> -            if (u->Layered) {
> -               img->u.tex.first_layer = 0;
> -               img->u.tex.last_layer = u_minify(stObj->pt->depth0, img->u.tex.level) - 1;
> -            } else {
> -               img->u.tex.first_layer = u->_Layer;
> -               img->u.tex.last_layer = u->_Layer;
> -            }
> -         } else {
> -            img->u.tex.first_layer = u->_Layer + stObj->base.MinLayer;
> -            img->u.tex.last_layer = u->_Layer + stObj->base.MinLayer;
> -            if (u->Layered && img->resource->array_size > 1) {
> -               if (stObj->base.Immutable)
> -                  img->u.tex.last_layer += stObj->base.NumLayers - 1;
> -               else
> -                  img->u.tex.last_layer += img->resource->array_size - 1;
> -            }
> -         }
> -      }
> +      st_convert_image(st, u, img);
>     }
>     cso_set_shader_images(st->cso_context, shader_type, 0,
>                           prog->info.num_images, images);
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index 0ce7989562..d9584c9acd 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -254,4 +254,10 @@ st_create_color_map_texture(struct gl_context *ctx);
>  bool
>  st_etc_fallback(struct st_context *st, struct gl_texture_image *texImage);
>
> +/* Convert a gl_image_unit object to a pipe_image_view object.
> + */

I think the more common pattern is to have a Doxygen comment in the .c file.

Also, looks like the order of the patches should be reversed.

Apart from those nits, it looks good to me.

Cheers,
Nicolai


> +void
> +st_convert_image(const struct st_context *st, const struct gl_image_unit *u,
> +                 struct pipe_image_view *img);
> +
>  #endif
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.