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

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/08/2011 12:18 PM, Roland Scheidegger wrote:
> Am 08.09.2011 21:03, schrieb Roland Scheidegger:
>> 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.
> 
> 
> Hmm actually a quick search of ARB_color_imaging reveals that 
> EXT_blend_logic_op isn't even part of it (not that it matters much
> as we don't support the imaging subset any longer anyway) so I
> don't know why it got enabled there. In any case since that imaging
> subset enable is gone, it's not really a problem anymore and
> drivers can just enable it if they really want - r200 certainly
> does not.

Okay.  I'll disable it in r200 and r300.  With that change, do I have
your Reviewed-by?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5pGscACgkQX1gOwKyEAw93RwCgiu/i79FyervNFtqdKxJC4NR5
N44An0e1KrPouqyjA/ZnecKGkxMw+8AE
=ds06
-----END PGP SIGNATURE-----