Re: [Mesa-dev] [PATCH 1/1] main: validate the type correctly against the OpenGL ES spec.

2012-09-13 Thread Oliver McFadden 
On Wed, Sep 12, 2012 at 11:26:55AM +0300, Oliver McFadden wrote:
 On Tue, Sep 11, 2012 at 11:15:18AM -0600, Brian Paul wrote:
  On 09/11/2012 10:21 AM, Oliver McFadden wrote:
   On Tue, Sep 11, 2012 at 07:54:36AM -0600, Brian Paul wrote:
   On 09/11/2012 06:09 AM, Oliver McFadden wrote:
   Previously Mesa would validate UNSIGNED_INT successfully in violation of
   the OpenGL(R) ES 1.0 and 2.0 Specification.
  
   http://www.khronos.org/registry/gles/specs/1.1/es_full_spec_1.1.12.pdf
   http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
   http://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.0.pdf
  
   Signed-off-by: Oliver McFaddenoliver.mcfad...@linux.intel.com
   ---
  src/mesa/main/api_validate.c |   22 ++
  1 files changed, 22 insertions(+), 0 deletions(-)
  
   diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
   index d0d2ca4..698394c 100644
   --- a/src/mesa/main/api_validate.c
   +++ b/src/mesa/main/api_validate.c
   @@ -296,10 +296,32 @@ _mesa_valid_prim_mode(struct gl_context *ctx, 
   GLenum mode, const char *name)
  static bool
  valid_elements_type(struct gl_context *ctx, GLenum type, const char 
   *name)
  {
   +  /*
   +   * Page 22 of the OpenGL(R) ES Version 1.0 Specification (PDF)
   +   * and
   +   * Page 21 of the OpenGL(R) ES Version 2.0 Specification (PDF) says:
   +   *
   +   * `type' must be one of UNSIGNED_BYTE or UNSIGNED_SHORT, indicating 
   that the
   +   * values in indices are indices of GL type ubyte or ushort, 
   respectively.
   +   *
   +   * Page 29 of the OpenGL(R) ES Version 3.0 Specification (PDF) says:
   +   *
   +   * `type' must be one of UNSIGNED_BYTE, UNSIGNED_SHORT, or 
   UNSIGNED_INT,
   +   * indicating that the index values are of GL type ubyte, ushort, or 
   uint
   +   * respectively.
   +   *
   +   */
 switch (type) {
 case GL_UNSIGNED_BYTE:
 case GL_UNSIGNED_SHORT:
   +  return true;
 case GL_UNSIGNED_INT:
   +  if (ctx-API == API_OPENGLES ||
   +  (ctx-API == API_OPENGLES2   ctx-Version   30)) {
   + _mesa_error(ctx, GL_INVALID_ENUM, %s(type = %s), name,
   + _mesa_lookup_enum_by_nr(type));
   + return false;
   +  }
return true;
  
 default:
  
   I suspect Ian's covered this in his ES overhaul work, but it looks
   good to me.
  
   You're giving the OK to push to master? I don't want to conflict with
   existing work...
  
  Ian hasn't posted all his ES-cleanup patches yet (though maybe they're 
  in a public tree of his) so I don't know if there's a conflict.
  
  If you can hold off for a bit, maybe Ian can review and comment.
 
 No problem. Adding him to CC.

Actually even if this would conflict with Ian's changes it's not exactly
a big deal to revert...  Brian, I think I'll push it with your
Reviewed-by to master later today.

I would rather get patches out from my local tree quickly, and this is
fixing a rather serious bug.  I actually hit this in real life because I
was using UNSIGNED_INT indices, which of course failed on another
non-Mesa OpenGL driver.

 
 -- 
 Oliver McFadden.
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Oliver McFadden.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/1] main: validate the type correctly against the OpenGL ES spec.

2012-09-13 Thread Oliver McFadden 
On Thu, Sep 13, 2012 at 12:18:09PM +0300, Oliver McFadden wrote:
 On Wed, Sep 12, 2012 at 11:26:55AM +0300, Oliver McFadden wrote:
  On Tue, Sep 11, 2012 at 11:15:18AM -0600, Brian Paul wrote:
   On 09/11/2012 10:21 AM, Oliver McFadden wrote:
On Tue, Sep 11, 2012 at 07:54:36AM -0600, Brian Paul wrote:
On 09/11/2012 06:09 AM, Oliver McFadden wrote:
Previously Mesa would validate UNSIGNED_INT successfully in violation 
of
the OpenGL(R) ES 1.0 and 2.0 Specification.
   
