Re: [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.

2015-01-22 Thread Emil Velikov
On 20/01/15 15:30, marius.pre...@intel.com wrote:
 From: Marius Predut marius.pre...@intel.com
 
Hi Marius,

Can you rework the lengthy Remove . commit summary. Please try to
explain briefly what you've done. Adding the relevant prefix is also
recommended - here is a wild guess:

mesa: correctly interpret integer to float casts in 

Also when nominating patches for stable, please include the Cc:
mesa-stable... line in the commit message, as per the instructions [1].

Thanks
Emil

[1] Section Marking a commit as a candidate for a stable branch
http://mesa3d.org/devinfo.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.

2015-01-22 Thread Predut, Marius


 -Original Message-
 From: Ian Romanick [mailto:i...@freedesktop.org]
 Sent: Wednesday, January 21, 2015 5:22 AM
 To: Predut, Marius; mesa-dev@lists.freedesktop.org
 Cc: mesa-sta...@lists.freedesktop.org
 Subject: Re: [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT,
 FLOAT_AS_FLT macros.No functional changes, only bug fixed.
 
 On 01/20/2015 07:30 AM, marius.pre...@intel.com wrote:
  From: Marius Predut marius.pre...@intel.com
 
  On 32-bit, for floating point operations is used x86 FPU registers
  instead SSE, reason for  when reinterprets an integer as a float
  result is unexpected (modify floats when they are written to memory).
  This fixes https://bugs.freedesktop.org/show_bug.cgi?id=82668
 
  Reviewed-by: Roberts, Neil Sneil.s.robe...@intel.com
 
 This should be formatted as:
 
 Reviewed-by: Neil Roberts neil.s.robe...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668
 
  ---
   src/mesa/main/context.c   |2 +-
   src/mesa/main/macros.h|   29 ++-
   src/mesa/vbo/vbo_attrib_tmp.h |   43 +-
 ---
   src/mesa/vbo/vbo_exec.h   |3 ++-
   src/mesa/vbo/vbo_exec_api.c   |   25 
   src/mesa/vbo/vbo_exec_eval.c  |   22 -
   src/mesa/vbo/vbo_save_api.c   |   10 +-
   7 files changed, 78 insertions(+), 56 deletions(-)
 
  diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c index
  400c158..3007491 100644
  --- a/src/mesa/main/context.c
  +++ b/src/mesa/main/context.c
  @@ -656,7 +656,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api
 api)
  consts-MaxSamples = 0;
 
  /* GLSL default if NativeIntegers == FALSE */
  -   consts-UniformBooleanTrue = FLT_AS_UINT(1.0f);
  +   consts-UniformBooleanTrue = 1;
 
 As Jason mentioned, this hunk must be dropped.

Jason comments:
Here can be use 0x3f80 (the binary representation of 1.0f) Or, could leave 
a macro.
Agree. Fixed with the next patch.

 
 
  /* GL_ARB_sync */
  consts-MaxServerWaitTimeout = 0x1fff7fffULL; diff --git
  a/src/mesa/main/macros.h b/src/mesa/main/macros.h index
  cd5f2d6..4d245e1 100644
  --- a/src/mesa/main/macros.h
  +++ b/src/mesa/main/macros.h
  @@ -170,27 +170,6 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
  ub = ((GLubyte) F_TO_I((f) * 255.0F))  #endif
 
  -static inline GLfloat INT_AS_FLT(GLint i) -{
  -   fi_type tmp;
  -   tmp.i = i;
  -   return tmp.f;
  -}
  -
  -static inline GLfloat UINT_AS_FLT(GLuint u) -{
  -   fi_type tmp;
  -   tmp.u = u;
  -   return tmp.f;
  -}
  -
  -static inline unsigned FLT_AS_UINT(float f) -{
  -   fi_type tmp;
  -   tmp.f = f;
  -   return tmp.u;
  -}
  -
   /**
* Convert a floating point value to an unsigned fixed point value.
*
  @@ -625,15 +604,11 @@ COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int
  sz, const GLfloat src[4],  {
  switch (type) {
  case GL_FLOAT:
  -  ASSIGN_4V(dst, 0, 0, 0, 1);
  +  ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f);
 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));
  +  ASSIGN_4V(dst, 0, 0, 0, 1);
 break;
 
 I'm having trouble understanding how this is correct.  This makes all three
 cases the same.  They all assign float values {0, 0, 0, 1} to dst.
  Code later in the function (not shown in the patch) then copies possibly
 integer or unsigned values into some of the components.  You then end up with
 a mix of integer values and floating point values.
 
 It seems like this function should take two gl_constant_value as parameters
 instead of GLfloat[4].

Seems here is a similar trouble like before.(
Marek comments on this : Integer one is equal to floating-point zero. 
Floating-point one is equal to 0x3f80
I back to macros see last patch sent(v1).)

 
  default:
 ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
  diff --git a/src/mesa/vbo/vbo_attrib_tmp.h
  b/src/mesa/vbo/vbo_attrib_tmp.h index ec66934..a853cb1 100644
  --- a/src/mesa/vbo/vbo_attrib_tmp.h
  +++ b/src/mesa/vbo/vbo_attrib_tmp.h
  @@ -28,6 +28,41 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
   #include util/u_format_r11g11b10f.h
   #include main/varray.h
 
  +#include program/prog_parameter.h
  +
  +
  +static union gl_constant_value UINT_AS_UNION(GLuint u) {
  +   union gl_constant_value tmp;
  +   tmp.u = u;
  +   return tmp;
  +}
  +
  +static inline union gl_constant_value INT_AS_UNION(GLint i) {
  +   union gl_constant_value tmp;
  +   tmp.i = i;
  +   return tmp;
  +}
  +
  +static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f) {
  +   union gl_constant_value tmp;
  +   tmp.f = f;
  +   return tmp;
  +}
  +
  +/* ATTR */
  +#define ATTR( A, N, T, V0, V1, V2, V3

Re: [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.

2015-01-21 Thread Neil Roberts
Marius, the ‘Reviewed-by’ tag should only be added if someone explicitly
replies to your patch and says that you can add it with their name. It's
supposed to mean that the person is happy for the patch to be pushed to
master. I did not do this, I only looked at a previous version of the
patch briefly and replied saying that it doesn't appear to fix the
problem. I gave some suggestions of where it might need further work.

Regards,
- Neil

marius.pre...@intel.com writes:

 From: Marius Predut marius.pre...@intel.com

 On 32-bit, for floating point operations is used x86 FPU registers instead 
 SSE,
 reason for  when reinterprets an integer as a float result is unexpected
 (modify floats when they are written to memory).
 This fixes https://bugs.freedesktop.org/show_bug.cgi?id=82668

 Reviewed-by: Roberts, Neil Sneil.s.robe...@intel.com
 ---
  src/mesa/main/context.c   |2 +-
  src/mesa/main/macros.h|   29 ++-
  src/mesa/vbo/vbo_attrib_tmp.h |   43 
 +
  src/mesa/vbo/vbo_exec.h   |3 ++-
  src/mesa/vbo/vbo_exec_api.c   |   25 
  src/mesa/vbo/vbo_exec_eval.c  |   22 -
  src/mesa/vbo/vbo_save_api.c   |   10 +-
  7 files changed, 78 insertions(+), 56 deletions(-)

 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index 400c158..3007491 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -656,7 +656,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api 
 api)
 consts-MaxSamples = 0;
  
 /* GLSL default if NativeIntegers == FALSE */
 -   consts-UniformBooleanTrue = FLT_AS_UINT(1.0f);
 +   consts-UniformBooleanTrue = 1;
  
 /* GL_ARB_sync */
 consts-MaxServerWaitTimeout = 0x1fff7fffULL;
 diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
 index cd5f2d6..4d245e1 100644
 --- a/src/mesa/main/macros.h
 +++ b/src/mesa/main/macros.h
 @@ -170,27 +170,6 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
   ub = ((GLubyte) F_TO_I((f) * 255.0F))
  #endif
  
 -static inline GLfloat INT_AS_FLT(GLint i)
 -{
 -   fi_type tmp;
 -   tmp.i = i;
 -   return tmp.f;
 -}
 -
 -static inline GLfloat UINT_AS_FLT(GLuint u)
 -{
 -   fi_type tmp;
 -   tmp.u = u;
 -   return tmp.f;
 -}
 -
 -static inline unsigned FLT_AS_UINT(float f)
 -{
 -   fi_type tmp;
 -   tmp.f = f;
 -   return tmp.u;
 -}
 -
  /**
   * Convert a floating point value to an unsigned fixed point value.
   *
 @@ -625,15 +604,11 @@ COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, 
 const GLfloat src[4],
  {
 switch (type) {
 case GL_FLOAT:
 -  ASSIGN_4V(dst, 0, 0, 0, 1);
 +  ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f);
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));
 +  ASSIGN_4V(dst, 0, 0, 0, 1);
break;
 default:
ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
 diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
 index ec66934..a853cb1 100644
 --- a/src/mesa/vbo/vbo_attrib_tmp.h
 +++ b/src/mesa/vbo/vbo_attrib_tmp.h
 @@ -28,6 +28,41 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
  #include util/u_format_r11g11b10f.h
  #include main/varray.h
  
 +#include program/prog_parameter.h
 +
 +
 +static union gl_constant_value UINT_AS_UNION(GLuint u)
 +{
 +   union gl_constant_value tmp;
 +   tmp.u = u;
 +   return tmp;
 +}
 +
 +static inline union gl_constant_value INT_AS_UNION(GLint i)
 +{
 +   union gl_constant_value tmp;
 +   tmp.i = i;
 +   return tmp;
 +}
 +
 +static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f)
 +{
 +   union gl_constant_value tmp;
 +   tmp.f = f;
 +   return tmp;
 +}
 +
 +/* ATTR */
 +#define ATTR( A, N, T, V0, V1, V2, V3 )ATTR_##T((A), (N), (T), (V0), 
 (V1), (V2), (V3))
 +
 +#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1), 
 UINT_AS_UNION(V2), UINT_AS_UNION(V3))
 +#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1), 
 INT_AS_UNION(V2), INT_AS_UNION(V3))
 +#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1), 
 FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
 +
 +
  /* float */
  #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 )
 @@ -41,8 +76,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
  
  /* int */
  #define ATTRI( A, N, X, Y, Z, W) ATTR( A, N, GL_INT, \
 -   INT_AS_FLT(X), INT_AS_FLT(Y), \
 -   INT_AS_FLT(Z), INT_AS_FLT(W) )
 +   X, Y, 

[Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.

2015-01-20 Thread marius . predut
From: Marius Predut marius.pre...@intel.com

On 32-bit, for floating point operations is used x86 FPU registers instead SSE,
reason for  when reinterprets an integer as a float result is unexpected
(modify floats when they are written to memory).
This fixes https://bugs.freedesktop.org/show_bug.cgi?id=82668

Reviewed-by: Roberts, Neil Sneil.s.robe...@intel.com
---
 src/mesa/main/context.c   |2 +-
 src/mesa/main/macros.h|   29 ++-
 src/mesa/vbo/vbo_attrib_tmp.h |   43 +
 src/mesa/vbo/vbo_exec.h   |3 ++-
 src/mesa/vbo/vbo_exec_api.c   |   25 
 src/mesa/vbo/vbo_exec_eval.c  |   22 -
 src/mesa/vbo/vbo_save_api.c   |   10 +-
 7 files changed, 78 insertions(+), 56 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 400c158..3007491 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -656,7 +656,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api 
api)
consts-MaxSamples = 0;
 
/* GLSL default if NativeIntegers == FALSE */
-   consts-UniformBooleanTrue = FLT_AS_UINT(1.0f);
+   consts-UniformBooleanTrue = 1;
 
/* GL_ARB_sync */
consts-MaxServerWaitTimeout = 0x1fff7fffULL;
diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
index cd5f2d6..4d245e1 100644
--- a/src/mesa/main/macros.h
+++ b/src/mesa/main/macros.h
@@ -170,27 +170,6 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
ub = ((GLubyte) F_TO_I((f) * 255.0F))
 #endif
 
