[Mesa-dev] [PATCH 1/3] mesa: fix glPushAttrib(0) / glPopAttrib() error

[Brian Paul]

On 05/26/2015 02:03 PM, Jose Fonseca wrote:
> On 25/05/15 15:41, Brian Paul wrote:
>> If the glPushAttrib() mask value was zero we didn't actually push
>> anything onto the attribute stack.  A subsequent glPopAttrib() call
>> would generate a GL_STACK_UNDERFLOW error.  Now push a dummy attribute
>> in that case to prevent the error.
>>
>> Mesa now matches nvidia's behavior.
>> ---
>>   src/mesa/main/attrib.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index b163c0a..365a79d 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -177,6 +177,10 @@ struct texture_state
>>   };
>>
>>
>> +/** An unused GL_*_BIT value */
>> +#define DUMMY_BIT 0x10000000
>
>
>
>> +
>> +
>>   /**
>>    * Allocate new attribute node of given type/kind.  Attach payload
>> data.
>>    * Insert it into the linked list named by 'head'.
>> @@ -253,6 +257,15 @@ _mesa_PushAttrib(GLbitfield mask)
>>      /* groups specified by the mask. */
>>      head = NULL;
>>
>> +   if (mask == 0) {
>> +      /* if mask is zero we still need to push something so that we
>> +       * don't get a GL_STACK_UNDERFLOW error in glPopAttrib().
>> +       */
>> +      GLuint dummy = 0;
>> +      if (!push_attrib(ctx, &head, DUMMY_BIT, sizeof(dummy), &dummy))
>> +         goto end;
>> +   }
>> +
>>      if (mask & GL_ACCUM_BUFFER_BIT) {
>>         if (!push_attrib(ctx, &head, GL_ACCUM_BUFFER_BIT,
>>                          sizeof(struct gl_accum_attrib),
>> @@ -928,6 +941,10 @@ _mesa_PopAttrib(void)
>>         }
>>
>>         switch (attr->kind) {
>> +         case DUMMY_BIT:
>> +            /* do nothing */
>> +            break;
>> +
>>            case GL_ACCUM_BUFFER_BIT:
>>               {
>>                  const struct gl_accum_attrib *accum;
>>
>
> If I'm not mistaken, you could use 0 for DUMMY_BIT (instead of
> 0x10000000), and therefore completely avoid the possibility of a clash.

Yes, I just thought it was more obvious this way what's going on.


> Either way
>
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

Thanks.

-Brian