Reviewed-by: Marek Olšák <[email protected]> Marek
On Tue, Oct 10, 2017 at 11:40 PM, Nicolai Hähnle <[email protected]> wrote: > From: Nicolai Hähnle <[email protected]> > > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
