[Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags

[Roland Scheidegger]

Am 08.09.2011 23:13, schrieb Marek Olšák:
> On Thu, Sep 8, 2011 at 9:03 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 08.09.2011 19:53, schrieb Ian Romanick:
>>> On 09/06/2011 03:21 PM, Roland Scheidegger wrote:
>>>> Am 06.09.2011 22:13, schrieb Ian Romanick:
>>>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>>>
>>>>> Core Mesa already does the dispatch offset remapping for every
>>>>> function that could possibly ever be supported.  There's no need to
>>>>> continue using that cruft in the driver.
>>>>>
>>>>> Since the call to _mesa_enable_imaging_extensions (via
>>>>> driInitExtensions) is removed, EXT_blend_logic_op is explicitly added
>>>>> to the list.  EXT_blend_color is also added, but it depends on the
>>>>> drmSupportsBlendColor flag.
>>>
>>>> Hmm, I don't think EXT_blend_logic_op was advertized before. The reason
>>>> for this is that EXT_blend_logic_op together with
>>>
>>> EXT_blend_logic_op *was* previously enabled.  r200CreateContext called
>>> driInitExtensions( ctx, card_extensions, GL_TRUE );.  The GL_TRUE
>>> parameter tells driInitExtensions to call
>>> _mesa_enable_imaging_extensions.  _mesa_enable_imaging_extensions in
>>> turn enables:
>>>
>>>    GL_EXT_blend_color
>>>    GL_EXT_blend_logic_op
>>>    GL_EXT_blend_minmax
>>>    GL_EXT_blend_subtract
>> That's right. _mesa_enable_imaging_extensions however did not always
>> enable EXT_blend_logic_op.
>> I was curious (as I was sure it was correct at one point for r200) when
>> this got broken for r200 and that's the answer:
>> 6775c1e8ccc2c543d97eb273a342140a62d99aee - that is OLD (interestingly
>> the commit mentions some discussion apparently however I did not realize
>> it in fact made r200 advertize EXT_blend_logic_op which I knew would be
>> incorrect).
>>
>>>
>>> I didn't see anything in r200_state.c to handle blend equation being set
>>> to GL_LOGIC_OP.
>> Yes - there was code initially handling this (trivial as long as it's
>> the same on all RGBA channels) but I removed that a decade or so ago
>> when adding support for EXT_blend_equation_separate (and removing
>> support for EXT_blend_logic_op at the same time).
>>
>>>
>>> Of course, we have *zero* piglit tests for this extension.
>>>
>>>> EXT_blend_equation_separate allows some unholy combinations which the
>>>> r200 (possibly other hw too) can't handle correctly. Namely this
>>>> combination makes it possible to have logic ops on rgb or alpha channels
>>>> and color blending on the other channels.
>>>> I know that at least sometime in the past this driver did not advertize
>>>> EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that
>>>> problem and EXT_blend_logic_op wasn't really all that important. I guess
>>>> though it's not exactly a severe problem since surely apps old enough to
>>>> use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate
>>>> (though in theory some app could be clever and really want to do that...).
>>>
>>> That's a good point.  I suspect that no hardware actually handles this
>>> case correctly.  I seem to recall that this is the reason NVIDIA doesn't
>>> support GL_EXT_blend_logic_op in their drivers.  I know the non-Quadro
>>> cards don't support it, anyway.
>>>
>>> Does this work on later chips in the Radeon family?
>>>
>>> I don't think anyone will miss GL_EXT_blend_logic_op if we just remove
>>> it altogether.
>>
>> I don't think it works at least for r300. FWIW there's a mesa helper
>> function (_mesa_rgba_logicop_enabled) which also only makes sense if the
>> logicop blend equation is set for either both of none of RGB/A.
>>
>> I certainly wouldn't mourn the loss of EXT_blend_logic_op.
> 
> Gallium implements blend_logic_op in terms of pure GL1.1 logic op. Assuming
> that's incorrect, we shouldn't advertise that extension for Gallium at
> all FWIW.

This sounds correct - I vaguely remember that we found noone really
needs separate RGB/A logic op blend equation (certainly not d3d it's
purely there for OpenGL) so that's how it was implemented.
Though actually mesa doesn't implement EXT_blend_logic_op correctly
anyway. It will prevent using GL_LOGIC_OP with blend_equation_separate
(see legal_blend_equation function). This directly contradicts the
dependency section of EXT_blend_equation_separate so seems wrong. Though
on the upside it means drivers don't really need to care :-).

Roland