[Mesa-dev] [PATCH] i965: Add an assertion to brwProgramStringNotify.

[Paul Berry]

On 3 July 2013 21:49, Kenneth Graunke <kenneth at whitecape.org> wrote:

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

That's a good idea.  Thanks for the suggestion.


>
> Either way,
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130708/a41c5398/attachment-0001.html>