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

Jason Ekstrand jason at jlekstrand.net
Tue Jan 20 17:48:17 PST 2015


On Tue, Jan 20, 2015 at 7:30 AM, <marius.predut at intel.com> wrote:

> From: Marius Predut <marius.predut at 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 S<neil.s.roberts at 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 = 0x1fff7fffffffULL;
> 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], (V)[1], 0, 1 )
>  #define ATTR3UIV( A, V ) ATTRUI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
> diff --git a/src/mesa/vbo/vbo_exec.h b/src/mesa/vbo/vbo_exec.h
> index bb265de..7db6b6b 100644
> --- a/src/mesa/vbo/vbo_exec.h
> +++ b/src/mesa/vbo/vbo_exec.h
> @@ -38,6 +38,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
>  #include "vbo.h"
>  #include "vbo_attrib.h"
>
> +#include "program/prog_parameter.h"
>
>  /**
>   * Max number of primitives (number of glBegin/End pairs) per VBO.
> @@ -104,7 +105,7 @@ struct vbo_exec_context
>        GLenum attrtype[VBO_ATTRIB_MAX];
>        GLubyte active_sz[VBO_ATTRIB_MAX];
>
> -      GLfloat *attrptr[VBO_ATTRIB_MAX];
> +      gl_constant_value *attrptr[VBO_ATTRIB_MAX];
>        struct gl_client_array arrays[VERT_ATTRIB_MAX];
>
>        /* According to program mode, the values above plus current
> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
> index 5f8250e..b6c1055 100644
> --- a/src/mesa/vbo/vbo_exec_api.c
> +++ b/src/mesa/vbo/vbo_exec_api.c
> @@ -46,7 +46,6 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
>  #include "vbo_context.h"
>  #include "vbo_noop.h"
>
> -
>  #ifdef ERROR
>  #undef ERROR
>  #endif
> @@ -163,7 +162,7 @@ static void vbo_exec_copy_to_current( struct
> vbo_exec_context *exec )
>
>           COPY_CLEAN_4V_TYPE_AS_FLOAT(tmp,
>                                       exec->vtx.attrsz[i],
> -                                     exec->vtx.attrptr[i],
> +                                     (GLfloat*)exec->vtx.attrptr[i],
>                                       exec->vtx.attrtype[i]);
>
>           if (exec->vtx.attrtype[i] != vbo->currval[i].Type ||
> @@ -216,10 +215,10 @@ vbo_exec_copy_from_current(struct vbo_exec_context
> *exec)
>     for (i = VBO_ATTRIB_POS + 1; i < VBO_ATTRIB_MAX; i++) {
>        const GLfloat *current = (GLfloat *) vbo->currval[i].Ptr;
>        switch (exec->vtx.attrsz[i]) {
> -      case 4: exec->vtx.attrptr[i][3] = current[3];
> -      case 3: exec->vtx.attrptr[i][2] = current[2];
> -      case 2: exec->vtx.attrptr[i][1] = current[1];
> -      case 1: exec->vtx.attrptr[i][0] = current[0];
> +      case 4: exec->vtx.attrptr[i][3].f = current[3];
> +      case 3: exec->vtx.attrptr[i][2].f = current[2];
> +      case 2: exec->vtx.attrptr[i][1].f = current[1];
> +      case 1: exec->vtx.attrptr[i][0].f = current[0];
>          break;
>        }
>     }
> @@ -291,7 +290,7 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context
> *exec,
>
>        for (i = 0 ; i < VBO_ATTRIB_MAX ; i++) {
>          if (exec->vtx.attrsz[i]) {
> -           exec->vtx.attrptr[i] = tmp;
> +           exec->vtx.attrptr[i] = (gl_constant_value*) tmp;
>             tmp += exec->vtx.attrsz[i];
>          }
>          else
> @@ -305,8 +304,8 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context
> *exec,
>     }
>     else {
>        /* Just have to append the new attribute at the end */
> -      exec->vtx.attrptr[attr] = exec->vtx.vertex +
> -        exec->vtx.vertex_size - newSize;
> +      exec->vtx.attrptr[attr] = (gl_constant_value*)(exec->vtx.vertex +
> +        exec->vtx.vertex_size - newSize);
>     }
>
>     /* Replay stored vertices to translate them
> @@ -327,7 +326,7 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context
> *exec,
>
>             if (sz) {
>                GLint old_offset = old_attrptr[j] - exec->vtx.vertex;
> -              GLint new_offset = exec->vtx.attrptr[j] - exec->vtx.vertex;
> +              GLint new_offset = (GLfloat*)exec->vtx.attrptr[j] -
> exec->vtx.vertex;
>
>                if (j == attr) {
>                   if (oldSize) {
> @@ -383,7 +382,7 @@ vbo_exec_fixup_vertex(struct gl_context *ctx, GLuint
> attr, GLuint newSize)
>         * zeros.  Don't need to flush or wrap.
>         */
>        for (i = newSize; i <= exec->vtx.attrsz[attr]; i++)
> -        exec->vtx.attrptr[attr][i-1] = id[i-1];
> +        exec->vtx.attrptr[attr][i-1].f = id[i-1];
>     }
>
>     exec->vtx.active_sz[attr] = newSize;
> @@ -401,7 +400,7 @@ vbo_exec_fixup_vertex(struct gl_context *ctx, GLuint
> attr, GLuint newSize)
>   * This macro is used to implement all the glVertex, glColor, glTexCoord,
>   * glVertexAttrib, etc functions.
>   */
> -#define ATTR( A, N, T, V0, V1, V2, V3 )
>       \
> +#define ATTR_UNION( A, N, T, V0, V1, V2, V3 )
>       \
>  do {                                                                   \
>     struct vbo_exec_context *exec = &vbo_context(ctx)->exec;            \
>                                                                         \
> @@ -412,7 +411,7 @@ do {
>                       \
>        vbo_exec_fixup_vertex(ctx, A, N);
>       \
>                                                                         \
>     {                                                                   \
> -      GLfloat *dest = exec->vtx.attrptr[A];                            \
> +      gl_constant_value *dest = exec->vtx.attrptr[A];
>       \
>        if (N>0) dest[0] = V0;                                           \
>        if (N>1) dest[1] = V1;                                           \
>        if (N>2) dest[2] = V2;                                           \
> diff --git a/src/mesa/vbo/vbo_exec_eval.c b/src/mesa/vbo/vbo_exec_eval.c
> index 82f89b9..670e763 100644
> --- a/src/mesa/vbo/vbo_exec_eval.c
> +++ b/src/mesa/vbo/vbo_exec_eval.c
> @@ -33,6 +33,18 @@
>  #include "vbo_exec.h"
>
>
> +/** Copy \p SZ elements into a 4-element vector */
> +#define COPY_UNION_SZ_4V(DST, SZ, SRC)  \
> +do {                              \
> +   switch (SZ) {                  \
> +   case 4: (DST)[3].f = (SRC)[3];   \
> +   case 3: (DST)[2].f = (SRC)[2];   \
> +   case 2: (DST)[1].f = (SRC)[1];   \
> +   case 1: (DST)[0].f = (SRC)[0];   \
> +   }                              \
> +} while(0)
> +
> +
>  static void clear_active_eval1( struct vbo_exec_context *exec, GLuint
> attr )
>  {
>     assert(attr < Elements(exec->eval.map1));
> @@ -138,9 +150,9 @@ void vbo_exec_do_EvalCoord1f(struct vbo_exec_context
> *exec, GLfloat u)
>                                    exec->eval.map1[attr].sz,
>                                    map->Order);
>
> -        COPY_SZ_4V( exec->vtx.attrptr[attr],
> -                    exec->vtx.attrsz[attr],
> -                    data );
> +        COPY_UNION_SZ_4V( exec->vtx.attrptr[attr],
> +             exec->vtx.attrsz[attr],
> +             data );
>        }
>     }
>
> @@ -186,7 +198,7 @@ void vbo_exec_do_EvalCoord2f( struct vbo_exec_context
> *exec,
>                                   exec->eval.map2[attr].sz,
>                                   map->Uorder, map->Vorder);
>
> -        COPY_SZ_4V( exec->vtx.attrptr[attr],
> +        COPY_UNION_SZ_4V( exec->vtx.attrptr[attr],
>                      exec->vtx.attrsz[attr],
>                      data );
>        }
> @@ -225,7 +237,7 @@ void vbo_exec_do_EvalCoord2f( struct vbo_exec_context
> *exec,
>           NORMALIZE_3FV(normal);
>          normal[3] = 1.0;
>
> -        COPY_SZ_4V( exec->vtx.attrptr[VBO_ATTRIB_NORMAL],
> +        COPY_UNION_SZ_4V( exec->vtx.attrptr[VBO_ATTRIB_NORMAL],
>                      exec->vtx.attrsz[VBO_ATTRIB_NORMAL],
>                      normal );
>
> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
> index 5055c22..2a43f15 100644
> --- a/src/mesa/vbo/vbo_save_api.c
> +++ b/src/mesa/vbo/vbo_save_api.c
> @@ -769,7 +769,7 @@ _save_reset_vertex(struct gl_context *ctx)
>   * 3f version won't otherwise set color[3] to 1.0 -- this is the job
>   * of the chooser function when switching between Color4f and Color3f.
>   */
> -#define ATTR(A, N, T, V0, V1, V2, V3)                          \
> +#define ATTR_UNION(A, N, T, V0, V1, V2, V3)                            \
>  do {                                                           \
>     struct vbo_save_context *save = &vbo_context(ctx)->save;    \
>                                                                 \
> @@ -778,10 +778,10 @@ do {
>               \
>                                                                 \
>     {                                                           \
>        GLfloat *dest = save->attrptr[A];                                \
> -      if (N>0) dest[0] = V0;                                   \
> -      if (N>1) dest[1] = V1;                                   \
> -      if (N>2) dest[2] = V2;                                   \
> -      if (N>3) dest[3] = V3;                                   \
> +      if (N>0) dest[0] = V0.f;                                 \
> +      if (N>1) dest[1] = V1.f;                                 \
> +      if (N>2) dest[2] = V2.f;                                 \
> +      if (N>3) dest[3] = V3.f;                                 \
>        save->attrtype[A] = T;                                    \
>     }                                                           \
>                                                                 \
> --
> 1.7.9.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20150120/e49633cf/attachment-0001.html>


More information about the mesa-stable mailing list