[Mesa-dev] [PATCH] mesa: expose ARB_compute_shader with compat profile

[Samuel Pitoiset]

On 06/08/2016 01:45 AM, Ian Romanick wrote:
> On 06/07/2016 04:28 PM, Nanley Chery wrote:
>> On Wed, May 25, 2016 at 12:52:38PM -0700, Matt Turner wrote:
>>> On Wed, May 25, 2016 at 12:17 PM, Samuel Pitoiset
>>> <samuel.pitoiset at gmail.com> wrote:
>>>> This fixes use of compute shaders with some NVIDIA GL 4.3 samples which
>>>> require a compat profile.
>>>
>>> I'm confused.
>>>
>>> The samples you're talking about... use 4.3 compatibility? We already
>>> don't support compatibility profile, so you presumably have to hack
>>> that on.
>>>
>>> The ARB_compute_shader extension spec says it requires 4.2, and we
>>> don't expose 4.2 with the compatibility profile. I think you should
>>> have some justification for why it's safe to expose in compatibility
>>> profile, since that's a pretty big change. Or, you don't want to
>>> expose it in compatibility but you need this patch to enable it with
>>> the 4.2-compat hack enabled...?
>>>
>>> Strangely, ARB_compute_shader says "GLL, GLC" in extensions_table.h,
>>> but it's only core-enabled on my glxinfo.
>>
>> Because extensions_table.h has "GLL" in the ARB_compute_shader entry,
>> the extension is already exposed in all compatibility contexts. This
>> patch fixes the bug of advertising the extension externally, but
>> internally failing to do the logic required by the extension.
>
> Thanks for clearing that up, Nanley.  That makes sense.  I believe the
> commit message is misleading, and that led to some confusion.  I think a
> better commit message is:
>
>     mesa: Let compute shaders work in compatibility profiles
>
>     The extension is already advertised in compatibility profile, but
>     the _mesa_has_compute_shaders only returns true in core profile.
>     If we advertise it, we should allow it to work.
>
> Now, whether or not we should advertise the extension in compatibility
> profiles is an orthogonal question.  We discussed this when compute
> shaders first landed, and we decided to enable it.  I don't know of a
> strong reason to change that, but I'm not married to it.
>
> Either way, with the above change to the commit message, this patch is
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

Thanks for the clarification, and you commit message is much better than 
mine. :)

I will push the patch.

>
>> - Nanley
>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>