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

[Samuel Pitoiset]

On 03/31/2017 08:11 AM, Nicolai Hähnle wrote:
> 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.

I will reorder before pushing.

>
> 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
>>
>
>