[Mesa-dev] [PATCH 01/41] glapi: Added ARB_direct_state_access.xml file.

[Jason Ekstrand]

On Fri, Jan 23, 2015 at 1:46 PM, Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> On 23/01/15 20:51, Jason Ekstrand wrote:
> >
> >
> > On Thu, Jan 22, 2015 at 9:27 AM, Emil Velikov <emil.l.velikov at gmail.com
> > <mailto:emil.l.velikov at gmail.com>> wrote:
> >
> >     On 05/01/15 17:45, Laura Ekstrand wrote:
> >     > This comment is vague.  Do you have a specific recommendation for
> the
> >     > code here?
> >     >
> >     Seems like I'm way too subtle - yes I have a few.
> >
> >
> >     1. Add ARB_direct_state_access to struct gl_extension
> >     --- a/src/mesa/main/mtypes.h
> >     +++ b/src/mesa/main/mtypes.h
> >     @@ -3731,6 +3731,7 @@ struct gl_extensions
> >         GLboolean ARB_depth_clamp;
> >         GLboolean ARB_depth_texture;
> >         GLboolean ARB_derivative_control;
> >     +   GLboolean ARB_direct_state_access
> >         GLboolean ARB_draw_buffers_blend;
> >         GLboolean ARB_draw_elements_base_vertex;
> >
> >
> >     2. Use it in the extensions table.
> >     --- a/src/mesa/main/extensions.c
> >     +++ b/src/mesa/main/extensions.c
> >     @@ -103,6 +103,7 @@ static const struct extension extension_table[]
> = {
> >         { "GL_ARB_depth_clamp",
>  o(ARB_depth_clamp),
> >                             GL,             2003 },
> >         { "GL_ARB_depth_texture",
> >     o(ARB_depth_texture),                       GLL,            2001 },
> >         { "GL_ARB_derivative_control",
> >     o(ARB_derivative_control),                  GL,             2014 },
> >     +   { "GL_ARB_direct_state_access",
> >     o(ARB_direct_state_access),                 GL,             2014 },
> >
> >
> >     3. Make use of if when the spec amends existing behaviour - most of
> the
> >     spec text as of section "New Tokens" onwards. Clearly with this
> series
> >     you're adding the new entry points(functions) so it does not apply
> >     here :)
> >
> >
> >     if (foo->Extensions.ARB_direct_state_access) {
> >      ....
> >     }
> >
> >
> >     Pretty much every extension that was added to mesa follows this
> approach
> >     so keeping up with traditions is always nice.
> >
> >
> > Yes, and no...  We have the table of booleans in gl_extensions so that
> > we can expose different extensions/behavior on different drivers.
> > However, ARB_direct_state_access doesn't actually add new functionality,
> > just new ways of getting at old functionality.  We *should* be able to
> > implement it in a driver-agnostic way entirely within core mesa.
> > Therefore, there's no reason to be able to shut it off on a per-driver
> > basis and no reason for the flag in gl_extensions.  If we find that, for
> > some reason, we only want to support it in core contexts or that it adds
> > something some drivers can't handle it, then we'll need the flag.
> True, yet the usual approach so far had been:
> 1. add the flag
> 2. enable when/where possible
> 3. evaluate if things can be enabled for everyone
> 4. drop it (replace with dummy_true).
> Why bother ? See below.
>

The "usual approach" is for extensions that add functionality and require
per-driver implementation.  This extension is kind of unique in that
*nothing* it adds is per-driver (as far as I know).


> There will be a point where the extension will still be dummy_false, yet
> the amendments to the spec will be applied.
>

What "ammendments to the spec"?  Once it gets implemented, we'll turn it on.


> At that point there will be a "few" reports from your QA team and other
> people, that piglit (other) has regressed. Going the usual route will
> save you that, at the cost of having one extra commit worth
> (presumingly) ~50loc.
>
> Hope with ^^ things make (a bit more) sense :)
>

Not really.  Right now it's not even 100% implemented, so it needs to be
off for everyone.  As far as anyone can tell, it will go directly from
dummy_false to dummy_true.  If we do find something in the way of
implementing it that can't be done on some drivers, we can add the flag and
then turn it on per-driver instead of turning it on for everyone.  I'm
really not seeing how a per-driver flag will do any good.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150123/1b2d1720/attachment.html>