[Mesa-dev] [Mesa-stable] [PATCH] mesa/main: fix version/extension checks in _mesa_ClampColor

[Ilia Mirkin]

On Thu, Jan 12, 2017 at 12:37 PM, Nicolai Hähnle
<nicolai.haehnle at amd.com> wrote:
> On 12.01.2017 17:57, Ilia Mirkin wrote:
>>
>> On Thu, Jan 12, 2017 at 11:46 AM, Nicolai Hähnle <nhaehnle at gmail.com>
>> wrote:
>>>
>>> On 12.01.2017 17:41, Ilia Mirkin wrote:
>>>>
>>>>
>>>> On Thu, Jan 12, 2017 at 11:39 AM, Emil Velikov
>>>> <emil.l.velikov at gmail.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 12 January 2017 at 09:55, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>>>>>
>>>>>>
>>>>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>>>>
>>>>>> Add a proper check for feature support, and raise an invalid enum for
>>>>>> GL_CLAMP_VERTEX/FRAGMENT_COLOR unconditionally in core profiles, since
>>>>>> those enums were explicitly removed after the extension was promoted
>>>>>> to core functionality (not in the profile sense) with OpenGL 3.0.
>>>>>>
>>>>> Wouldn't it be better to simply not advertise the extension for Core
>>>>> profiles ?
>>>>
>>>>
>>>>
>>>> I'm fine with either approach FWIW. My initial comments were based on
>>>> the theory that the ext enabled clamping in core contexts, but that
>>>> doesn't sound like it's the case.
>>>>
>>>> Could be more straightforward to just flip the ext to be GLL instead.
>>>
>>>
>>>
>>> I'm fine with that.
>>>
>>> I should point out though that this patch as-is makes some sense
>>> regardless,
>>> because the check for feature support was actually totally missing (think
>>> GL
>>> < 3.0, no support for float buffers -- that would have let one of the
>>> enums
>>> through).
>>
>>
>> Perhaps make the ext GLL and make this function check
>> _mesa_has_ARB_color_buffer_float() which should DTRT?
>
>
> Actually, _mesa_ClampColor would have to check ARB_color_buffer_float
> directly, because it needs to be supported in core contexts.
>
> Also, this requires the Intel driver to set ARB_color_buffer_float
> unconditionally, but then there's a bunch of meta code that might fail. This
> is surprisingly messy :/

OK. My suggestions were meant to make things simpler, not harder. If
they make things harder, ignore them. You can have a R-b from me on
either patch that you sent - if you'd like to do some third thing, I'd
probably be fine with that as well.

  -ilia