Re: [Mesa-dev] [PATCH] vbo: fix glVertexAttribI* functions

2012-10-31 Thread Brian Paul

On 10/30/2012 04:32 PM, Marek Olšák wrote:

The functions were broken, because they converted ints to floats.
Now we can finally advertise OpenGL 3.0. ;)

In this commit, the vbo module also tracks the type for each attrib
in addition to the size. It can be one of FLOAT, INT, UNSIGNED_INT.

The little ugliness is the vertex attribs are declared as floats even though
there may be integer values. The code just copies integer values into them
without any conversion.

This implementation passes the glVertexAttribI piglit test which I am going
to commit in piglit soon. The test covers vertex arrays, immediate mode and
display lists.

NOTE: This is likely a candidate for the stable branches.


Looks good.  Just some minor things below.

Reviewed-by: Brian Paul bri...@vmware.com



---
  docs/GL3.txt  |3 +-
  src/mesa/main/macros.h|   44 +
  src/mesa/vbo/vbo_attrib_tmp.h |   86 ++---
  src/mesa/vbo/vbo_context.h|   42 
  src/mesa/vbo/vbo_exec.h   |1 +
  src/mesa/vbo/vbo_exec_api.c   |   29 +-
  src/mesa/vbo/vbo_exec_draw.c  |7 ++--
  src/mesa/vbo/vbo_save.h   |2 +
  src/mesa/vbo/vbo_save_api.c   |   12 --
  src/mesa/vbo/vbo_save_draw.c  |   21 +++---
  10 files changed, 183 insertions(+), 64 deletions(-)

diff --git a/docs/GL3.txt b/docs/GL3.txt
index 4f44764..28f6ae6 100644
--- a/docs/GL3.txt
+++ b/docs/GL3.txt
@@ -34,8 +34,7 @@ sRGB framebuffer format (GL_EXT_framebuffer_sRGB) DONE 
(i965, r600)
  glClearBuffer commandsDONE
  glGetStringi command  DONE
  glTexParameterI, glGetTexParameterI commands  DONE
-glVertexAttribI commands  ~50% done (converts int
- values to 
floats)
+glVertexAttribI commands  DONE
  Depth format cube texturesDONE
  GLX_ARB_create_context (GLX 1.4 is required)  DONE

diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
index 7b7fd1b..f89533f 100644
--- a/src/mesa/main/macros.h
+++ b/src/mesa/main/macros.h
@@ -171,6 +171,26 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
ub = ((GLubyte) F_TO_I((f) * 255.0F))
  #endif

+static inline float INT_AS_FLT(int i)
+{
+   union {
+  int i;
+  float f;
+   } tmp;
+   tmp.i = i;
+   return tmp.f;
+}


We have an fi_type union in imports.h, so:

static inline float
INT_AS_FLT(int i)
{
   fi_type tmp;
   tmp.i = i;
   return tmp.f;
}



+
+static inline float UINT_AS_FLT(unsigned u)
+{
+   union {
+  unsigned u;
+  float f;
+   } tmp;
+   tmp.u = u;
+   return tmp.f;
+}


You could add an unsigned field to the fi_type union and use it here 
too, if you want.





+
  /*@}*/


@@ -573,6 +593,30 @@ do {   \

  /*@}*/

+/** Copy \p sz elements into a homegeneous (4-element) vector, giving
+ * default values to the remaining components.
+ * The default values are chosen based on \p type. */


Closing */ on it's own line, please.



+static inline void
+COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, const GLfloat src[4],
+GLenum type)
+{
+   switch (type) {
+   case GL_FLOAT:
+  ASSIGN_4V(dst, 0, 0, 0, 1);
+  break;
+   case GL_INT:
+  ASSIGN_4V(dst, INT_AS_FLT(0), INT_AS_FLT(0),
+ INT_AS_FLT(0), INT_AS_FLT(1));
+  break;
+   case GL_UNSIGNED_INT:
+  ASSIGN_4V(dst, UINT_AS_FLT(0), UINT_AS_FLT(0),
+ UINT_AS_FLT(0), UINT_AS_FLT(1));
+  break;
+   default:
+  ASSERT(0);
+   }
+   COPY_SZ_4V(dst, sz, src);
+}

  /** \name Linear interpolation functions */
  /*@{*/
diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
index 8848445..6bc53ba 100644
--- a/src/mesa/vbo/vbo_attrib_tmp.h
+++ b/src/mesa/vbo/vbo_attrib_tmp.h
@@ -26,38 +26,46 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
  **/

  /* float */
-#define ATTR1FV( A, V ) ATTR( A, 1, (V)[0], 0, 0, 1 )
-#define ATTR2FV( A, V ) ATTR( A, 2, (V)[0], (V)[1], 0, 1 )
-#define ATTR3FV( A, V ) ATTR( A, 3, (V)[0], (V)[1], (V)[2], 1 )
-#define ATTR4FV( A, V ) ATTR( A, 4, (V)[0], (V)[1], (V)[2], (V)[3] )
+#define ATTR1FV( A, V ) ATTR( A, 1, GL_FLOAT, (V)[0], 0, 0, 1 )
+#define ATTR2FV( A, V ) ATTR( A, 2, GL_FLOAT, (V)[0], (V)[1], 0, 1 )
+#define ATTR3FV( A, V ) ATTR( A, 3, GL_FLOAT, (V)[0], (V)[1], (V)[2], 1 )
+#define ATTR4FV( A, V ) ATTR( A, 4, GL_FLOAT, (V)[0], (V)[1], (V)[2], (V)[3] )

-#define ATTR1F( A, X )  ATTR( A, 1, X, 0, 0, 1 )
-#define ATTR2F( A, X, Y )   ATTR( A, 2, X, Y, 0, 1 )
-#define ATTR3F( A, X, Y, Z )ATTR( A, 3, X, Y, Z, 1 )
-#define ATTR4F( A, X, Y, Z, W ) ATTR( A, 4, X, Y, Z, W )
+#define ATTR1F( A, X )  ATTR( A, 1, GL_FLOAT, X, 0, 0, 1 )
+#define 

[Mesa-dev] [PATCH] vbo: fix glVertexAttribI* functions

2012-10-30 Thread Marek Olšák
The functions were broken, because they converted ints to floats.
Now we can finally advertise OpenGL 3.0. ;)

In this commit, the vbo module also tracks the type for each attrib
in addition to the size. It can be one of FLOAT, INT, UNSIGNED_INT.

The little ugliness is the vertex attribs are declared as floats even though
there may be integer values. The code just copies integer values into them
without any conversion.

This implementation passes the glVertexAttribI piglit test which I am going
to commit in piglit soon. The test covers vertex arrays, immediate mode and
display lists.

NOTE: This is likely a candidate for the stable branches.
---
 docs/GL3.txt  |3 +-
 src/mesa/main/macros.h|   44 +
 src/mesa/vbo/vbo_attrib_tmp.h |   86 ++---
 src/mesa/vbo/vbo_context.h|   42 
 src/mesa/vbo/vbo_exec.h   |1 +
 src/mesa/vbo/vbo_exec_api.c   |   29 +-
 src/mesa/vbo/vbo_exec_draw.c  |7 ++--
 src/mesa/vbo/vbo_save.h   |2 +
 src/mesa/vbo/vbo_save_api.c   |   12 --
 src/mesa/vbo/vbo_save_draw.c  |   21 +++---
 10 files changed, 183 insertions(+), 64 deletions(-)

diff --git a/docs/GL3.txt b/docs/GL3.txt
index 4f44764..28f6ae6 100644
--- a/docs/GL3.txt
+++ b/docs/GL3.txt
@@ -34,8 +34,7 @@ sRGB framebuffer format (GL_EXT_framebuffer_sRGB) DONE 
(i965, r600)
 glClearBuffer commandsDONE
 glGetStringi command  DONE
 glTexParameterI, glGetTexParameterI commands  DONE
-glVertexAttribI commands  ~50% done (converts int
- values to 
floats)
+glVertexAttribI commands  DONE
 Depth format cube texturesDONE
 GLX_ARB_create_context (GLX 1.4 is required)  DONE
 
diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
index 7b7fd1b..f89533f 100644
--- a/src/mesa/main/macros.h
+++ b/src/mesa/main/macros.h
@@ -171,6 +171,26 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
ub = ((GLubyte) F_TO_I((f) * 255.0F))
 #endif
 