-static inline GLfloat INT_AS_FLT(GLint i)
-{
-   fi_type tmp;
-   tmp.i = i;
-   return tmp.f;
-}
-
-static inline GLfloat UINT_AS_FLT(GLuint u)
-{
-   fi_type tmp;
-   tmp.u = u;
-   return tmp.f;
-}
-
-static inline unsigned FLT_AS_UINT(float f)
-{
-   fi_type tmp;
-   tmp.f = f;
-   return tmp.u;
-}
-
 /**
  * Convert a floating point value to an unsigned fixed point value.
  *
@@ -625,15 +604,11 @@ COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, const 
GLfloat src[4],
 {
switch (type) {
case GL_FLOAT:
-  ASSIGN_4V(dst, 0, 0, 0, 1);
+  ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f);
   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));
+  ASSIGN_4V(dst, 0, 0, 0, 1);
   break;
default:
   ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
index ec66934..a853cb1 100644
--- a/src/mesa/vbo/vbo_attrib_tmp.h
+++ b/src/mesa/vbo/vbo_attrib_tmp.h
@@ -28,6 +28,41 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include util/u_format_r11g11b10f.h
 #include main/varray.h
 
+#include program/prog_parameter.h
+
+
+static union gl_constant_value UINT_AS_UNION(GLuint u)
+{
+   union gl_constant_value tmp;
+   tmp.u = u;
+   return tmp;
+}
+
+static inline union gl_constant_value INT_AS_UNION(GLint i)
+{
+   union gl_constant_value tmp;
+   tmp.i = i;
+   return tmp;
+}
+
+static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f)
+{
+   union gl_constant_value tmp;
+   tmp.f = f;
+   return tmp;
+}
+
+/* ATTR */
+#define ATTR( A, N, T, V0, V1, V2, V3 )  ATTR_##T((A), (N), (T), (V0), 
(V1), (V2), (V3))
+
+#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
+ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1), 
UINT_AS_UNION(V2), UINT_AS_UNION(V3))
+#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
+ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1), INT_AS_UNION(V2), 
INT_AS_UNION(V3))
+#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 )   \
+ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1), 
FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
+
+
 /* float */
 #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 )
