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

[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.

Marek