[Mesa-dev] [PATCH] mesa/uniforms: fix get_uniform for doubles

[Ilia Mirkin]

On Wed, Sep 30, 2015 at 3:48 AM, Dave Airlie <airlied at gmail.com> wrote:
> The initial glGetUniformdv support didn't cover all the
> casting cases that are apparantly legal, and cts seems to
> test for them.
>
> I've updated the piglit test to cover these cases now.
>
> cc: "11.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/mesa/main/uniform_query.cpp | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index 0bee594..4b23fe2 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -319,19 +319,12 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>
>        return;
>     }
> -   if ((uni->type->base_type == GLSL_TYPE_DOUBLE &&
> -        returnType != GLSL_TYPE_DOUBLE) ||
> -       (uni->type->base_type != GLSL_TYPE_DOUBLE &&
> -        returnType == GLSL_TYPE_DOUBLE)) {
> -        _mesa_error( ctx, GL_INVALID_OPERATION,
> -                    "glGetnUniform*vARB(incompatible uniform types)");
> -       return;
> -   }
>
>     {
>        unsigned elements = (uni->type->is_sampler())
>          ? 1 : uni->type->components();
>        const int dmul = uni->type->base_type == GLSL_TYPE_DOUBLE ? 2 : 1;
> +      const int rmul = returnType == GLSL_TYPE_DOUBLE ? 2 : 1;
>
>        /* Calculate the source base address *BEFORE* modifying elements to
>         * account for the size of the user's buffer.
> @@ -343,7 +336,7 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>               returnType == GLSL_TYPE_UINT || returnType == GLSL_TYPE_DOUBLE);
>
>        /* doubles have a different size than the other 3 types */
> -      unsigned bytes = sizeof(src[0]) * elements * dmul;
> +      unsigned bytes = sizeof(src[0]) * elements * rmul;
>        if (bufSize < 0 || bytes > (unsigned) bufSize) {
>          _mesa_error( ctx, GL_INVALID_OPERATION,
>                      "glGetnUniform*vARB(out of bounds: bufSize is %d,"
> @@ -387,12 +380,35 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>                case GLSL_TYPE_BOOL:
>                   dst[i].f = src[i].i ? 1.0f : 0.0f;
>                   break;
> +              case GLSL_TYPE_DOUBLE:
> +                 dst[i].f = *(double *)&src[i].f;
> +                 break;

This doesn't seem like it'll work. This is all inside a

    for (unsigned i = 0; i < elements; i++) {

And by definition, we only get here if rtype is double and stype isn't
(or vice-versa). So the indices between src and dst won't match up. Am
I missing something?

>                default:
>                   assert(!"Should not get here.");
>                   break;
>                }
>                break;
> -
> +            case GLSL_TYPE_DOUBLE:
> +               switch (uni->type->base_type) {
> +               case GLSL_TYPE_UINT:
> +                  *(double *)&dst[i].f = (double) src[i].u;
> +                  break;
> +               case GLSL_TYPE_INT:
> +               case GLSL_TYPE_SAMPLER:
> +               case GLSL_TYPE_IMAGE:
> +                  *(double *)&dst[i].f = (double) src[i].i;
> +                  break;
> +               case GLSL_TYPE_BOOL:
> +                 *(double *)&dst[i].f = src[i].i ? 1.0f : 0.0f;
> +                 break;

indentation. Also probably 1.0 : 0.0 -- we're storing it to a double
here. Not that it really matters.

> +               case GLSL_TYPE_FLOAT:
> +                  *(double *)&dst[i].f = (double) src[i].f;
> +                  break;
> +               default:
> +                  assert(!"Should not get here.");
> +                  break;
> +               }
> +               break;
>             case GLSL_TYPE_INT:
>             case GLSL_TYPE_UINT:
>                switch (uni->type->base_type) {
> @@ -419,6 +435,9 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>                case GLSL_TYPE_BOOL:
>                   dst[i].i = src[i].i ? 1 : 0;
>                   break;
> +              case GLSL_TYPE_DOUBLE:
> +                 dst[i].i = *(double *)&src[i].f;
> +                 break;
>                default:
>                   assert(!"Should not get here.");
>                   break;
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev