On 23/01/15 20:51, Jason Ekstrand wrote: > > > On Thu, Jan 22, 2015 at 9:27 AM, Emil Velikov <emil.l.veli...@gmail.com > <mailto:emil.l.veli...@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.
There will be a point where the extension will still be dummy_false, yet the amendments to the spec will be applied. 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 :) Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
