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

[Christoph Bumiller]

On 21.08.2011 06:00, Kenneth Graunke wrote:
> 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...
Right, sorry, I assumed the behaviour for load might apply to Get as
well, but it doesn't.

Then most likely 6.1.2 applies, Querying GL State, Data Conversions.

E.g. float to int is rounded to nearest except for RGBA or depth values,
booleans are returned as 1/0 or 1.0/0.0, ...