@@ -41,8 +76,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* int */
 #define ATTRI( A, N, X, Y, Z, W) ATTR( A, N, GL_INT, \
-   INT_AS_FLT(X), INT_AS_FLT(Y), \
-   INT_AS_FLT(Z), INT_AS_FLT(W) )
+   X, Y, \
+   Z, W )
 
 #define ATTR2IV( A, V ) ATTRI( A, 2, (V)[0], (V)[1], 0, 1 )
 #define ATTR3IV( A, V ) ATTRI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
@@ -56,8 +91,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* uint */
 #define ATTRUI( A, N, X, Y, Z, W) ATTR( A, N, GL_UNSIGNED_INT, \
-UINT_AS_FLT(X), UINT_AS_FLT(Y), \
-UINT_AS_FLT(Z), UINT_AS_FLT(W) )
+X, Y, \
+Z, W )
 
 #define ATTR2UIV( A, V ) ATTRUI( A, 2, (V)[0], 

Re: [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.

2015-01-20 Thread Ian Romanick
On 01/20/2015 07:30 AM, marius.pre...@intel.com wrote:
 From: Marius Predut marius.pre...@intel.com
 
 On 32-bit, for floating point operations is used x86 FPU registers instead 
 SSE,
 reason for  when reinterprets an integer as a float result is unexpected
 (modify floats when they are written to memory).
 This fixes https://bugs.freedesktop.org/show_bug.cgi?id=82668
 
 Reviewed-by: Roberts, Neil Sneil.s.robe...@intel.com

This should be formatted as:

Reviewed-by: Neil Roberts neil.s.robe...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668

 ---
  src/mesa/main/context.c   |2 +-
  src/mesa/main/macros.h|   29 ++-
  src/mesa/vbo/vbo_attrib_tmp.h |   43 
 +
  src/mesa/vbo/vbo_exec.h   |3 ++-
  src/mesa/vbo/vbo_exec_api.c   |   25 
  src/mesa/vbo/vbo_exec_eval.c  |   22 -
  src/mesa/vbo/vbo_save_api.c   |   10 +-
  7 files changed, 78 insertions(+), 56 deletions(-)
 
 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index 400c158..3007491 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -656,7 +656,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api 
 api)
 consts-MaxSamples = 0;
  
 /* GLSL default if NativeIntegers == FALSE */
 -   consts-UniformBooleanTrue = FLT_AS_UINT(1.0f);
 +   consts-UniformBooleanTrue = 1;

