[Mesa-dev] [PATCH 3/4] mesa: Draw Indirect return wrong error code on unalinged

[Ian Romanick]

On 10/20/2015 01:03 PM, Ilia Mirkin wrote:
> On Tue, Oct 20, 2015 at 10:19 AM, Marta Lofstedt
> <marta.lofstedt at linux.intel.com> wrote:
>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>
>> From OpenGL 4.4 specification, section 10.4 and
>> Open GL Es 3.1 section 10.5:
>> "An INVALID_VALUE error is generated if indirect is not a multiple
>> of the size, in basic machine units, of uint."
>>
>> However, the current code follow the ARB_draw_indirect:
>> https://www.opengl.org/registry/specs/ARB/draw_indirect.txt
>> "INVALID_OPERATION is generated by DrawArraysIndirect and
>> DrawElementsIndirect if commands source data beyond the end
>> of a buffer object or if <indirect> is not word aligned."
>>
>> Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
>> ---
>>  src/mesa/main/api_validate.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>> index 7062cbd..a084362 100644
>> --- a/src/mesa/main/api_validate.c
>> +++ b/src/mesa/main/api_validate.c
>> @@ -732,10 +732,19 @@ valid_draw_indirect(struct gl_context *ctx,
>>     /* From the ARB_draw_indirect specification:
>>      * "An INVALID_OPERATION error is generated [...] if <indirect> is no
>>      *  word aligned."
>> +    * However, from OpenGL version 4.4. section 10.5
> 
> 4.4,
> 
> I double-checked and you're right -- it was INVALID_OPERATION in GL
> 4.3, but INVALID_VALUE in GL 4.4. Weird.

We (Khronos) changed the error because, in fact, the value is invalid.
Generating GL_INVALID_VALUE is more consistent with other similar errors. :)

Since Khronos doesn't usually update older specs, we (Mesa) usually
interpret such changes as clarifications that should be retroactively
applied.  I really hate having a mess of "if this API generate this
error otherwise generate that error."

Let's just always generate GL_INVALID_VALUE.

> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> 
> Should probably fix up the piglit test as well, if any.

Yes.  My recommendation would be to accept either error in desktop GL <=
4.3, but only accept GL_INVALID_VALUE in other versions / APIs.

>> +    * and OpenGL ES 3.1, section 10.6:
>> +    * "An INVALID_VALUE error is generated if indirect is not a multiple
>> +    * of the size, in basic machine units, of uint."
>>      */
>>     if ((GLsizeiptr)indirect & (sizeof(GLuint) - 1)) {
>> -      _mesa_error(ctx, GL_INVALID_OPERATION,
>> -                  "%s(indirect is not aligned)", name);
>> +      if ((_mesa_is_desktop_gl(ctx) && ctx->Version >= 44) ||
>> +          _mesa_is_gles31(ctx))
>> +         _mesa_error(ctx, GL_INVALID_VALUE,
>> +                     "%s(indirect is not aligned)", name);
>> +      else
>> +         _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                     "%s(indirect is not aligned)", name);
>>        return GL_FALSE;
>>     }
>>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev