Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.

2012-06-20 Thread Kenneth Graunke
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.

2012-06-20 Thread Brian Paul

On 06/19/2012 01:26 PM, Eric Anholt wrote:

On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paulbri...@vmware.com  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.

2012-06-20 Thread Kristian Høgsberg
On Tue, Jun 19, 2012 at 3:26 PM, Eric Anholt e...@anholt.net wrote:
 On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul bri...@vmware.com 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.

2012-06-20 Thread Brian Paul

On 06/20/2012 08:00 AM, Kristian Høgsberg wrote:

On Tue, Jun 19, 2012 at 3:26 PM, Eric Anholte...@anholt.net  wrote:

On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paulbri...@vmware.com  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 addextra_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.

2012-06-20 Thread Kristian Høgsberg
On Wed, Jun 20, 2012 at 10:11 AM, Brian Paul bri...@vmware.com wrote:
 On 06/20/2012 08:00 AM, Kristian Høgsberg wrote:

 On Tue, Jun 19, 2012 at 3:26 PM, Eric Anholte...@anholt.net  wrote:

 On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paulbri...@vmware.com  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 addextra_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.

2012-06-19 Thread Brian Paul

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


Re: [Mesa-dev] [PATCH 8/9] mesa: Add support for glGetIntegeri_v from GL_ARB_uniform_buffer_object.

2012-06-19 Thread Eric Anholt
On Tue, 19 Jun 2012 08:10:25 -0600, Brian Paul bri...@vmware.com 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