[Mesa-dev] [PATCH] st/mesa: store state that affects sampler views per context

Marek Olšák maraeo at gmail.com
Tue Oct 10 22:01:45 UTC 2017


Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Tue, Oct 10, 2017 at 11:40 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> This fixes sequences like:
>
> 1. Context 1 samples from texture with sRGB decode enabled
> 2. Context 2 samples from texture with sRGB decode disabled
> 3. Context 1 samples from texture with sRGB decode disabled
>
> Previously, step 3 would see the prev_sRGBDecode value from context 2
> and would incorrectly use the old sampler view with sRGB decode enabled.
> ---
> This patch is originally from my texture locking series, which I'm delaying
> to take a closer look at performance.
>
> It is independent from the locking stuff, so I rebased it on master as I'd
> like to push it earlier and would appreciate a separate review.
>
> Cheers,
> Nicolai
> ---
>  src/mesa/state_tracker/st_atom_sampler.c |  4 +--
>  src/mesa/state_tracker/st_atom_texture.c | 12 --------
>  src/mesa/state_tracker/st_sampler_view.c | 49 ++++++++++++++++++--------------
>  src/mesa/state_tracker/st_texture.h      | 20 +++++++++----
>  4 files changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_sampler.c b/src/mesa/state_tracker/st_atom_sampler.c
> index d9e8de3c9e0..ff3f49fa4e4 100644
> --- a/src/mesa/state_tracker/st_atom_sampler.c
> +++ b/src/mesa/state_tracker/st_atom_sampler.c
> @@ -163,22 +163,22 @@ st_convert_sampler(const struct st_context *st,
>        GLenum texBaseFormat = _mesa_base_tex_image(texobj)->_BaseFormat;
>
>        if (st->apply_texture_swizzle_to_border_color) {
>           const struct st_texture_object *stobj = st_texture_object_const(texobj);
>           const struct pipe_sampler_view *sv = NULL;
>
>           /* 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 (unsigned i = 0; i < stobj->num_sampler_views; ++i) {
> -            if (stobj->sampler_views[i]) {
> -               sv = stobj->sampler_views[i];
> +            if (stobj->sampler_views[i].view) {
> +               sv = stobj->sampler_views[i].view;
>                 break;
>              }
>           }
>
>           if (sv) {
>              union pipe_color_union tmp;
>              const unsigned char swz[4] =
>              {
>                 sv->swizzle_r,
>                 sv->swizzle_g,
> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
> index 81bf62908f1..90828bb4cf9 100644
> --- a/src/mesa/state_tracker/st_atom_texture.c
> +++ b/src/mesa/state_tracker/st_atom_texture.c
> @@ -77,32 +77,20 @@ st_update_single_texture(struct st_context *st,
>        return;
>     }
>
>     if (!st_finalize_texture(ctx, st->pipe, texObj, 0) ||
>         !stObj->pt) {
>        /* out of mem */
>        *sampler_view = NULL;
>        return;
>     }
>
> -   /* Check a few pieces of state outside the texture object to see if we
> -    * need to force revalidation.
> -    */
> -   if (stObj->prev_glsl130_or_later != glsl130_or_later ||
> -       stObj->prev_sRGBDecode != samp->sRGBDecode) {
> -
> -      st_texture_release_all_sampler_views(st, stObj);
> -
> -      stObj->prev_glsl130_or_later = glsl130_or_later;
> -      stObj->prev_sRGBDecode = samp->sRGBDecode;
> -   }
> -
>     if (texObj->TargetIndex == TEXTURE_EXTERNAL_INDEX &&
>         stObj->pt->screen->resource_changed)
>           stObj->pt->screen->resource_changed(stObj->pt->screen, stObj->pt);
>
>     *sampler_view =
>        st_get_texture_sampler_view_from_stobj(st, stObj, samp,
>                                               glsl130_or_later);
>  }
>
>
> diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c
> index 014b4d26784..99c4f74ae08 100644
> --- a/src/mesa/state_tracker/st_sampler_view.c
> +++ b/src/mesa/state_tracker/st_sampler_view.c
> @@ -40,70 +40,70 @@
>  #include "st_format.h"
>  #include "st_cb_bufferobjects.h"
>  #include "st_cb_texture.h"
>
>
>  /**
>   * Try to find a matching sampler view for the given context.
>   * If none is found an empty slot is initialized with a
>   * template and returned instead.
>   */
> -static struct pipe_sampler_view **
> +static struct st_sampler_view *
>  st_texture_get_sampler_view(struct st_context *st,
>                              struct st_texture_object *stObj)
>  {
> -   struct pipe_sampler_view **free = NULL;
> +   struct st_sampler_view *free = NULL;
>     GLuint i;
>
>     for (i = 0; i < stObj->num_sampler_views; ++i) {
> -      struct pipe_sampler_view **sv = &stObj->sampler_views[i];
> +      struct st_sampler_view *sv = &stObj->sampler_views[i];
>        /* Is the array entry used ? */
> -      if (*sv) {
> +      if (sv->view) {
>           /* check if the context matches */
> -         if ((*sv)->context == st->pipe) {
> +         if (sv->view->context == st->pipe) {
>              return 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 */
>        unsigned new_size = (stObj->num_sampler_views + 1) *
> -         sizeof(struct pipe_sampler_view *);
> +         sizeof(struct st_sampler_view);
>        stObj->sampler_views = realloc(stObj->sampler_views, new_size);
>        free = &stObj->sampler_views[stObj->num_sampler_views++];
> -      *free = NULL;
> +      free->view = NULL;
>     }
>
> -   assert(*free == NULL);
> +   assert(free->view == NULL);
>
>     return free;
>  }
>
>
>  /**
>   * For the given texture object, release any sampler views which belong
>   * to the calling context.
>   */
>  void
>  st_texture_release_sampler_view(struct st_context *st,
>                                  struct st_texture_object *stObj)
>  {
>     GLuint i;
>
>     for (i = 0; i < stObj->num_sampler_views; ++i) {
> -      struct pipe_sampler_view **sv = &stObj->sampler_views[i];
> +      struct pipe_sampler_view **sv = &stObj->sampler_views[i].view;
>
>        if (*sv && (*sv)->context == st->pipe) {
>           pipe_sampler_view_reference(sv, NULL);
>           break;
>        }
>     }
>  }
>
>
>  /**
> @@ -111,21 +111,21 @@ st_texture_release_sampler_view(struct st_context *st,
>   * of the context.
>   */
>  void
>  st_texture_release_all_sampler_views(struct st_context *st,
>                                       struct st_texture_object *stObj)
>  {
>     GLuint i;
>
>     /* XXX This should use sampler_views[i]->pipe, not st->pipe */
>     for (i = 0; i < stObj->num_sampler_views; ++i)
> -      pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i]);
> +      pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i].view);
>  }
>
>
>  void
>  st_texture_free_sampler_views(struct st_texture_object *stObj)
>  {
>     free(stObj->sampler_views);
>     stObj->sampler_views = NULL;
>     stObj->num_sampler_views = 0;
>  }
> @@ -403,70 +403,77 @@ st_create_texture_sampler_view_from_stobj(struct st_context *st,
>     return st->pipe->create_sampler_view(st->pipe, stObj->pt, &templ);
>  }
>
>
>  struct pipe_sampler_view *
>  st_get_texture_sampler_view_from_stobj(struct st_context *st,
>                                         struct st_texture_object *stObj,
>                                         const struct gl_sampler_object *samp,
>                                         bool glsl130_or_later)
>  {
> -   struct pipe_sampler_view **sv;
> +   struct st_sampler_view *sv;
> +   struct pipe_sampler_view *view;
>
>     sv = st_texture_get_sampler_view(st, stObj);
> +   view = sv->view;
>
> -   if (*sv) {
> +   if (view &&
> +       sv->glsl130_or_later == glsl130_or_later &&
> +       sv->sRGBDecode == samp->sRGBDecode) {
>        /* Debug check: make sure that the sampler view's parameters are
>         * what they're supposed to be.
>         */
> -      MAYBE_UNUSED struct pipe_sampler_view *view = *sv;
> +      MAYBE_UNUSED struct pipe_sampler_view *view = sv->view;
>        assert(stObj->pt == view->texture);
>        assert(!check_sampler_swizzle(st, stObj, view, glsl130_or_later));
>        assert(get_sampler_view_format(st, stObj, samp) == view->format);
>        assert(gl_target_to_pipe(stObj->base.Target) == view->target);
>        assert(stObj->level_override ||
>               stObj->base.MinLevel + stObj->base.BaseLevel == view->u.tex.first_level);
>        assert(stObj->level_override || last_level(stObj) == view->u.tex.last_level);
>        assert(stObj->layer_override || stObj->base.MinLayer == view->u.tex.first_layer);
>        assert(stObj->layer_override || last_layer(stObj) == view->u.tex.last_layer);
>        assert(!stObj->layer_override ||
>               (stObj->layer_override == view->u.tex.first_layer &&
>                stObj->layer_override == view->u.tex.last_layer));
>     }
>     else {
>        /* create new sampler view */
>        enum pipe_format format = get_sampler_view_format(st, stObj, samp);
>
> -      *sv = st_create_texture_sampler_view_from_stobj(st, stObj,
> -                                                      format, glsl130_or_later);
> +      sv->glsl130_or_later = glsl130_or_later;
> +      sv->sRGBDecode = samp->sRGBDecode;
>
> +      pipe_sampler_view_release(st->pipe, &sv->view);
> +      view = sv->view =
> +         st_create_texture_sampler_view_from_stobj(st, stObj, format, glsl130_or_later);
>     }
>
> -   return *sv;
> +   return view;
>  }
>
>
>  struct pipe_sampler_view *
>  st_get_buffer_sampler_view_from_stobj(struct st_context *st,
>                                        struct st_texture_object *stObj)
>  {
> -   struct pipe_sampler_view **sv;
> +   struct st_sampler_view *sv;
>     struct st_buffer_object *stBuf =
>        st_buffer_object(stObj->base.BufferObject);
>
>     if (!stBuf || !stBuf->buffer)
>        return NULL;
>
>     sv = st_texture_get_sampler_view(st, stObj);
>
>     struct pipe_resource *buf = stBuf->buffer;
> -   struct pipe_sampler_view *view = *sv;
> +   struct pipe_sampler_view *view = sv->view;
>
>     if (view && view->texture == buf) {
>        /* Debug check: make sure that the sampler view's parameters are
>         * what they're supposed to be.
>         */
>        assert(st_mesa_format_to_pipe_format(st, stObj->base._BufferObjectFormat)
>               == view->format);
>        assert(view->target == PIPE_BUFFER);
>        unsigned base = stObj->base.BufferOffset;
>        MAYBE_UNUSED unsigned size = MIN2(buf->width0 - base,
> @@ -492,15 +499,15 @@ st_get_buffer_sampler_view_from_stobj(struct st_context *st,
>        templ.format =
>           st_mesa_format_to_pipe_format(st, stObj->base._BufferObjectFormat);
>        templ.target = PIPE_BUFFER;
>        templ.swizzle_r = PIPE_SWIZZLE_X;
>        templ.swizzle_g = PIPE_SWIZZLE_Y;
>        templ.swizzle_b = PIPE_SWIZZLE_Z;
>        templ.swizzle_a = PIPE_SWIZZLE_W;
>        templ.u.buf.offset = base;
>        templ.u.buf.size = size;
>
> -      pipe_sampler_view_reference(sv, NULL);
> -      *sv = st->pipe->create_sampler_view(st->pipe, buf, &templ);
> +      pipe_sampler_view_release(st->pipe, &sv->view);
> +      view = sv->view = st->pipe->create_sampler_view(st->pipe, buf, &templ);
>     }
> -   return *sv;
> +   return view;
>  }
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index ea459bf6e24..4f41aac53cf 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -42,20 +42,33 @@ struct st_texture_image_transfer {
>     struct pipe_transfer *transfer;
>
>     /* For ETC fallback. */
>     GLubyte *temp_data; /**< Temporary ETC texture storage. */
>     unsigned temp_stride; /**< Stride of the ETC texture storage. */
>     GLubyte *map; /**< Saved map pointer of the uncompressed transfer. */
>  };
>
>
>  /**
> + * Container for one context's validated sampler view.
> + */
> +struct st_sampler_view {
> +   struct pipe_sampler_view *view;
> +
> +   /** The glsl version of the shader seen during validation */
> +   bool glsl130_or_later;
> +   /** The value of the sampler's sRGBDecode state during validation */
> +   GLenum sRGBDecode;
> +};
> +
> +
> +/**
>   * Subclass of gl_texure_image.
>   */
>  struct st_texture_image
>  {
>     struct gl_texture_image base;
>
>     /* If stImage->pt != NULL, image data is stored here.
>      * Else there is no image data.
>      */
>     struct pipe_resource *pt;
> @@ -91,21 +104,21 @@ struct st_texture_object
>      * textures will be copied to this texture and the old storage freed.
>      */
>     struct pipe_resource *pt;
>
>     /* Number of views in sampler_views array */
>     GLuint num_sampler_views;
>
>     /* Array of sampler views (one per context) attached to this texture
>      * object. Created lazily on first binding in context.
>      */
> -   struct pipe_sampler_view **sampler_views;
> +   struct st_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
>      * view or a surface.
>      */
>     GLboolean surface_based;
>
>     /* If surface_based is true, this format should be used for all sampler
>      * views and surfaces instead of pt->format.
>      */
> @@ -122,25 +135,20 @@ struct st_texture_object
>     /* When non-zero, samplers should use this layer instead of the one
>      * specified by the GL state.
>      *
>      * This is used for EGL images and VDPAU interop, where imported
>      * pipe_resources may be cube, 3D, or array textures (containing layers
>      * with different fields in the case of VDPAU) even though the GL state
>      * describes one non-array texture per field.
>      */
>     uint layer_override;
>
> -   /** The glsl version of the shader seen during the previous validation */
> -   bool prev_glsl130_or_later;
> -   /** The value of the sampler's sRGBDecode state at the previous validation */
> -   GLenum prev_sRGBDecode;
> -
>      /**
>       * Set when the texture images of this texture object might not all be in
>       * the pipe_resource *pt above.
>       */
>      bool needs_validation;
>  };
>
>
>  static inline struct st_texture_image *
>  st_texture_image(struct gl_texture_image *img)
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list