As Jason mentioned, this hunk must be dropped.

  
 /* GL_ARB_sync */
 consts-MaxServerWaitTimeout = 0x1fff7fffULL;
 diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
 index cd5f2d6..4d245e1 100644
 --- a/src/mesa/main/macros.h
 +++ b/src/mesa/main/macros.h
 @@ -170,27 +170,6 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
   ub = ((GLubyte) F_TO_I((f) * 255.0F))
  #endif
  
 -static inline GLfloat INT_AS_FLT(GLint i)
 -{
 -   fi_type tmp;
 -   tmp.i = i;
 -   return tmp.f;
 -}
 -
 -static inline GLfloat UINT_AS_FLT(GLuint u)
 -{
 -   fi_type tmp;
 -   tmp.u = u;
 -   return tmp.f;
 -}
 -
 -static inline unsigned FLT_AS_UINT(float f)
 -{
 -   fi_type tmp;
 -   tmp.f = f;
 -   return tmp.u;
 -}
 -
  /**
   * Convert a floating point value to an unsigned fixed point value.
   *
 @@ -625,15 +604,11 @@ COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, 
 const GLfloat src[4],
  {
 switch (type) {
 case GL_FLOAT:
 -  ASSIGN_4V(dst, 0, 0, 0, 1);
 +  ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f);
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));
 +  ASSIGN_4V(dst, 0, 0, 0, 1);
break;

I'm having trouble understanding how this is correct.  This makes all
three cases the same.  They all assign float values {0, 0, 0, 1} to dst.
 Code later in the function (not shown in the patch) then copies
possibly integer or unsigned values into some of the components.  You
then end up with a mix of integer values and floating point values.

It seems like this function should take two gl_constant_value as
parameters instead of GLfloat[4].

 default:
ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
 diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
 index ec66934..a853cb1 100644
 --- a/src/mesa/vbo/vbo_attrib_tmp.h
 +++ b/src/mesa/vbo/vbo_attrib_tmp.h
 @@ -28,6 +28,41 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
  #include util/u_format_r11g11b10f.h
  #include main/varray.h
  
 +#include program/prog_parameter.h
 +
 +
 +static union gl_constant_value UINT_AS_UNION(GLuint u)
 +{
 +   union gl_constant_value tmp;
 +   tmp.u = u;
 +   return tmp;
 +}
 +
 +static inline union gl_constant_value INT_AS_UNION(GLint i)
 +{
 +   union gl_constant_value tmp;
 +   tmp.i = i;
 +   return tmp;
 +}
 +
 +static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f)
 +{
 +   union gl_constant_value tmp;
 +   tmp.f = f;
 +   return tmp;
 +}
 +
 +/* ATTR */
 +#define ATTR( A, N, T, V0, V1, V2, V3 )ATTR_##T((A), (N), (T), (V0), 
 (V1), (V2), (V3))
 +
 +#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1), 
 UINT_AS_UNION(V2), UINT_AS_UNION(V3))
 +#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1), 
 INT_AS_UNION(V2), INT_AS_UNION(V3))
 +#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1), 
 FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
 +
 +
  /* float */
  #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 )
 @@ -41,8 +76,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
  
  /* int */
  #define ATTRI( A, N, X, 

