[Mesa-dev] [PATCH 2/2] st/mesa: improve sampler view handling

Brian Paul brianp at vmware.com
Tue Mar 25 12:54:53 PDT 2014


Minor nits below.

On 03/25/2014 11:28 AM, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> Keep a dynamically increasing array of all the views
> created for a texture instead of just the last one.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   src/mesa/state_tracker/st_atom_sampler.c | 21 ++++++++---
>   src/mesa/state_tracker/st_atom_texture.c | 53 +++++++++++++---------------
>   src/mesa/state_tracker/st_cb_eglimage.c  |  2 +-
>   src/mesa/state_tracker/st_cb_texture.c   | 20 +++++------
>   src/mesa/state_tracker/st_gen_mipmap.c   |  2 +-
>   src/mesa/state_tracker/st_texture.c      | 60 ++++++++++++++++++++++++++++++--
>   src/mesa/state_tracker/st_texture.h      | 16 +++++++--
>   src/mesa/state_tracker/st_vdpau.c        | 11 +++---
>   8 files changed, 129 insertions(+), 56 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_sampler.c b/src/mesa/state_tracker/st_atom_sampler.c
> index ec2bab2..f89013f 100644
> --- a/src/mesa/state_tracker/st_atom_sampler.c
> +++ b/src/mesa/state_tracker/st_atom_sampler.c
> @@ -182,15 +182,26 @@ convert_sampler(struct st_context *st,
>          msamp->BorderColor.ui[3]) {
>         const struct st_texture_object *stobj = st_texture_object_const(texobj);
>         const GLboolean is_integer = texobj->_IsIntegerFormat;
> +      const struct pipe_sampler_view *sv = NULL;
>         union pipe_color_union border_color;
> +      GLuint i;
> +
> +      /* Just search for the first used view */

/* We can do this because the swizzle is per-texture, not per context. */


> +      /* XXX: clean that up to not use the sampler view at all */
> +      for (i = 0; i < stobj->num_sampler_views; ++i) {
> +         if (stobj->sampler_views[i]) {
> +            sv = stobj->sampler_views[i];
> +            break;
> +         }
> +      }
>
> -      if (st->apply_texture_swizzle_to_border_color && stobj->sampler_view) {
> +      if (st->apply_texture_swizzle_to_border_color && sv) {
>            const unsigned char swz[4] =
>            {
> -            stobj->sampler_view->swizzle_r,
> -            stobj->sampler_view->swizzle_g,
> -            stobj->sampler_view->swizzle_b,
> -            stobj->sampler_view->swizzle_a,
> +            sv->swizzle_r,
> +            sv->swizzle_g,
> +            sv->swizzle_b,
> +            sv->swizzle_a,
>            };
>
>            st_translate_color(&msamp->BorderColor,
> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
> index 75e6fac..245e615 100644
> --- a/src/mesa/state_tracker/st_atom_texture.c
> +++ b/src/mesa/state_tracker/st_atom_texture.c
> @@ -186,40 +186,47 @@ st_create_texture_sampler_view_from_stobj(struct pipe_context *pipe,
>
>
>   static struct pipe_sampler_view *
> -st_get_texture_sampler_view_from_stobj(struct st_texture_object *stObj,
> -				       struct pipe_context *pipe,
> +st_get_texture_sampler_view_from_stobj(struct st_context *st,
> +                                       struct st_texture_object *stObj,
>                                          const struct gl_sampler_object *samp,
>   				       enum pipe_format format)
>   {
> +   struct pipe_sampler_view **sv;
> +
>      if (!stObj || !stObj->pt) {
>         return NULL;
>      }
>
> -   if (!stObj->sampler_view) {
> -      stObj->sampler_view =
> -         st_create_texture_sampler_view_from_stobj(pipe, stObj, samp, format);
> +   sv = st_texture_get_sampler_view(st, stObj);
> +
> +   /* if sampler view has changed dereference it */
> +   if (*sv) {
> +      if (check_sampler_swizzle(*sv, stObj->base._Swizzle, stObj->base.DepthMode) ||
> +	  (format != (*sv)->format) ||
> +	  stObj->base.BaseLevel != (*sv)->u.tex.first_level) {
> +	 pipe_sampler_view_reference(sv, NULL);
> +      }
> +   }
> +
> +   if (!*sv) {
> +      *sv = st_create_texture_sampler_view_from_stobj(st->pipe, stObj, samp, format);
>
> -   } else if (stObj->sampler_view->context != pipe) {
> +   } else if ((*sv)->context != st->pipe) {
>         /* Recreate view in correct context, use existing view as template */
> -      /* XXX: This isn't optimal, we should try to use more than one view.
> -              Otherwise we create/destroy the view all the time
> -       */
> -      struct pipe_sampler_view *sv =
> -         pipe->create_sampler_view(pipe, stObj->pt, stObj->sampler_view);
> -      pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> -      stObj->sampler_view = sv;
> +      struct pipe_sampler_view *new_sv =
> +         st->pipe->create_sampler_view(st->pipe, stObj->pt, *sv);
> +      pipe_sampler_view_reference(sv, NULL);
> +      *sv = new_sv;
>      }
>
> -   return stObj->sampler_view;
> +   return *sv;
>   }
>
> -
>   static GLboolean
>   update_single_texture(struct st_context *st,
>                         struct pipe_sampler_view **sampler_view,
>   		      GLuint texUnit)
>   {
> -   struct pipe_context *pipe = st->pipe;
>      struct gl_context *ctx = st->ctx;
>      const struct gl_sampler_object *samp;
>      struct gl_texture_object *texObj;
> @@ -258,19 +265,7 @@ update_single_texture(struct st_context *st,
>         }
>      }
>
> -   /* if sampler view has changed dereference it */
> -   if (stObj->sampler_view) {
> -      if (check_sampler_swizzle(stObj->sampler_view,
> -				stObj->base._Swizzle,
> -				stObj->base.DepthMode) ||
> -	  (view_format != stObj->sampler_view->format) ||
> -	  stObj->base.BaseLevel != stObj->sampler_view->u.tex.first_level) {
> -	 pipe_sampler_view_release(pipe, &stObj->sampler_view);
> -      }
> -   }
> -
> -   *sampler_view = st_get_texture_sampler_view_from_stobj(stObj, pipe,
> -							  samp,
> +   *sampler_view = st_get_texture_sampler_view_from_stobj(st, stObj, samp,
>   							  view_format);
>      return GL_TRUE;
>   }
> diff --git a/src/mesa/state_tracker/st_cb_eglimage.c b/src/mesa/state_tracker/st_cb_eglimage.c
> index 561967d..34eb809 100644
> --- a/src/mesa/state_tracker/st_cb_eglimage.c
> +++ b/src/mesa/state_tracker/st_cb_eglimage.c
> @@ -124,7 +124,7 @@ st_bind_surface(struct gl_context *ctx, GLenum target,
>
>      /* FIXME create a non-default sampler view from the pipe_surface? */
>      pipe_resource_reference(&stObj->pt, ps->texture);
> -   pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> +   st_texture_release_all_sampler_views(stObj);
>      pipe_resource_reference(&stImage->pt, stObj->pt);
>
>      stObj->width0 = ps->width;
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index f0bf374..353415b 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -151,13 +151,11 @@ static void
>   st_DeleteTextureObject(struct gl_context *ctx,
>                          struct gl_texture_object *texObj)
>   {
> -   struct st_context *st = st_context(ctx);
>      struct st_texture_object *stObj = st_texture_object(texObj);
> -   if (stObj->pt)
> -      pipe_resource_reference(&stObj->pt, NULL);
> -   if (stObj->sampler_view) {
> -      pipe_sampler_view_release(st->pipe, &stObj->sampler_view);
> -   }
> +
> +   pipe_resource_reference(&stObj->pt, NULL);
> +   st_texture_release_all_sampler_views(stObj);
> +   FREE(stObj->sampler_views);
>      _mesa_delete_texture_object(ctx, texObj);
>   }
>
> @@ -454,7 +452,7 @@ st_AllocTextureImageBuffer(struct gl_context *ctx,
>      /* The parent texture object does not have space for this image */
>
>      pipe_resource_reference(&stObj->pt, NULL);
> -   pipe_sampler_view_release(st->pipe, &stObj->sampler_view);
> +   st_texture_release_all_sampler_views(stObj);
>
>      if (!guess_and_alloc_texture(st, stObj, stImage)) {
>         /* Probably out of memory.
> @@ -1487,13 +1485,13 @@ st_finalize_texture(struct gl_context *ctx,
>
>         if (!st_obj) {
>            pipe_resource_reference(&stObj->pt, NULL);
> -         pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> +         st_texture_release_all_sampler_views(stObj);
>            return GL_TRUE;
>         }
>
>         if (st_obj->buffer != stObj->pt) {
>            pipe_resource_reference(&stObj->pt, st_obj->buffer);
> -         pipe_sampler_view_release(st->pipe, &stObj->sampler_view);
> +         st_texture_release_all_sampler_views(stObj);
>            stObj->width0 = stObj->pt->width0 / _mesa_get_format_bytes(tObj->_BufferObjectFormat);
>            stObj->height0 = 1;
>            stObj->depth0 = 1;
> @@ -1514,7 +1512,7 @@ st_finalize_texture(struct gl_context *ctx,
>          firstImage->pt != stObj->pt &&
>          (!stObj->pt || firstImage->pt->last_level >= stObj->pt->last_level)) {
>         pipe_resource_reference(&stObj->pt, firstImage->pt);
> -      pipe_sampler_view_release(st->pipe, &stObj->sampler_view);
> +      st_texture_release_all_sampler_views(stObj);
>      }
>
>      /* If this texture comes from a window system, there is nothing else to do. */
> @@ -1561,7 +1559,7 @@ st_finalize_texture(struct gl_context *ctx,
>             * gallium texture now.  We'll make a new one below.
>             */
>            pipe_resource_reference(&stObj->pt, NULL);
> -         pipe_sampler_view_release(st->pipe, &stObj->sampler_view);
> +         st_texture_release_all_sampler_views(stObj);
>            st->dirty.st |= ST_NEW_FRAMEBUFFER;
>         }
>      }
> diff --git a/src/mesa/state_tracker/st_gen_mipmap.c b/src/mesa/state_tracker/st_gen_mipmap.c
> index b615575..f1864fc 100644
> --- a/src/mesa/state_tracker/st_gen_mipmap.c
> +++ b/src/mesa/state_tracker/st_gen_mipmap.c
> @@ -182,7 +182,7 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
>
>         /* release the old tex (will likely be freed too) */
>         pipe_resource_reference(&oldTex, NULL);
> -      pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> +      st_texture_release_all_sampler_views(stObj);
>      }
>      else {
>         /* Make sure that the base texture image data is present in the
> diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c
> index a3b3453..27a0922 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -40,6 +40,7 @@
>   #include "util/u_format.h"
>   #include "util/u_rect.h"
>   #include "util/u_math.h"
> +#include "util/u_memory.h"
>
>
>   #define DBG if(0) printf
> @@ -412,10 +413,65 @@ st_create_color_map_texture(struct gl_context *ctx)
>      return pt;
>   }
>


Please add a comment explaining the function:

> +struct pipe_sampler_view **
> +st_texture_get_sampler_view(struct st_context *st,
> +                            struct st_texture_object *stObj)
> +{
> +   struct pipe_sampler_view **used = NULL, **free = NULL;
> +   GLuint i;
> +
> +   for (i = 0; i < stObj->num_sampler_views; ++i) {
> +      struct pipe_sampler_view **sv = &stObj->sampler_views[i];
> +      /* Is the array entry used ? */
> +      if (*sv) {
> +         /* Yes, check if it's the right one */
> +         if ((*sv)->context == st->pipe)
> +            return sv;
> +
> +         /* Wasn't the right one, but remember it as template */
> +         used = sv;
> +      } else {
> +         /* Found a free slot, remember that */
> +         free = sv;
> +      }
> +   }
> +
> +   /* Couldn't find a slot for our context, create a new one */
> +
> +   if (!free) {
> +      /* Haven't even found a free one, resize the array */
> +      stObj->sampler_views = REALLOC(stObj->sampler_views,
> +         stObj->num_sampler_views, stObj->num_sampler_views + 1);

Don't the old and new sizes need to be in bytes?


> +      free = &stObj->sampler_views[stObj->num_sampler_views++];
> +      *free = NULL;
> +   }
> +
> +   /* Add just any sampler view to be used as a template */
> +   if (used)
> +      pipe_sampler_view_reference(free, *used);
> +
> +   return free;
> +}
> +
>   void
>   st_texture_release_sampler_view(struct st_context *st,
>                                   struct st_texture_object *stObj)
>   {
> -   if (stObj->sampler_view && stObj->sampler_view->context == st->pipe)
> -      pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> +   GLuint i;
> +
> +   for (i = 0; i < stObj->num_sampler_views; ++i) {
> +      struct pipe_sampler_view **sv = &stObj->sampler_views[i];
> +
> +      if (*sv && (*sv)->context == st->pipe)
> +         pipe_sampler_view_reference(sv, NULL);

I think you could return early here.  There should never be more than 
one sampler view for any given context.


> +   }
> +}
> +
> +void
> +st_texture_release_all_sampler_views(struct st_texture_object *stObj)
> +{
> +   GLuint i;
> +
> +   for (i = 0; i < stObj->num_sampler_views; ++i)
> +      pipe_sampler_view_reference(&stObj->sampler_views[i], NULL);
>   }
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index b4a27a0..188fd52 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -82,10 +82,13 @@ struct st_texture_object
>       */
>      struct pipe_resource *pt;
>
> -   /* Default sampler view attached to this texture object. Created lazily
> -    * on first binding.
> +   /* Number of views in sampler_views array */
> +   GLuint num_sampler_views;
> +
> +   /* Sampler views attached to this texture object. Created lazily

"Array of sampler views (one per context) attached to this ..."


> +    * on first binding in context.
>       */
> -   struct pipe_sampler_view *sampler_view;
> +   struct pipe_sampler_view **sampler_views;
>
>      /* True if this texture comes from the window system. Such a texture
>       * cannot be reallocated and the format can only be changed with a sampler
> @@ -227,8 +230,15 @@ st_texture_image_copy(struct pipe_context *pipe,
>   extern struct pipe_resource *
>   st_create_color_map_texture(struct gl_context *ctx);
>
> +extern struct pipe_sampler_view **
> +st_texture_get_sampler_view(struct st_context *st,
> +                            struct st_texture_object *stObj);
> +
>   extern void
>   st_texture_release_sampler_view(struct st_context *st,
>                                   struct st_texture_object *stObj);
>
> +extern void
> +st_texture_release_all_sampler_views(struct st_texture_object *stObj);
> +
>   #endif
> diff --git a/src/mesa/state_tracker/st_vdpau.c b/src/mesa/state_tracker/st_vdpau.c
> index c5b4780..8c10cda 100644
> --- a/src/mesa/state_tracker/st_vdpau.c
> +++ b/src/mesa/state_tracker/st_vdpau.c
> @@ -63,7 +63,7 @@ st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>      struct st_texture_image *stImage = st_texture_image(texImage);
>
>      struct pipe_resource *res;
> -   struct pipe_sampler_view *sv, templ;
> +   struct pipe_sampler_view templ, **sampler_view;
>      mesa_format texFormat;
>
>      getProcAddr = ctx->vdpGetProcAddress;
> @@ -83,6 +83,7 @@ st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>         }
>
>      } else {
> +      struct pipe_sampler_view *sv;
>         VdpVideoSurfaceGallium *f;
>
>         struct pipe_video_buffer *buffer;
> @@ -138,7 +139,7 @@ st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>                                 texFormat);
>
>      pipe_resource_reference(&stObj->pt, res);
> -   pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> +   st_texture_release_all_sampler_views(stObj);
>      pipe_resource_reference(&stImage->pt, res);
>
>      u_sampler_view_default_template(&templ, res, res->format);
> @@ -148,7 +149,9 @@ st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>      templ.swizzle_g = GET_SWZ(stObj->base._Swizzle, 1);
>      templ.swizzle_b = GET_SWZ(stObj->base._Swizzle, 2);
>      templ.swizzle_a = GET_SWZ(stObj->base._Swizzle, 3);
> -   stObj->sampler_view = st->pipe->create_sampler_view(st->pipe, res, &templ);
> +
> +   sampler_view = st_texture_get_sampler_view(st, stObj);
> +   *sampler_view = st->pipe->create_sampler_view(st->pipe, res, &templ);
>
>      stObj->width0 = res->width0;
>      stObj->height0 = res->height0;
> @@ -169,7 +172,7 @@ st_vdpau_unmap_surface(struct gl_context *ctx, GLenum target, GLenum access,
>      struct st_texture_image *stImage = st_texture_image(texImage);
>
>      pipe_resource_reference(&stObj->pt, NULL);
> -   pipe_sampler_view_reference(&stObj->sampler_view, NULL);
> +   st_texture_release_all_sampler_views(stObj);
>      pipe_resource_reference(&stImage->pt, NULL);
>
>      _mesa_dirty_texobj(ctx, texObj);
>



More information about the mesa-dev mailing list