On 03/06/2013 11:46 AM, Alan Hourihane wrote:
On 03/06/13 18:36, Brian Paul wrote:
On 03/06/2013 11:23 AM, Alan Hourihane wrote:
If the sampler object has been deleted on another context, an
alternative context may reference the old sampler. So ensure the
sampler
object still exists.
Alan, is this specified somewhere in a spec? I can't find a
description of this behaviour and we don't do this for texture
objects or buffer objects, etc.
I can't see it specifically mentioned, apart from the note that when
deleting the sampler object it should be unbound from the texture
unit, and I did consider the case of buffer & texture objects whether
to do this there too.
But getting the GL_SAMPLER_BINDING id when switching contexts and
attempting to re-bind with glBindSampler() gives a GL error, which
seems wrong to me.
I checked with the NVIDIA driver and no GL error is generated.
OK, sounds good.
BTW, just a few minor comments on your patch:
The subject of the patch should start with "mesa: fix glGetInteger..."
And, candidate for the stable branches?
> If the sampler object has been deleted on another context, an
> alternative context may reference the old sampler. So ensure the
sampler
> object still exists.
>
> Signed-off-by: Alan Hourihane <al...@vmware.com>
> ---
> src/mesa/main/get.c | 12 +++++++++++-
> src/mesa/main/samplerobj.c | 2 +-
> src/mesa/main/samplerobj.h | 2 ++
> 3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index 2399f9c..c627dd6 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -34,6 +34,7 @@
> #include "state.h"
> #include "texcompress.h"
> #include "framebuffer.h"
> +#include "samplerobj.h"
>
> /* This is a table driven implemetation of the glGet*v() functions.
> * The basic idea is that most getters just look up an int somewhere
> @@ -827,7 +828,16 @@ find_custom_value(struct gl_context *ctx,
const struct valu
> e_desc *d, union valu
> {
> struct gl_sampler_object *samp =
> ctx->Texture.Unit[ctx->Texture.CurrentUnit].Sampler;
> - v->value_int = samp ? samp->Name : 0;
> +
> + /*
> + * The sampler object may have been deleted on another context,
> + * so we try to lookup the sampler object before returning
it's Name.
"its"
> + */
> + if (samp && _mesa_lookup_samplerobj(ctx, samp->Name)) {
> + v->value_int = samp->Name;
> + } else {
> + v->value_int = 0;
> + }
Looks like a mix of spaces and tabs for indentation. Can you just use
spaces?
> }
> break;
> /* GL_ARB_uniform_buffer_object */
> diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c
> index 4664cc3..5cff329 100644
> --- a/src/mesa/main/samplerobj.c
> +++ b/src/mesa/main/samplerobj.c
> @@ -40,7 +40,7 @@
> #include "main/samplerobj.h"
>
>
> -static struct gl_sampler_object *
> +struct gl_sampler_object *
> _mesa_lookup_samplerobj(struct gl_context *ctx, GLuint name)
> {
> if (name == 0)
> diff --git a/src/mesa/main/samplerobj.h b/src/mesa/main/samplerobj.h
> index 3114257..69e3899 100644
> --- a/src/mesa/main/samplerobj.h
> +++ b/src/mesa/main/samplerobj.h
> @@ -62,6 +62,8 @@ _mesa_reference_sampler_object(struct gl_context
*ctx,
> _mesa_reference_sampler_object_(ctx, ptr, samp);
> }
>
> +extern struct gl_sampler_object *
> +_mesa_lookup_samplerobj(struct gl_context *ctx, GLuint name);
>
> extern struct gl_sampler_object *
> _mesa_new_sampler_object(struct gl_context *ctx, GLuint name);
Reviewed-by: Brian Paul <bri...@vmware.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