http://www.khronos.org/registry/gles/specs/1.1/es_full_spec_1.1.12.pdf
http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
http://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.0.pdf
   
Signed-off-by: Oliver McFaddenoliver.mcfad...@linux.intel.com
---
   src/mesa/main/api_validate.c |   22 ++
   1 files changed, 22 insertions(+), 0 deletions(-)
   
diff --git a/src/mesa/main/api_validate.c 
b/src/mesa/main/api_validate.c
index d0d2ca4..698394c 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -296,10 +296,32 @@ _mesa_valid_prim_mode(struct gl_context *ctx, 
GLenum mode, const char *name)
   static bool
   valid_elements_type(struct gl_context *ctx, GLenum type, const 
char *name)
   {
+  /*
+   * Page 22 of the OpenGL(R) ES Version 1.0 Specification (PDF)
+   * and
+   * Page 21 of the OpenGL(R) ES Version 2.0 Specification (PDF) 
says:
+   *
+   * `type' must be one of UNSIGNED_BYTE or UNSIGNED_SHORT, 
indicating that the
+   * values in indices are indices of GL type ubyte or ushort, 
respectively.
+   *
+   * Page 29 of the OpenGL(R) ES Version 3.0 Specification (PDF) 
says:
+   *
+   * `type' must be one of UNSIGNED_BYTE, UNSIGNED_SHORT, or 
UNSIGNED_INT,
+   * indicating that the index values are of GL type ubyte, ushort, 
or uint
+   * respectively.
+   *
+   */
  switch (type) {
  case GL_UNSIGNED_BYTE:
  case GL_UNSIGNED_SHORT:
+  return true;
  case GL_UNSIGNED_INT:
+  if (ctx-API == API_OPENGLES ||
+  (ctx-API == API_OPENGLES2   ctx-Version   30)) {
+ _mesa_error(ctx, GL_INVALID_ENUM, %s(type = %s), name,
+ _mesa_lookup_enum_by_nr(type));
+ return false;
+  }
 return true;
   
  default:
   
I suspect Ian's covered this in his ES overhaul work, but it looks
good to me.
   
You're giving the OK to push to master? I don't want to conflict with
existing work...
   
   Ian hasn't posted all his ES-cleanup patches yet (though maybe they're 
   in a public tree of his) so I don't know if there's a conflict.
   
   If you can hold off for a bit, maybe Ian can review and comment.
  
  No problem. Adding him to CC.
 
 Actually even if this would conflict with Ian's changes it's not exactly
 a big deal to revert...  Brian, I think I'll push it with your
 Reviewed-by to master later today.
 
 I would rather get patches out from my local tree quickly, and this is
 fixing a rather serious bug.  I actually hit this in real life because I
 was using UNSIGNED_INT indices, which of course failed on another
 non-Mesa OpenGL driver.

Okay, I just spoke with Ian in person and it turns out I'm an idiot.
GL_OES_element_index_uint is unconditionally supported in Mesa therefore
we have no need to check for the OpenGL spec restrictions.

 
  
  -- 
  Oliver McFadden.
  ___
  mesa-dev mailing list
  mesa-dev@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/mesa-dev
 
 -- 
 Oliver McFadden.

-- 
Oliver McFadden.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/1] main: validate the type correctly against the OpenGL ES spec.

