[Mesa-dev] [PATCH 4/5] mesa: Implement functions for clear_buffer_object extensions

[Brian Paul]

On 12/11/2013 05:21 AM, Pi Tabred wrote:
>
>
> On 10.12.2013 17:44, Brian Paul wrote:
>> On 12/10/2013 06:13 AM, Pi Tabred wrote:
>>> ...
>>> +      }
>>> +   }
>>> +
>>> +   if (!_mesa_is_color_format(format)) {
>>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>>> +                  "glClearBufferData(no color format)");
>>> +   }
>>
>> Are you sure about that error?  Where is this mentioned in the spec?
>> Plus, you're missing a 'return' statement.
>
> I am not sure, but is it possible to convert data from a depth/stencil
> format to a color format? If not, the statement
>
> "INVALID_ENUM is generated by ClearBufferSubData if <format> or <type>
> is not one of the supported format or type tokens."
>
> could be interpreted as to include this error as only certain color
> formats are allowed for the internalformat.

OK, I've read the spec, and I think you're right.  But the code as-is 
may need a few more changes.

Table 3.15 of the 4.2 spec lists a subset of internal formats compared 
to what we check for in get_texbuffer_format().  The 3.15 table doesn't 
contain luminance or intensity values.  Our code might be wrong.  It 
looks like luminance/intensity formats aren't legal for texture buffers 
in core profiles.

I think the call to _mesa_is_color_format() is too lenient since it 
accepts compressed formats, etc.  _mesa_error_check_format_and_type() 
might not catch all the invalid formats either.  <sigh> format/type 
error checking is such a hassle.  I think you should write some piglit 
tests that exercise invalid format/type/internalformat combinations to 
make sure that we really catch these invalid combinations.

-Brian