[Mesa-dev] [PATCH 2/6] mesa: fix interpretation of glClearBuffer(drawbuffer)

[Marek Olšák]

The spec is cleaned up in GL 4.4 Compatibility (this is the only one I
read except GL 3.0). It doesn't mention FRONT, BACK, etc. Instead it
just says that DRAW_BUFFERi can identify one or more colorbuffers and
all of them are cleared, therefore no clarification is needed for
readers of GL 4.4. Some clarification may be needed for readers of
older specs indeed.

Marek

On Sat, Dec 7, 2013 at 12:12 AM, Ian Romanick <idr at freedesktop.org> wrote:
> On 12/05/2013 09:52 AM, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> This corresponding piglit tests supported this incorrect behavior instead of
>> pointing at it.
>>
>> See the GL 4.4 spec if the GL 3.0 spec is not clear enough.
>
> I looked at the 4.0 spec, and I think I see where the initial confusion
> was.  The spec said:
>
>     If buffer is COLOR, a particular draw buffer DRAW_BUFFERi is
>     specified by passing i as the parameter drawbuffer, and value
>     points to a four-element vector specifying the R, G, B, and A
>     color to clear that draw buffer to. If the draw buffer is one
>     of FRONT, BACK, LEFT, RIGHT, or FRONT_AND_BACK, identifying
>     multiple buffers, each selected buffer is cleared to the same
>     value.
>
> "drawbuffer" and "draw buffer" having two different meanings in two
> sentences in the same paragraph is... suck.
>
> Could we get the above text and some clarification of "drawbuffer" vs.
> "draw buffer" as a comment in make_color_buffer_mask?  With that change,
> this patch is
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
>> Cc: 10.0 9.2 9.1 <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/mesa/main/clear.c | 17 +++++++++++------
>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/main/clear.c b/src/mesa/main/clear.c
>> index 304d135..4239240 100644
>> --- a/src/mesa/main/clear.c
>> +++ b/src/mesa/main/clear.c
>> @@ -219,7 +219,11 @@ make_color_buffer_mask(struct gl_context *ctx, GLint drawbuffer)
>>     const struct gl_renderbuffer_attachment *att = ctx->DrawBuffer->Attachment;
>>     GLbitfield mask = 0x0;
>>
>> -   switch (drawbuffer) {
>> +   if (drawbuffer < 0 || drawbuffer >= (GLint)ctx->Const.MaxDrawBuffers) {
>> +      return INVALID_MASK;
>> +   }
>> +
>> +   switch (ctx->DrawBuffer->ColorDrawBuffer[drawbuffer]) {
>>     case GL_FRONT:
>>        if (att[BUFFER_FRONT_LEFT].Renderbuffer)
>>           mask |= BUFFER_BIT_FRONT_LEFT;
>> @@ -255,11 +259,12 @@ make_color_buffer_mask(struct gl_context *ctx, GLint drawbuffer)
>>           mask |= BUFFER_BIT_BACK_RIGHT;
>>        break;
>>     default:
>> -      if (drawbuffer < 0 || drawbuffer >= (GLint)ctx->Const.MaxDrawBuffers) {
>> -         mask = INVALID_MASK;
>> -      }
>> -      else if (att[BUFFER_COLOR0 + drawbuffer].Renderbuffer) {
>> -         mask |= (BUFFER_BIT_COLOR0 << drawbuffer);
>> +      {
>> +         GLuint buf = ctx->DrawBuffer->_ColorDrawBufferIndexes[drawbuffer];
>> +
>> +         if (buf >= 0 && att[buf].Renderbuffer) {
>> +            mask |= 1 << buf;
>> +         }
>>        }
>>     }
>>
>