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

[Eduardo Lima Mitev]

On 06/05/2015 03:38 AM, 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...
> 

The OpenGL(ES) specs are full of this kind of distinctions, where
different errors are emitted depending on subtly different error conditions.

(Stupid) tests like this are the result of such fine-grained error
reporting in the specs.

> 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 quite a few cases where the OpenGL(ES) manuals and the spec
have ambiguous differences, and I have come to learn that the spec
always has the last word :).

cheers,
Eduardo