[Mesa-dev] [PATCH 1/5] mesa: Fix errors values returned by glShaderBinary()

[Ian Romanick]

On 06/04/2015 06:38 PM, Ben Widawsky wrote:
> On Wed, Mar 11, 2015 at 10:01:24AM +0100, Eduardo Lima Mitev wrote:
>> Page 68, section 7.2 'Shader Binaries" of the of the OpenGL ES 3.1,
>> and page 88 of the OpenGL 4.5 specs state:
>>
>>     "An INVALID_VALUE error is generated if count or length is negative.
>>      An INVALID_ENUM error is generated if binaryformat is not a supported
>>      format returned in SHADER_BINARY_FORMATS."
>>
>> Currently, an INVALID_OPERATION error is returned for all cases.
>>
>> Fixes 1 dEQP test:
>> * dEQP-GLES3.functional.negative_api.shader.shader_binary
>> ---
>>  src/mesa/main/shaderapi.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index 5731d58..49a9c80 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -1711,7 +1711,20 @@ _mesa_ShaderBinary(GLint n, const GLuint* shaders, GLenum binaryformat,
>>     (void) binaryformat;
>>     (void) binary;
>>     (void) length;
>> -   _mesa_error(ctx, GL_INVALID_OPERATION, "glShaderBinary");
>> +
>> +   /* Page 68, section 7.2 'Shader Binaries" of the of the OpenGL ES 3.1, and
>> +    * page 88 of the OpenGL 4.5 specs state:
>> +    *
>> +    *     "An INVALID_VALUE error is generated if count or length is negative.
>> +    *      An INVALID_ENUM error is generated if binaryformat is not a supported
>> +    *      format returned in SHADER_BINARY_FORMATS."
>> +    */
>> +   if (n < 0 || length < 0) {
>> +      _mesa_error(ctx, GL_INVALID_VALUE, "glShaderBinary(count or length < 0)");
>> +      return;
>> +   }
>> +
>> +   _mesa_error(ctx, GL_INVALID_ENUM, "glShaderBinary(format)");
>>  }
>>  
>>  
> 
> Seems like a stupid test, seems like INVALID_ENUM is always the right choice
> given the situation...

Well, I was lazy. :)  It probably is technically more correct to
validate the input and generate errors, then generate INVALID_ENUM.

> Anyway, I see this
> (https://www.khronos.org/opengles/sdk/docs/man3/html/glShaderBinary.xhtml):
> 
>   GL_INVALID_OPERATION is generated if more than one of the handles in shaders
>   refers to the same shader object. 
> 
>   GL_INVALID_VALUE is generated if the data pointed to by binary does not match
>   the format specified by binaryFormat. 
> 
> Kinda confused what's right. It doesn't seem to match your implementation.

There are some generic errors that apply to all GL functions.  These are
listed in section 2.3.1 (Errors).  The full text from section 7.2 (from
the GL 4.4 spec) says:

    An INVALID_VALUE error is generated if count or length is negative.

    An INVALID_ENUM error is generated if binaryformat is not a
    supported format returned in SHADER_BINARY_FORMATS.

    An INVALID_VALUE error is generated if the data pointed to by binary
    does not match the specified binaryformat.

    An INVALID_VALUE error is generated if any of the handles in
    shaders is not the name of either a program or shader object.

    An INVALID_OPERATION error is generated if any of the handles in
    shader is the name of a program object.

    An INVALID_OPERATION error is generated if more than one of the
    handles in shaders refers to the same type of shader object.

    Additional errors corresponding to specific binary formats may be
    generated as specified by the extensions defining those formats.

If we're going to add code to generate two of the specified error codes,
we probably ought to add code to generate the other as well.

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev