On 2 November 2012 12:16, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 2 November 2012 09:24, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br></div><div class="gmail_quote"><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On 1 November 2012 21:55, Jordan Justen <span dir="ltr"><<a href="mailto:jljusten@gmail.com" target="_blank">jljusten@gmail.com</a>></span> wrote:<br></div><div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Excellent Paul! I didn't realize you were so close to having this<br>
ready. It makes a good portion of my GL Core dispatch sanity v2 series<br>
unnecessary.<br></blockquote></div></div></blockquote></div><div><br>I don't think it makes your series unnecessary, just...different. For instance, your patch "mesa shaderapi: don't enable various functions for GL CORE" disables three functions for core contexts by adjusting the hand coded function _mesa_init_shader_dispatch(). After my series lands, _mesa_init_shader_dispatch() will only used for the "save" dispatch table, which doesn't apply to core contexts, so it won't be necessary to adjust _mesa_init_shader_dispatch() anymore. Instead, we'll need to adjust the XML so that the generated code will disable these functions appropriately.<br>
<br>I think similar reasoning applies to most (all?) of the other patches in your series.<br><br>As an experiment, I tried rebasing your patch series on top of my work-in-progress branch, taking these kinds of considerations into account. It's on branch "gl-core-sanity" of git://<a href="http://github.com/stereotype441/mesa.git" target="_blank">github.com/stereotype441/mesa.git</a> if you want to look at it.<br>
<br></div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Anyway, I rebased my 5 dispatch-sanity changes onto your branch, and<br>
it reported that these functions should be nop in GL Core profiles:<br>
* ActiveProgramEXT<br>
* CreateShaderProgramEXT<br>
* UseShaderProgramEXT<br>
* StencilFuncSeparateATI<br>
<br>
The first 3 were changed in my recent v2 03/13 patch.<br></blockquote></div></div></blockquote></div><div><br>Yeah, adjusting the XML to reproduce the effect of your patch fixed those three. As for StencilFuncSeparateATI, I screwed it up in commit a21116f, which has already landed in master. I'll put out a patch to fix that ASAP.<br>
</div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Anyway, I pushed my rebased changes to:<br>
git://<a href="http://people.freedesktop.org/~jljusten/mesa" target="_blank">people.freedesktop.org/~jljusten/mesa</a><br>
branch=code-gen-api-exec+gl-core-sanity<br>
<br>
Those 5 changes haven't been fully code reviewed yet, but I think they<br>
are reasonably close to ready. What would you think about adding them<br>
to the end of your series?<br></blockquote></div></div></blockquote><br></div><div>Given the fact that your series is reasonably close to ready, I would prefer to let it land first, then adjust mine to incorporate your changes into the XML. Is that ok with you? I'll try to review your series by the end of the day.<br>
</div></div>
</blockquote></div><br>I suppose I should add that part of my motivation for wanting your patch series to land first is that your series introduces additional testing and mine does a lot of refactoring. It seems safer to add the test first and then do the refactor, so that it's easy to confirm that the code passes the test both before and after the refactor.<br>