On 11.12.2013 16:51, Brian Paul wrote:
> 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
>
all luminance, intensity and alpha formats are removed in core profiles.
It's good that you mention this, that's something I wanted to ask about.
My solution would be to check in get_texbuffer_format() if it's a core
context and if that's the case return MESA_FORMAT_NONE for these formats.
with regards to your second point, I have to disagree. you are right
that _mesa_is_color_format() is too lenient, but the combination with
_mesa_error_check_format_and_type makes it work as compressed formats
are not allowed for the format parameter.
I already wrote some piglit tests for the format/type combination (see
piglit mailing list) but I can extend them.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