2012-09-12 Thread Oliver McFadden 
On Tue, Sep 11, 2012 at 11:15:18AM -0600, Brian Paul wrote:
 On 09/11/2012 10:21 AM, Oliver McFadden wrote:
  On Tue, Sep 11, 2012 at 07:54:36AM -0600, Brian Paul wrote:
  On 09/11/2012 06:09 AM, Oliver McFadden wrote:
  Previously Mesa would validate UNSIGNED_INT successfully in violation of
  the OpenGL(R) ES 1.0 and 2.0 Specification.
 
  http://www.khronos.org/registry/gles/specs/1.1/es_full_spec_1.1.12.pdf
  http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
  http://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.0.pdf
 
  Signed-off-by: Oliver McFaddenoliver.mcfad...@linux.intel.com
  ---
 src/mesa/main/api_validate.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)
 
  diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
  index d0d2ca4..698394c 100644
  --- a/src/mesa/main/api_validate.c
  +++ b/src/mesa/main/api_validate.c
  @@ -296,10 +296,32 @@ _mesa_valid_prim_mode(struct gl_context *ctx, 
  GLenum mode, const char *name)
 static bool
 valid_elements_type(struct gl_context *ctx, GLenum type, const char 
  *name)
 {
  +  /*
  +   * Page 22 of the OpenGL(R) ES Version 1.0 Specification (PDF)
  +   * and
  +   * Page 21 of the OpenGL(R) ES Version 2.0 Specification (PDF) says:
  +   *
  +   * `type' must be one of UNSIGNED_BYTE or UNSIGNED_SHORT, indicating 
  that the
  +   * values in indices are indices of GL type ubyte or ushort, 
  respectively.
  +   *
  +   * Page 29 of the OpenGL(R) ES Version 3.0 Specification (PDF) says:
  +   *
  +   * `type' must be one of UNSIGNED_BYTE, UNSIGNED_SHORT, or 
  UNSIGNED_INT,
  +   * indicating that the index values are of GL type ubyte, ushort, or 
  uint
  +   * respectively.
  +   *
  +   */
switch (type) {
case GL_UNSIGNED_BYTE:
case GL_UNSIGNED_SHORT:
  +  return true;
case GL_UNSIGNED_INT:
  +  if (ctx-API == API_OPENGLES ||
  +  (ctx-API == API_OPENGLES2   ctx-Version   30)) {
  + _mesa_error(ctx, GL_INVALID_ENUM, %s(type = %s), name,
  + _mesa_lookup_enum_by_nr(type));
  + return false;
  +  }
   return true;
 
default:
 
  I suspect Ian's covered this in his ES overhaul work, but it looks
  good to me.
 
  You're giving the OK to push to master? I don't want to conflict with
  existing work...
 
 Ian hasn't posted all his ES-cleanup patches yet (though maybe they're 
 in a public tree of his) so I don't know if there's a conflict.
 
 If you can hold off for a bit, maybe Ian can review and comment.

No problem. Adding him to CC.

-- 
Oliver McFadden.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/1] main: validate the type correctly against the OpenGL ES spec.

2012-09-11 Thread Oliver McFadden 
Previously Mesa would validate UNSIGNED_INT successfully in violation of
the OpenGL(R) ES 1.0 and 2.0 Specification.

http://www.khronos.org/registry/gles/specs/1.1/es_full_spec_1.1.12.pdf
http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
http://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.0.pdf

Signed-off-by: Oliver McFadden oliver.mcfad...@linux.intel.com
---
 src/mesa/main/api_validate.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index d0d2ca4..698394c 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -296,10 +296,32 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum 
