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

2015-10-21 Thread Ian Romanick
On 10/20/2015 01:03 PM, Ilia Mirkin wrote:
> On Tue, Oct 20, 2015 at 10:19 AM, Marta Lofstedt
>  wrote:
>> From: Marta Lofstedt 
>>
>> 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  is not word aligned."
>>
>> Signed-off-by: Marta Lofstedt 
>> ---
>>  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  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 
> 
> 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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-10-20 Thread Marta Lofstedt
From: Marta Lofstedt 

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  is not word aligned."

Signed-off-by: Marta Lofstedt 
---
 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  is no
 *  word aligned."
+* However, from OpenGL version 4.4. section 10.5
+* 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-10-20 Thread Ilia Mirkin
On Tue, Oct 20, 2015 at 10:19 AM, Marta Lofstedt
 wrote:
> From: Marta Lofstedt 
>
> 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  is not word aligned."
>
> Signed-off-by: Marta Lofstedt 
> ---
>  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  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.

Reviewed-by: Ilia Mirkin 

Should probably fix up the piglit test as well, if any.

> +* 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev