Nicolai Hähnle wrote on 10.12.2016 14:54:
> From: Nicolai Hähnle <nicolai.haeh...@amd.com>
> 
> Section 2.2.2 (Data Conversions For State Query Commands) of the OpenGL 4.5
> (Compatibility Profile) spec says:
> 
>     "If a value is so large in magnitude that it cannot be represented by the
>      returned data type, then the nearest value representable using the
>      requested type is returned."
> 
> This is reasonably interpreted as saying that e.g. float uniforms with a
> negative value should be clamped to 0 when queried as an unsigned integer.
> 
> Fixes GL45-CTS.gpu_shader_fp64.state_query.
> ---
>  src/mesa/main/imports.h         | 17 +++++++++++++++++
>  src/mesa/main/uniform_query.cpp | 21 ++++++++++++++++++++-
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/imports.h b/src/mesa/main/imports.h
> index ef7c378..2af0add 100644
> --- a/src/mesa/main/imports.h
> +++ b/src/mesa/main/imports.h
> @@ -158,20 +158,37 @@ static inline int IROUNDD(double d)
>  }
>  
>  /**
>   * Convert float to int64 by rounding to nearest integer.
>   */
>  static inline GLint64 IROUND64(float f)
>  {
>     return (GLint64) ((f >= 0.0F) ? (f + 0.5F) : (f - 0.5F));
>  }
>  
> +/**
> + * Convert float to unsigned int by rounding to nearest integer, away from 
> zero.
> + */
> +static inline unsigned int UROUND(float f)
> +{
> +   return (unsigned int) ((f >= 0.0F) ? (f + 0.5F) : 0.0F);
> +}
> +
> +/**
> + * Convert double to unsigned int by rounding to nearest integer, away from
> + * zero.
> + */
> +static inline unsigned int UROUNDD(double d)
> +{
> +   return (unsigned int) ((d >= 0.0) ? (d + 0.5) : 0.0);
> +}
> +
>  
>  /**
>   * Convert positive float to int by rounding to nearest integer.
>   */
>  static inline int IROUND_POS(float f)
>  {
>     assert(f >= 0.0F);
>     return (int) (f + 0.5F);
>  }
>  
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index 3108d34..8e390cc 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -415,21 +415,20 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint 
> program, GLint location,
>                    double tmp = src[sidx].f;
>                    memcpy(&dst[didx].f, &tmp, sizeof(tmp));
>                 break;
>                 }
>              default:
>                 assert(!"Should not get here.");
>                 break;
>              }
>              break;
>           case GLSL_TYPE_INT:
> -         case GLSL_TYPE_UINT:
>              switch (uni->type->base_type) {
>              case GLSL_TYPE_FLOAT:
>                 /* While the GL 3.2 core spec doesn't explicitly
>                  * state how conversion of float uniforms to integer
>                  * values works, in section 6.2 "State Tables" on
>                  * page 267 it says:
>                  *
>                  *     "Unless otherwise specified, when floating
>                  *      point state is returned as integer values or
>                  *      integer state is returned as floating-point
> @@ -452,20 +451,40 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint 
> program, GLint location,
>                    memcpy(&tmp, &src[sidx].f, sizeof(tmp));
>                    dst[didx].i = IROUNDD(tmp);
>                 break;
>                 }
>              default:
>                 assert(!"Should not get here.");
>                 break;
>              }
>              break;
>  
> +         case GLSL_TYPE_UINT:
> +            switch (uni->type->base_type) {
> +            case GLSL_TYPE_FLOAT:
> +               dst[didx].u = UROUND(src[sidx].f);
> +               break;
> +            case GLSL_TYPE_BOOL:
> +               dst[didx].u = src[sidx].i ? 1 : 0;
> +               break;
> +               case GLSL_TYPE_DOUBLE: {
> +                  double tmp;
> +                  memcpy(&tmp, &src[sidx].f, sizeof(tmp));
> +                  dst[didx].u = UROUNDD(tmp);
> +               break;
> +               }
> +            default:
> +               assert(!"Should not get here.");

Shouldn't this be a call to unreachable()? (And if so, then probably change the
default a few lines below as well?)

> +               break;
> +            }
> +            break;
> +
>           default:
>              assert(!"Should not get here.");
>              break;
>           }
>        }
>        }
>     }
>  }
>  
>  static void

The rest of the change looks reasonable to me.

Cheers,
Kai

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to