mode, const char *name)
 static bool
 valid_elements_type(struct gl_context *ctx, GLenum type, const char *name)
 {
+  /*
+   * Page 22 of the OpenGL(R) ES Version 1.0 Specification (PDF)
+   * and
+   * Page 21 of the OpenGL(R) ES Version 2.0 Specification (PDF) says:
+   *
+   * `type' must be one of UNSIGNED_BYTE or UNSIGNED_SHORT, indicating that the
+   * values in indices are indices of GL type ubyte or ushort, respectively.
+   *
+   * Page 29 of the OpenGL(R) ES Version 3.0 Specification (PDF) says:
+   *
+   * `type' must be one of UNSIGNED_BYTE, UNSIGNED_SHORT, or UNSIGNED_INT,
+   * indicating that the index values are of GL type ubyte, ushort, or uint
+   * respectively.
+   *
+   */
switch (type) {
case GL_UNSIGNED_BYTE:
case GL_UNSIGNED_SHORT:
+  return true;
case GL_UNSIGNED_INT:
+  if (ctx-API == API_OPENGLES ||
+  (ctx-API == API_OPENGLES2  ctx-Version  30)) {
+ _mesa_error(ctx, GL_INVALID_ENUM, %s(type = %s), name,
+ _mesa_lookup_enum_by_nr(type));
+ return false;
+  }
   return true;
 
default:
-- 
1.7.8.6

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/1] main: validate the type correctly against the OpenGL ES spec.

2012-09-11 Thread Brian Paul 

On 09/11/2012 06:09 AM, Oliver McFadden wrote:

Previously Mesa would validate UNSIGNED_INT successfully in violation of
the OpenGL(R) ES 1.0 and 2.0 Specification.

http://www.khronos.org/registry/gles/specs/1.1/es_full_spec_1.1.12.pdf
http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
http://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.0.pdf

Signed-off-by: Oliver McFaddenoliver.mcfad...@linux.intel.com
---
  src/mesa/main/api_validate.c |   22 ++
  1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index d0d2ca4..698394c 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -296,10 +296,32 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum 
mode, const char *name)
  static bool
  valid_elements_type(struct gl_context *ctx, GLenum type, const char *name)
  {
+  /*
+   * Page 22 of the OpenGL(R) ES Version 1.0 Specification (PDF)
+   * and
+   * Page 21 of the OpenGL(R) ES Version 2.0 Specification (PDF) says:
+   *
+   * `type' must be one of UNSIGNED_BYTE or UNSIGNED_SHORT, indicating that the
+   * values in indices are indices of GL type ubyte or ushort, respectively.
+   *
+   * Page 29 of the OpenGL(R) ES Version 3.0 Specification (PDF) says:
+   *
+   * `type' must be one of UNSIGNED_BYTE, UNSIGNED_SHORT, or UNSIGNED_INT,
+   * indicating that the index values are of GL type ubyte, ushort, or uint
+   * respectively.
+   *
+   */
 switch (type) {
 case GL_UNSIGNED_BYTE:
 case GL_UNSIGNED_SHORT:
+  return true;
 case GL_UNSIGNED_INT:
+  if (ctx-API == API_OPENGLES ||
+  (ctx-API == API_OPENGLES2  ctx-Version  30)) {
+ _mesa_error(ctx, GL_INVALID_ENUM, %s(type = %s), name,
+ _mesa_lookup_enum_by_nr(type));
+ return false;
+  }
return true;

 default:


I suspect Ian's covered this in his ES overhaul work, but it looks 
good to me.


Reviewed-by: Brian Paul bri...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/1] main: validate the type correctly against the OpenGL ES spec.

2012-09-11 Thread Brian Paul 

On 09/11/2012 10:21 AM, Oliver McFadden wrote:

On Tue, Sep 11, 2012 at 07:54:36AM -0600, Brian Paul wrote:

On 09/11/2012 06:09 AM, Oliver McFadden wrote:

Previously Mesa would validate UNSIGNED_INT successfully in violation of
the OpenGL(R) ES 1.0 and 2.0 Specification.

http://www.khronos.org/registry/gles/specs/1.1/es_full_spec_1.1.12.pdf
http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
http://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.0.pdf

Signed-off-by: Oliver McFaddenoliver.mcfad...@linux.intel.com
---
   src/mesa/main/api_validate.c |   22 ++
   1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index d0d2ca4..698394c 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -296,10 +296,32 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum 
mode, const char *name)
   static bool
   valid_elements_type(struct gl_context *ctx, GLenum type, const char *name)
   {
+  /*
+   * Page 22 of the OpenGL(R) ES Version 1.0 Specification (PDF)
+   * and
+   * Page 21 of the OpenGL(R) ES Version 2.0 Specification (PDF) says:
+   *
+   * `type' must be one of UNSIGNED_BYTE or UNSIGNED_SHORT, indicating that the
+   * values in indices are indices of GL type ubyte or ushort, respectively.
+   *
+   * Page 29 of the OpenGL(R) ES Version 3.0 Specification (PDF) says:
+   *
+   * `type' must be one of UNSIGNED_BYTE, UNSIGNED_SHORT, or UNSIGNED_INT,
+   * indicating that the index values are of GL type ubyte, ushort, or uint
+   * respectively.
+   *
+   */
  switch (type) {
  case GL_UNSIGNED_BYTE:
  case GL_UNSIGNED_SHORT:
+  return true;
  case GL_UNSIGNED_INT:
+  if (ctx-API == API_OPENGLES ||
+  (ctx-API == API_OPENGLES2   ctx-Version   30)) {
+ _mesa_error(ctx, GL_INVALID_ENUM, %s(type = %s), name,
+ _mesa_lookup_enum_by_nr(type));
+ return false;
+  }
 return true;

  default:


I suspect Ian's covered this in his ES overhaul work, but it looks
good to me.


You're giving the OK to push to master? I don't want to conflict with
existing work...


Ian hasn't posted all his ES-cleanup patches yet (though maybe they're 
in a public tree of his) so I don't know if there's a conflict.


If you can hold off for a bit, maybe Ian can review and comment.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev