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

[Ian Romanick]

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;
> +         }
>        }
>     }
>