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

[Nanley Chery]

On Tue, Jun 07, 2016 at 04:45:39PM -0700, 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.
> 

No problem. Thanks for coming up with a solution to improve the patch's
clarity.

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

Agreed. With the clearer commit message, this patch is also
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>

> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> 
> > - 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
> > 
>