<div dir="ltr"><div>Emil,<br><br></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.<br><br>Laura<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 23, 2015 at 2:20 PM, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 23/01/15 21:53, Jason Ekstrand wrote:<br>
><br>
><br>
> 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><br>
</span><span class="">> <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>>> wrote:<br>
><br>
> On 23/01/15 20:51, Jason Ekstrand wrote:<br>
> ><br>
> ><br>
> > 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>><br>
</span>> > <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a><br>
<div><div class="h5">> <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>>>> wrote:<br>
> ><br>
> > On 05/01/15 17:45, Laura Ekstrand wrote:<br>
> > > This comment is vague. Do you have a specific<br>
> recommendation for the<br>
> > > code here?<br>
> > ><br>
> > Seems like I'm way too subtle - yes I have a few.<br>
> ><br>
> ><br>
> > 1. Add ARB_direct_state_access to struct gl_extension<br>
> > --- a/src/mesa/main/mtypes.h<br>
> > +++ b/src/mesa/main/mtypes.h<br>
> > @@ -3731,6 +3731,7 @@ struct gl_extensions<br>
> > GLboolean ARB_depth_clamp;<br>
> > GLboolean ARB_depth_texture;<br>
> > GLboolean ARB_derivative_control;<br>
> > + GLboolean ARB_direct_state_access<br>
> > GLboolean ARB_draw_buffers_blend;<br>
> > GLboolean ARB_draw_elements_base_vertex;<br>
> ><br>
> ><br>
> > 2. Use it in the extensions table.<br>
> > --- a/src/mesa/main/extensions.c<br>
> > +++ b/src/mesa/main/extensions.c<br>
> > @@ -103,6 +103,7 @@ static const struct extension<br>
> extension_table[] = {<br>
> > { "GL_ARB_depth_clamp",<br>
> o(ARB_depth_clamp),<br>
> > GL, 2003 },<br>
> > { "GL_ARB_depth_texture",<br>
> > o(ARB_depth_texture), GLL,<br>
> 2001 },<br>
> > { "GL_ARB_derivative_control",<br>
> > o(ARB_derivative_control), GL,<br>
> 2014 },<br>
> > + { "GL_ARB_direct_state_access",<br>
> > o(ARB_direct_state_access), GL,<br>
> 2014 },<br>
> ><br>
> ><br>
> > 3. Make use of if when the spec amends existing behaviour -<br>
> most of the<br>
> > spec text as of section "New Tokens" onwards. Clearly with<br>
> this series<br>
> > you're adding the new entry points(functions) so it does not apply<br>
> > here :)<br>
> ><br>
> ><br>
> > if (foo->Extensions.ARB_direct_state_access) {<br>
> > ....<br>
> > }<br>
> ><br>
> ><br>
> > Pretty much every extension that was added to mesa follows<br>
> this approach<br>
> > so keeping up with traditions is always nice.<br>
> ><br>
> ><br>
> > Yes, and no... We have the table of booleans in gl_extensions so that<br>
> > we can expose different extensions/behavior on different drivers.<br>
> > However, ARB_direct_state_access doesn't actually add new<br>
> functionality,<br>
> > just new ways of getting at old functionality. We *should* be able to<br>
> > implement it in a driver-agnostic way entirely within core mesa.<br>
> > Therefore, there's no reason to be able to shut it off on a per-driver<br>
> > basis and no reason for the flag in gl_extensions. If we find<br>
> that, for<br>
> > some reason, we only want to support it in core contexts or that<br>
> it adds<br>
> > something some drivers can't handle it, then we'll need the flag.<br>
> True, yet the usual approach so far had been:<br>
> 1. add the flag<br>
> 2. enable when/where possible<br>
> 3. evaluate if things can be enabled for everyone<br>
> 4. drop it (replace with dummy_true).<br>
> Why bother ? See below.<br>
><br>
><br>
> The "usual approach" is for extensions that add functionality and<br>
> require per-driver implementation. This extension is kind of unique in<br>
> that *nothing* it adds is per-driver (as far as I know).<br>
><br>
</div></div>There has been other similar cases, yet I cannot pick one from the top<br>
of my head. And yes I did understand that is has *nothing* driver<br>
specific about it :)<br>
<span class=""><br>
><br>
> There will be a point where the extension will still be dummy_false, yet<br>
> the amendments to the spec will be applied.<br>
><br>
><br>
> What "ammendments to the spec"? Once it gets implemented, we'll turn it on.<br>
><br>
</span>See note 3, that I've mentioned above. Here is a rough example:<br>
<br>
As you handle the following<br>
"<br>
Accepted by the <pname> parameter of GetTextureParameter{if}v and<br>
GetTextureParameterI{i ui}v:<br>
<br>
TEXTURE_TARGET 0x1006<br>
"<br>
<br>
you will allow the pname, in a scenario when one should not.<br>
I.e. the extension will not be advertised, yet the parameter will be<br>
accepted and no error will be thrown.<br>
<br>
This is a silly example, yet I hope it illustrates the point.<br>
<span class=""><br>
><br>
> At that point there will be a "few" reports from your QA team and other<br>
> people, that piglit (other) has regressed. Going the usual route will<br>
> save you that, at the cost of having one extra commit worth<br>
> (presumingly) ~50loc.<br>
><br>
> Hope with ^^ things make (a bit more) sense :)<br>
><br>
><br>
> Not really. Right now it's not even 100% implemented, so it needs to be<br>
> off for everyone.<br>
</span>True, I'm not against that.<br>
<span class=""><br>
> As far as anyone can tell, it will go directly from<br>
> dummy_false to dummy_true. If we do find something in the way of<br>
> implementing it that can't be done on some drivers, we can add the flag<br>
> and then turn it on per-driver instead of turning it on for everyone.<br>
> I'm really not seeing how a per-driver flag will do any good.<br>
</span>The idea behind the flag is to control/distinguish if the extension is<br>
advertised _and_ if its functionality is enabled.<br>
<br>
Presently you're gradually enabling the functionality without<br>
advertising the extension. As such there are going to be cases, where<br>
you allow/forbid X or Y (as per the spec text), yet the user will be<br>
confused, as mesa does not advertise support for arb_dsa.<br>
<br>
Does this shed more light on what I'm thinking ?<br>
<span class="HOEnZb"><font color="#888888"><br>
-Emil<br>
</font></span></blockquote></div><br></div>