Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On Wed, Jun 20, 2012 at 10:11 AM, Brian Paul wrote: > On 06/20/2012 08:00 AM, Kristian Høgsberg wrote: >> >> On Tue, Jun 19, 2012 at 3:26 PM, Eric Anholt wrote: >>> >>> On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul wrote: On 06/18/2012 07:35 PM, Eric Anholt wrote: > > Fixes piglit ARB_uniform_buffer_object/getintegeri_v. > --- > src/mesa/main/get.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c > index 933bfe7..4798c02 100644 > --- a/src/mesa/main/get.c > +++ b/src/mesa/main/get.c > @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum > pname, int index, union value *v) > goto invalid_enum; > v->value_int = > ctx->TransformFeedback.CurrentObject->BufferNames[index]; > return TYPE_INT; > + > + case GL_UNIFORM_BUFFER_BINDING: > + if (index>= ctx->Const.MaxUniformBufferBindings) > + goto invalid_value; > + if (!ctx->Extensions.ARB_uniform_buffer_object) > + goto invalid_enum; I think it's a bit more natural to do the extension check before the index check. >>> >>> >>> Since all the other enums are handled in this order, too, I'd rather >>> that be an independent change. It does seem like a silly ordering, >>> though. >> >> >> You can use the "extra" mechanism to check the extension automatically >> before even getting into find_value_indexed. Add: >> >> EXTRA_EXT(ARB_uniform_buffer_object); >> >> and then add&extra_ARB_uniform_buffer_object instead of NO_EXTRA >> >> where the token is defined. > > > Actually, it looks like check_extra() isn't called for the indexed glGet > functions. find_value_indexed() isn't using any sort of table for its > implementation. There aren't too many indexed queries yet but maybe someone > should take a look at that. Hm, yea, I even wrote that code myself, apparently. Kristian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On 06/20/2012 08:00 AM, Kristian Høgsberg wrote: On Tue, Jun 19, 2012 at 3:26 PM, Eric Anholt wrote: On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul wrote: On 06/18/2012 07:35 PM, Eric Anholt wrote: Fixes piglit ARB_uniform_buffer_object/getintegeri_v. --- src/mesa/main/get.c | 24 1 file changed, 24 insertions(+) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 933bfe7..4798c02 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, int index, union value *v) goto invalid_enum; v->value_int = ctx->TransformFeedback.CurrentObject->BufferNames[index]; return TYPE_INT; + + case GL_UNIFORM_BUFFER_BINDING: + if (index>= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; I think it's a bit more natural to do the extension check before the index check. Since all the other enums are handled in this order, too, I'd rather that be an independent change. It does seem like a silly ordering, though. You can use the "extra" mechanism to check the extension automatically before even getting into find_value_indexed. Add: EXTRA_EXT(ARB_uniform_buffer_object); and then add&extra_ARB_uniform_buffer_object instead of NO_EXTRA where the token is defined. Actually, it looks like check_extra() isn't called for the indexed glGet functions. find_value_indexed() isn't using any sort of table for its implementation. There aren't too many indexed queries yet but maybe someone should take a look at that. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On Tue, Jun 19, 2012 at 3:26 PM, Eric Anholt wrote: > On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul wrote: >> On 06/18/2012 07:35 PM, Eric Anholt wrote: >> > Fixes piglit ARB_uniform_buffer_object/getintegeri_v. >> > --- >> > src/mesa/main/get.c | 24 >> > 1 file changed, 24 insertions(+) >> > >> > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c >> > index 933bfe7..4798c02 100644 >> > --- a/src/mesa/main/get.c >> > +++ b/src/mesa/main/get.c >> > @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, >> > int index, union value *v) >> > goto invalid_enum; >> > v->value_int = >> > ctx->TransformFeedback.CurrentObject->BufferNames[index]; >> > return TYPE_INT; >> > + >> > + case GL_UNIFORM_BUFFER_BINDING: >> > + if (index>= ctx->Const.MaxUniformBufferBindings) >> > + goto invalid_value; >> > + if (!ctx->Extensions.ARB_uniform_buffer_object) >> > + goto invalid_enum; >> >> I think it's a bit more natural to do the extension check before the >> index check. > > Since all the other enums are handled in this order, too, I'd rather > that be an independent change. It does seem like a silly ordering, > though. You can use the "extra" mechanism to check the extension automatically before even getting into find_value_indexed. Add: EXTRA_EXT(ARB_uniform_buffer_object); and then add &extra_ARB_uniform_buffer_object instead of NO_EXTRA where the token is defined. Kristian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On 06/19/2012 01:26 PM, Eric Anholt wrote: On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul wrote: On 06/18/2012 07:35 PM, Eric Anholt wrote: Fixes piglit ARB_uniform_buffer_object/getintegeri_v. --- src/mesa/main/get.c | 24 1 file changed, 24 insertions(+) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 933bfe7..4798c02 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, int index, union value *v) goto invalid_enum; v->value_int = ctx->TransformFeedback.CurrentObject->BufferNames[index]; return TYPE_INT; + + case GL_UNIFORM_BUFFER_BINDING: + if (index>= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; I think it's a bit more natural to do the extension check before the index check. Since all the other enums are handled in this order, too, I'd rather that be an independent change. It does seem like a silly ordering, though. Kenneth spotted the same thing. Reordering in a follow-on commit sounds fine. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On 06/18/2012 06:35 PM, Eric Anholt wrote: > Fixes piglit ARB_uniform_buffer_object/getintegeri_v. > --- > src/mesa/main/get.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c > index 933bfe7..4798c02 100644 > --- a/src/mesa/main/get.c > +++ b/src/mesa/main/get.c > @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, int > index, union value *v) >goto invalid_enum; >v->value_int = > ctx->TransformFeedback.CurrentObject->BufferNames[index]; >return TYPE_INT; > + > + case GL_UNIFORM_BUFFER_BINDING: > + if (index >= ctx->Const.MaxUniformBufferBindings) > + goto invalid_value; > + if (!ctx->Extensions.ARB_uniform_buffer_object) > + goto invalid_enum; > + v->value_int = ctx->UniformBufferBindings[index].BufferObject->Name; > + return TYPE_INT; > + > + case GL_UNIFORM_BUFFER_START: > + if (index >= ctx->Const.MaxUniformBufferBindings) > + goto invalid_value; > + if (!ctx->Extensions.ARB_uniform_buffer_object) > + goto invalid_enum; > + v->value_int = ctx->UniformBufferBindings[index].Offset; > + return TYPE_INT; > + > + case GL_UNIFORM_BUFFER_SIZE: > + if (index >= ctx->Const.MaxUniformBufferBindings) > + goto invalid_value; > + if (!ctx->Extensions.ARB_uniform_buffer_object) > + goto invalid_enum; > + v->value_int = ctx->UniformBufferBindings[index].Size; > + return TYPE_INT; > } > > invalid_enum: Total nitpick: it seems like you should reorder the two checks, i.e. if (!ctx->Extensions.ARB_uniform_buffer_object) goto invalid_enum; if (index out of bounds) goto invalid_value; because if I query these on an implementation that doesn't support ARB_uniform_buffer_object, I would expect to get GL_INVALID_ENUM ("WTF are you talking about?"), not a complaint that my index value is out of bounds. (Not to mention the fact that you can't glGet() the upper bound in the first place...) It doesn't really matter, though I bet there's some picky conformance suite out there that might care. (I haven't checked oglconform, but it just sounds like the sort of thing they'd check for. :)) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul wrote: > On 06/18/2012 07:35 PM, Eric Anholt wrote: > > Fixes piglit ARB_uniform_buffer_object/getintegeri_v. > > --- > > src/mesa/main/get.c | 24 > > 1 file changed, 24 insertions(+) > > > > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c > > index 933bfe7..4798c02 100644 > > --- a/src/mesa/main/get.c > > +++ b/src/mesa/main/get.c > > @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, > > int index, union value *v) > > goto invalid_enum; > > v->value_int = > > ctx->TransformFeedback.CurrentObject->BufferNames[index]; > > return TYPE_INT; > > + > > + case GL_UNIFORM_BUFFER_BINDING: > > + if (index>= ctx->Const.MaxUniformBufferBindings) > > +goto invalid_value; > > + if (!ctx->Extensions.ARB_uniform_buffer_object) > > +goto invalid_enum; > > I think it's a bit more natural to do the extension check before the > index check. Since all the other enums are handled in this order, too, I'd rather that be an independent change. It does seem like a silly ordering, though. pgpF8jfHSt3ZQ.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
On 06/18/2012 07:35 PM, Eric Anholt wrote: Fixes piglit ARB_uniform_buffer_object/getintegeri_v. --- src/mesa/main/get.c | 24 1 file changed, 24 insertions(+) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 933bfe7..4798c02 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, int index, union value *v) goto invalid_enum; v->value_int = ctx->TransformFeedback.CurrentObject->BufferNames[index]; return TYPE_INT; + + case GL_UNIFORM_BUFFER_BINDING: + if (index>= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; I think it's a bit more natural to do the extension check before the index check. + v->value_int = ctx->UniformBufferBindings[index].BufferObject->Name; + return TYPE_INT; + + case GL_UNIFORM_BUFFER_START: + if (index>= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; + v->value_int = ctx->UniformBufferBindings[index].Offset; + return TYPE_INT; + + case GL_UNIFORM_BUFFER_SIZE: + if (index>= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; + v->value_int = ctx->UniformBufferBindings[index].Size; + return TYPE_INT; } invalid_enum: ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.
Fixes piglit ARB_uniform_buffer_object/getintegeri_v. --- src/mesa/main/get.c | 24 1 file changed, 24 insertions(+) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 933bfe7..4798c02 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -2566,6 +2566,30 @@ find_value_indexed(const char *func, GLenum pname, int index, union value *v) goto invalid_enum; v->value_int = ctx->TransformFeedback.CurrentObject->BufferNames[index]; return TYPE_INT; + + case GL_UNIFORM_BUFFER_BINDING: + if (index >= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; + v->value_int = ctx->UniformBufferBindings[index].BufferObject->Name; + return TYPE_INT; + + case GL_UNIFORM_BUFFER_START: + if (index >= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; + v->value_int = ctx->UniformBufferBindings[index].Offset; + return TYPE_INT; + + case GL_UNIFORM_BUFFER_SIZE: + if (index >= ctx->Const.MaxUniformBufferBindings) +goto invalid_value; + if (!ctx->Extensions.ARB_uniform_buffer_object) +goto invalid_enum; + v->value_int = ctx->UniformBufferBindings[index].Size; + return TYPE_INT; } invalid_enum: -- 1.7.10 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev