<div dir="ltr">On 3 July 2013 21:49, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On 07/03/2013 10:50 AM, Paul Berry wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
driver->ProgramStringNotify is only called for ARB programs, fixed<br>
function vertex programs, and ir_to_mesa (which isn't used by the i965<br>
back-end).  Therefore, even after geometry shaders are added,<br>
brwProgramStringNotify should only ever be called with a target of<br>
GL_VERTEX_PROGRAM_ARB or GL_FRAGMENT_PROGRAM_ARB.<br>
<br>
This patch adds an assertion to clarify that.<br>
---<br>
  src/mesa/drivers/dri/i965/brw_<u></u>program.c | 11 +++++++++--<br>
  1 file changed, 9 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/<u></u>brw_program.c b/src/mesa/drivers/dri/i965/<u></u>brw_program.c<br>
index 87986a9..4c5acb5 100644<br>
--- a/src/mesa/drivers/dri/i965/<u></u>brw_program.c<br>
+++ b/src/mesa/drivers/dri/i965/<u></u>brw_program.c<br>
@@ -126,7 +126,8 @@ brwProgramStringNotify(struct gl_context *ctx,<br>
  {<br>
     struct brw_context *brw = brw_context(ctx);<br>
<br>
-   if (target == GL_FRAGMENT_PROGRAM_ARB) {<br>
+   switch (target) {<br>
+   case GL_FRAGMENT_PROGRAM_ARB: {<br>
        struct gl_fragment_program *fprog = (struct gl_fragment_program *) prog;<br>
        struct brw_fragment_program *newFP = brw_fragment_program(fprog);<br>
        const struct brw_fragment_program *curFP =<br>
@@ -135,8 +136,9 @@ brwProgramStringNotify(struct gl_context *ctx,<br>
        if (newFP == curFP)<br>
         brw->state.dirty.brw |= BRW_NEW_FRAGMENT_PROGRAM;<br>
        newFP->id = get_new_program_id(brw->intel.<u></u>intelScreen);<br>
+      break;<br>
     }<br>
-   else if (target == GL_VERTEX_PROGRAM_ARB) {<br>
+   case GL_VERTEX_PROGRAM_ARB: {<br>
        struct gl_vertex_program *vprog = (struct gl_vertex_program *) prog;<br>
        struct brw_vertex_program *newVP = brw_vertex_program(vprog);<br>
        const struct brw_vertex_program *curVP =<br>
@@ -152,6 +154,11 @@ brwProgramStringNotify(struct gl_context *ctx,<br>
        /* Also tell tnl about it:<br>
         */<br>
        _tnl_program_string(ctx, target, prog);<br>
+      break;<br>
+   }<br>
+   default:<br>
+      assert(!"Unexpected target in brwProgramStringNotify");<br>
+      break;<br>
     }<br>
</blockquote>
<br></div></div>
I'm really glad to see this.  However, I suspect people coming across it will perpetually wonder why the other targets aren't present.<br>
<br>
Perhaps add a block comment below the "default:" containing your commit message?<br></blockquote><div><br></div><div>That's a good idea.  Thanks for the suggestion.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<br>
Either way,<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>><br>
</blockquote></div><br></div></div>