[Mesa-dev] [PATCH 2/6] mesa: Clamp GetUniformuiv values to be >= 0

[Nicolai Hähnle]

On 11.05.2017 13:10, Iago Toral Quiroga wrote:
> From: Kenneth Graunke <kenneth at whitecape.org>
>
> Section 2.2.2 (Data Conversions For State Query Commands) of the
> OpenGL 4.5 October 24th 2016 specification says:
>
> "If a command returning unsigned integer data is called, such as
>  GetSamplerParameterIuiv, negative values are clamped to zero."
>
> v2: uint to int conversion should clamp to INT_MAX (Nicolai)
>
> v3 (Iago)
>   - Add conversions conversions from 64-bit integer paths
>   - Rebase on master
>
> Fixes:
> KHR-GL45.gpu_shader_fp64.state_query
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com> (v2)
> ---
>  src/mesa/main/uniform_query.cpp | 64 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 51 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index bd5b4c4..315973a 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -352,18 +352,9 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>         * just memcpy the data.  If the types are not compatible, perform a
>         * slower convert-and-copy process.
>         */
> -      if (returnType == uni->type->base_type
> -          || ((returnType == GLSL_TYPE_INT
> -               || returnType == GLSL_TYPE_UINT)
> -              &&
> -              (uni->type->base_type == GLSL_TYPE_INT
> -               || uni->type->base_type == GLSL_TYPE_UINT
> -               || uni->type->is_sampler()
> -               || uni->type->is_image()))
> -               || ((returnType == GLSL_TYPE_UINT64 ||
> -                    returnType == GLSL_TYPE_INT64) &&
> -                  (uni->type->base_type == GLSL_TYPE_UINT64 ||
> -                   uni->type->base_type == GLSL_TYPE_INT64))) {
> +      if (returnType == uni->type->base_type ||
> +          ((returnType == GLSL_TYPE_INT || returnType == GLSL_TYPE_UINT) &&
> +           (uni->type->is_sampler() || uni->type->is_image()))) {
>           memcpy(paramsOut, src, bytes);
>        } else {
>           union gl_constant_value *const dst =
> @@ -461,7 +452,6 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>                 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
> @@ -486,6 +476,9 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>                 case GLSL_TYPE_BOOL:
>                    dst[didx].i = src[sidx].i ? 1 : 0;
>                    break;
> +               case GLSL_TYPE_UINT:
> +                  dst[didx].i = MIN2(src[sidx].i, INT_MAX);
> +                  break;
>                 case GLSL_TYPE_DOUBLE: {
>                    double tmp;
>                    memcpy(&tmp, &src[sidx].f, sizeof(tmp));
> @@ -510,6 +503,51 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>                 }
>                 break;
>
> +            case GLSL_TYPE_UINT:
> +               switch (uni->type->base_type) {
> +               case GLSL_TYPE_FLOAT:
> +                  /* The spec isn't terribly clear how to handle negative
> +                   * values with an unsigned return type.
> +                   *
> +                   * GL 4.5 section 2.2.2 ("Data Conversions for State
> +                   * Query Commands") 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."
> +                   */
> +                  dst[didx].i = src[sidx].f < 0.0f ? 0 : IROUND(src[sidx].f);

This needs an unsigned rounding function, doesn't it?


> +                  break;
> +               case GLSL_TYPE_BOOL:
> +                  dst[didx].i = src[sidx].i ? 1 : 0;
> +                  break;
> +               case GLSL_TYPE_INT:
> +                  dst[didx].i = MAX2(src[sidx].i, 0);
> +                  break;
> +               case GLSL_TYPE_DOUBLE: {
> +                  double tmp;
> +                  memcpy(&tmp, &src[sidx].f, sizeof(tmp));
> +                  dst[didx].i = tmp < 0.0 ? 0 : IROUNDD(tmp);
> +                  break;

Also, unsigned rounding.

Cheers,
Nicolai

> +               }
> +               case GLSL_TYPE_UINT64: {
> +                  uint64_t tmp;
> +                  memcpy(&tmp, &src[sidx].u, sizeof(tmp));
> +                  dst[didx].i = MIN2(tmp, INT_MAX);
> +                  break;
> +               }
> +               case GLSL_TYPE_INT64: {
> +                  int64_t tmp;
> +                  memcpy(&tmp, &src[sidx].i, sizeof(tmp));
> +                  dst[didx].i = MAX2(tmp, 0);
> +                  break;
> +               }
> +               default:
> +                  unreachable("invalid uniform type");
> +               }
> +               break;
> +
>              case GLSL_TYPE_INT64:
>              case GLSL_TYPE_UINT64:
>                 switch (uni->type->base_type) {
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.