Re: [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.

2015-01-20 Thread Jason Ekstrand
On Tue, Jan 20, 2015 at 7:30 AM, marius.pre...@intel.com wrote:

 From: Marius Predut marius.pre...@intel.com

 On 32-bit, for floating point operations is used x86 FPU registers instead
 SSE,
 reason for  when reinterprets an integer as a float result is unexpected
 (modify floats when they are written to memory).
 This fixes https://bugs.freedesktop.org/show_bug.cgi?id=82668

 Reviewed-by: Roberts, Neil Sneil.s.robe...@intel.com
 ---
  src/mesa/main/context.c   |2 +-
  src/mesa/main/macros.h|   29 ++-
  src/mesa/vbo/vbo_attrib_tmp.h |   43
 +
  src/mesa/vbo/vbo_exec.h   |3 ++-
  src/mesa/vbo/vbo_exec_api.c   |   25 
  src/mesa/vbo/vbo_exec_eval.c  |   22 -
  src/mesa/vbo/vbo_save_api.c   |   10 +-
  7 files changed, 78 insertions(+), 56 deletions(-)

 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index 400c158..3007491 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -656,7 +656,7 @@ _mesa_init_constants(struct gl_constants *consts,
 gl_api api)
 consts-MaxSamples = 0;

 /* GLSL default if NativeIntegers == FALSE */
 -   consts-UniformBooleanTrue = FLT_AS_UINT(1.0f);
 +   consts-UniformBooleanTrue = 1;


This doesn't do what you think it does. FLT_AS_UINT(1.0f) and 1 are very
different values.  We need to leave the above alone as it's the uniform
value passed in as true to uniforms on hardware that can only handle
floating-point values.

I haven't looked very thoroughly at the rest of the patch but I didn't see
anything wrong with it either.
--Jason


 /* GL_ARB_sync */
 consts-MaxServerWaitTimeout = 0x1fff7fffULL;
 diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
 index cd5f2d6..4d245e1 100644
 --- a/src/mesa/main/macros.h
 +++ b/src/mesa/main/macros.h
 @@ -170,27 +170,6 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
 ub = ((GLubyte) F_TO_I((f) * 255.0F))
  #endif

 -static inline GLfloat INT_AS_FLT(GLint i)
 -{
 -   fi_type tmp;
 -   tmp.i = i;
 -   return tmp.f;
 -}
 -
 -static inline GLfloat UINT_AS_FLT(GLuint u)
 -{
 -   fi_type tmp;
 -   tmp.u = u;
 -   return tmp.f;
 -}
 -
 -static inline unsigned FLT_AS_UINT(float f)
 -{
 -   fi_type tmp;
 -   tmp.f = f;
 -   return tmp.u;
 -}
 -
  /**
   * Convert a floating point value to an unsigned fixed point value.
   *
 @@ -625,15 +604,11 @@ COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz,
 const GLfloat src[4],
  {
 switch (type) {
 case GL_FLOAT:
 -  ASSIGN_4V(dst, 0, 0, 0, 1);
 +  ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f);
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));
 +  ASSIGN_4V(dst, 0, 0, 0, 1);
break;
 default:
ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
 diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
 index ec66934..a853cb1 100644
 --- a/src/mesa/vbo/vbo_attrib_tmp.h
 +++ b/src/mesa/vbo/vbo_attrib_tmp.h
 @@ -28,6 +28,41 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
  #include util/u_format_r11g11b10f.h
  #include main/varray.h

 +#include program/prog_parameter.h
 +
 +
 +static union gl_constant_value UINT_AS_UNION(GLuint u)
 +{
 +   union gl_constant_value tmp;
 +   tmp.u = u;
 +   return tmp;
 +}
 +
 +static inline union gl_constant_value INT_AS_UNION(GLint i)
 +{
 +   union gl_constant_value tmp;
 +   tmp.i = i;
 +   return tmp;
 +}
 +
 +static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f)
 +{
 +   union gl_constant_value tmp;
 +   tmp.f = f;
 +   return tmp;
 +}
 +
 +/* ATTR */
 +#define ATTR( A, N, T, V0, V1, V2, V3 )  ATTR_##T((A), (N), (T),
 (V0), (V1), (V2), (V3))
 +
 +#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1),
 UINT_AS_UNION(V2), UINT_AS_UNION(V3))
 +#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
 +ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1),
 INT_AS_UNION(V2), INT_AS_UNION(V3))
 +#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 )   \
 +ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1),
 FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
 +
 +
  /* float */
  #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 )
 @@ -41,8 +76,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.

  /* int */
  #define ATTRI( A, N, X, Y, Z, W) ATTR( A, N, GL_INT, \
 -   INT_AS_FLT(X), INT_AS_FLT(Y), \
 -   INT_AS_FLT(Z), INT_AS_FLT(W) )
 +   X, Y, \
 +   Z, W )

  #define