<div dir="ltr"><div>Emil, </div><div>In situations such as your TEXTURE_TARGET example, the functionality is not exposed to non-DSA functions. I've been making the backend functions take a bool dsa that instructs them how to behave depending on whether or not glTexParameter or glTextureParameter is called. Now this is arguably more cumbersome than ctx->Extensions.ARB_direct_state_access, but it works to prevent the user from seeing DSA functionality that would confuse them. Laura </div></div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Jan 23, 2015 at 2:20 PM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 23/01/15 21:53, Jason Ekstrand wrote: > > > On Fri, Jan 23, 2015 at 1:46 PM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a> > <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>>> wrote: > > On 23/01/15 20:51, Jason Ekstrand wrote: > > > > > > On Thu, Jan 22, 2015 at 9:27 AM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a> <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> > > <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a> <div><div class="h5">> <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>>>> 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). > </div></div>There has been other similar cases, yet I cannot pick one from the top of my head. And yes I did understand that is has *nothing* driver specific about it :) > > 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. > See note 3, that I've mentioned above. Here is a rough example: As you handle the following " Accepted by the <pname> parameter of GetTextureParameter{if}v and GetTextureParameterI{i ui}v: TEXTURE_TARGET 0x1006 " you will allow the pname, in a scenario when one should not. I.e. the extension will not be advertised, yet the parameter will be accepted and no error will be thrown. This is a silly example, yet I hope it illustrates the point. > > 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. True, I'm not against that. > 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. The idea behind the flag is to control/distinguish if the extension is advertised _and_ if its functionality is enabled. Presently you're gradually enabling the functionality without advertising the extension. As such there are going to be cases, where you allow/forbid X or Y (as per the spec text), yet the user will be confused, as mesa does not advertise support for arb_dsa. Does this shed more light on what I'm thinking ? -Emil </blockquote></div> </div>