On 10/06/2016 01:45 AM, Nicolai Hähnle wrote:
On 06.10.2016 02:42, Brian Paul wrote:
Before, st_get_texture_sampler_view_from_stobj() did a lot of work to
check if the texture parameters matched the sampler view (format,
swizzle, min/max lod, first/last layer, etc).  We did this every time
we validated the texture state.

Now, we use a ctx->Driver.TexParameter() callback and a couple other
checks to proactively release texture views when we know that
view-related parameters have changed.  Then, the validation step is
simplified:
- Search the texture's list of sampler views (just match the context).
- If found, we're done.
- Else, create a new sampler view.

There will never be old, out-of-date sampler views attached to texture
objects that we have to test.

Most apps create textures and set the texture parameters once.  This
make sampler view validation much cheaper for that case.

Note that the old texture/sampler comparison code has been converted
into a set of assertions to verify that the sampler view is in fact
consistent with the texture parameters.  This should help to spot any
potential regressions.

Reviewed-by: Edward O'Callaghan <funfunc...@folklore1984.net>
---
 src/mesa/state_tracker/st_atom_texture.c | 58
++++++++++++++++++++------------
 src/mesa/state_tracker/st_cb_texture.c   | 52
++++++++++++++++++++++++++++
 src/mesa/state_tracker/st_texture.c      | 16 ++++-----
 src/mesa/state_tracker/st_texture.h      |  5 +++
 4 files changed, 101 insertions(+), 30 deletions(-)

diff --git a/src/mesa/state_tracker/st_atom_texture.c
b/src/mesa/state_tracker/st_atom_texture.c
index bfa16dc..45f1f6b 100644
--- a/src/mesa/state_tracker/st_atom_texture.c
+++ b/src/mesa/state_tracker/st_atom_texture.c
@@ -370,7 +370,7 @@ st_create_texture_sampler_view_from_stobj(struct
st_context *st,
 static struct pipe_sampler_view *
 st_get_texture_sampler_view_from_stobj(struct st_context *st,
                                        struct st_texture_object *stObj,
-                       enum pipe_format format,
+                                       const struct gl_sampler_object
*samp,
                                        unsigned glsl_version)
 {
    struct pipe_sampler_view **sv;
@@ -381,34 +381,42 @@ st_get_texture_sampler_view_from_stobj(struct
st_context *st,

    sv = st_texture_get_sampler_view(st, stObj);

-   /* if sampler view has changed dereference it */
    if (*sv) {
-      if (check_sampler_swizzle(st, stObj, *sv, glsl_version) ||
-      (format != (*sv)->format) ||
-          gl_target_to_pipe(stObj->base.Target) != (*sv)->target ||
-          stObj->base.MinLevel + stObj->base.BaseLevel !=
(*sv)->u.tex.first_level ||
-          last_level(stObj) != (*sv)->u.tex.last_level ||
-          stObj->base.MinLayer != (*sv)->u.tex.first_layer ||
-          last_layer(stObj) != (*sv)->u.tex.last_layer) {
-     pipe_sampler_view_reference(sv, NULL);
+      /* Debug check: make sure that the sampler view's parameters are
+       * what they're supposed to be.
+       */
+      struct pipe_sampler_view *view = *sv;
+      assert(!check_sampler_swizzle(st, stObj, view, glsl_version));
+      assert(get_sampler_view_format(st, stObj, samp) == view->format);
+      assert(gl_target_to_pipe(stObj->base.Target) == view->target);
+      if (stObj->base.Target == GL_TEXTURE_BUFFER) {
+         unsigned base = stObj->base.BufferOffset;
+         unsigned size = MIN2(stObj->pt->width0 - base,
+                              (unsigned) stObj->base.BufferSize);
+         assert(view->u.buf.offset == base);
+         assert(view->u.buf.size == size);
+      }
+      else {

Huh, so is this else-style intentional, or a spill-over from another
code base? I wasn't aware of this style in st/mesa.

I've been doing it that way for about 25 years. The } else { style is relatively new to Mesa and I guess I haven't adopted that habit yet.

'git grep else' shows a mix of styles.


Also, the whole sampler lookup code gives me the race condition chills,
but it looks like it's always been that way, so... patches 4 & 5:

Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>

Patch 6 is Acked-by: Nicolai Hähnle <nicolai.haeh...@amd.com>

Thanks for reviewing.

-Brian


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to