[Mesa-dev] [PATCH 4/5] mesa: Fix glGetUniformfv of native integer uniforms.

[Kenneth Graunke]

On 08/20/2011 02:47 PM, Christoph Bumiller wrote:
> On 20.08.2011 22:07, Dan McCabe wrote:
>> On 08/20/2011 12:16 AM, Kenneth Graunke wrote:
>>> On 08/19/2011 05:56 PM, Eric Anholt wrote:
>>>> We have to actually convert the values on the way out.  Fixes piglit
>>>> ARB_shader_objects/getuniform.
>>>> ---
>>>>   src/mesa/main/uniforms.c |   32 ++++++++++++++++++++++++++++----
>>>>   1 files changed, 28 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
>>>> index cda840f..fbaa810 100644
>>>> --- a/src/mesa/main/uniforms.c
>>>> +++ b/src/mesa/main/uniforms.c
>>>> @@ -55,13 +55,11 @@ static GLenum
>>>>   base_uniform_type(GLenum type)
>>>>   {
>>>>      switch (type) {
>>>> -#if 0 /* not needed, for now */
>>>>      case GL_BOOL:
>>>>      case GL_BOOL_VEC2:
>>>>      case GL_BOOL_VEC3:
>>>>      case GL_BOOL_VEC4:
>>>>         return GL_BOOL;
>>>> -#endif
>>>>      case GL_FLOAT:
>>>>      case GL_FLOAT_VEC2:
>>>>      case GL_FLOAT_VEC3:
>>>> @@ -408,8 +406,12 @@ get_uniform(struct gl_context *ctx, GLuint
>>>> program, GLint location,
>>>>      else {
>>>>         const struct gl_program_parameter *p =
>>>>            &prog->Parameters->Parameters[paramPos];
>>>> +      gl_constant_value (*values)[4];
>>>>         GLint rows, cols, i, j, k;
>>>>         GLsizei numBytes;
>>>> +      GLenum storage_type;
>>>> +
>>>> +      values = prog->Parameters->ParameterValues + paramPos + offset;
>>>>
>>>>         get_uniform_rows_cols(p,&rows,&cols);
>>>>
>>>> @@ -421,15 +423,37 @@ get_uniform(struct gl_context *ctx, GLuint
>>>> program, GLint location,
>>>>            return;
>>>>         }
>>>>
>>>> +      if (ctx->Const.NativeIntegers) {
>>>> +     storage_type = base_uniform_type(p->DataType);
>>>> +      } else {
>>>> +     storage_type = GL_FLOAT;
>>>> +      }
>>>> +
>>>>         switch (returnType) {
>>>>         case GL_FLOAT:
>>>>            {
>>>>               GLfloat *params = (GLfloat *) paramsOut;
>>>>               k = 0;
>>>>               for (i = 0; i<  rows; i++) {
>>>> -               const int base = paramPos + offset + i;
>>>>                  for (j = 0; j<  cols; j++ ) {
>>>> -                  params[k++] =
>>>> prog->Parameters->ParameterValues[base][j].f;
>>>> +          switch (storage_type) {
>>>> +          case GL_FLOAT:
>>>> +             params[k++] = values[i][j].f;
>>>> +             break;
>>>> +          case GL_INT:
>>>> +             params[k++] = values[i][j].i;
>>>> +             break;
>>>> +          case GL_UNSIGNED_INT:
>>>> +             params[k++] = values[i][j].u;
>>>> +             break;
>>>> +          case GL_BOOL:
>>>> +             params[k++] = values[i][j].b;
>>>> +             break;
>>>> +          default:
>>>> +             _mesa_problem(ctx, "Invalid type in glGetUniform()");
>>>> +             params[k++] = 0.0;
>>>> +             break;
>>>> +          }
>>>>                  }
>>>>               }
>>>>            }
>>>
>>> Argh.  The OpenGL specification is so unclear on what happens when you
>>> call GetUniform{f, i, ui}v on a value of a different type.  Your code
>>> here assumes that it's legal to call GetUniformfv on an int/bool/uint
>>> uniform, and that the uniform's value should be implicitly converted to
>>> floating point.  That seems plausible.
>>>
>>> But what about calling GetUniformiv or GetUniformuiv on a float uniform?
>>>   Is it supposed to convert the float value to an int/uint?  By what
>>> rounding rules...truncate?  We're certainly not doing that now.  If
>>> that's the proper behavior, we need your switch statement in the GL_INT
>>> and GL_UNSIGNED_INT returnType cases as well.
>>>
> The GL4.2 spec is quite clear on the issue:
> from 2.11.7:
> For all other [than boolean] uniform types, except for subroutine
> uniforms and atomic counters, the Uniform* command used must match the
> size and type of the uniform, as declared in the shader. No type
> conversions are done.
> An INVALID_OPERATION error will be generated if an attempt is made to
> use a non-matching Uniform* command.

That's for glUniform, though, which -loads- uniforms.  I found no such
text for gl*Get*Uniform...