+static inline float INT_AS_FLT(int i)
+{
+   union {
+  int i;
+  float f;
+   } tmp;
+   tmp.i = i;
+   return tmp.f;
+}
+
+static inline float UINT_AS_FLT(unsigned u)
+{
+   union {
+  unsigned u;
+  float f;
+   } tmp;
+   tmp.u = u;
+   return tmp.f;
+}
+
 /*@}*/
 
 
@@ -573,6 +593,30 @@ do {   \
 
 /*@}*/
 
+/** Copy \p sz elements into a homegeneous (4-element) vector, giving
+ * default values to the remaining components.
+ * The default values are chosen based on \p type. */
+static inline void
+COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, const GLfloat src[4],
+GLenum type)
+{
+   switch (type) {
+   case GL_FLOAT:
+  ASSIGN_4V(dst, 0, 0, 0, 1);
+  break;
+   case GL_INT:
+  ASSIGN_4V(dst, INT_AS_FLT(0), INT_AS_FLT(0),
+ INT_AS_FLT(0), INT_AS_FLT(1));
+  break;
+   case GL_UNSIGNED_INT:
+  ASSIGN_4V(dst, UINT_AS_FLT(0), UINT_AS_FLT(0),
+ UINT_AS_FLT(0), UINT_AS_FLT(1));
+  break;
+   default:
+  ASSERT(0);
+   }
+   COPY_SZ_4V(dst, sz, src);
+}
 
 /** \name Linear interpolation functions */
 /*@{*/
diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
index 8848445..6bc53ba 100644
--- a/src/mesa/vbo/vbo_attrib_tmp.h
+++ b/src/mesa/vbo/vbo_attrib_tmp.h
@@ -26,38 +26,46 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 **/
 
 /* float */
-#define ATTR1FV( A, V ) ATTR( A, 1, (V)[0], 0, 0, 1 )
-#define ATTR2FV( A, V ) ATTR( A, 2, (V)[0], (V)[1], 0, 1 )
-#define ATTR3FV( A, V ) ATTR( A, 3, (V)[0], (V)[1], (V)[2], 1 )
-#define ATTR4FV( A, V ) ATTR( A, 4, (V)[0], (V)[1], (V)[2], (V)[3] )
+#define ATTR1FV( A, V ) ATTR( A, 1, GL_FLOAT, (V)[0], 0, 0, 1 )
+#define ATTR2FV( A, V ) ATTR( A, 2, GL_FLOAT, (V)[0], (V)[1], 0, 1 )
+#define ATTR3FV( A, V ) ATTR( A, 3, GL_FLOAT, (V)[0], (V)[1], (V)[2], 1 )
+#define ATTR4FV( A, V ) ATTR( A, 4, GL_FLOAT, (V)[0], (V)[1], (V)[2], (V)[3] )
 
-#define ATTR1F( A, X )  ATTR( A, 1, X, 0, 0, 1 )
-#define ATTR2F( A, X, Y )   ATTR( A, 2, X, Y, 0, 1 )
-#define ATTR3F( A, X, Y, Z )ATTR( A, 3, X, Y, Z, 1 )
-#define ATTR4F( A, X, Y, Z, W ) ATTR( A, 4, X, Y, Z, W )
+#define ATTR1F( A, X )  ATTR( A, 1, GL_FLOAT, X, 0, 0, 1 )
+#define ATTR2F( A, X, Y )   ATTR( A, 2, GL_FLOAT, X, Y, 0, 1 )
+#define ATTR3F( A, X, Y, Z )ATTR( A, 3, GL_FLOAT, X, Y, Z, 1 )
+#define ATTR4F( A, X, Y, Z, W ) ATTR( A, 4, GL_FLOAT, X, Y, Z, W )
 
 /* int */
-#define ATTR2IV( A, V ) ATTR( A, 2, (V)[0], (V)[1], 0, 1 )
-#define ATTR3IV( A, V ) ATTR( A, 3, (V)[0], (V)[1], (V)[2], 1 )
-#define ATTR4IV( A, V ) ATTR( A, 4, (V)[0], (V)[1], (V)[2], (V)[3] )
+#define ATTRI( A, N,