[Mesa-dev] [PATCH 1/2] st/mesa: Handle disabled draw buffers in st_Clear().

[Marek Olšák]

This looks good, but it's only papering over the real problem, which
is that st/mesa doesn't bind NULL colorbuffers and skips them instead.
For example, if DRAWBUFFER0 is NONE, st/mesa binds DRAWBUFFER1 to
cb[0], but then all writes to gl_FragData[1] are broken, because the
draw buffer has been moved to cb[0] and the shader doesn't know about
it.

I think st/mesa should bind NULL colorbuffers and drivers should check
for NULL colorbuffers and disable the writes accordingly. I think most
drivers don't check for NULL colorbuffers, but at least fixing r600g
and radeonsi should be very easy by just looking for NULL pointer
dereferences and disabling colorbuffers by setting
CB_COLORi_INFO.FORMAT=COLOR_INVALID.

Marek

On Wed, Dec 25, 2013 at 12:41 PM, Henri Verbeet <hverbeet at gmail.com> wrote:
> This fixes piglit arb_draw_buffers-rt0_none on AMD CEDAR. No piglit
> regressions on the same hardware.
>
> Signed-off-by: Henri Verbeet <hverbeet at gmail.com>
> ---
>  src/mesa/state_tracker/st_cb_clear.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
> index 887e58b..e7345a2 100644
> --- a/src/mesa/state_tracker/st_cb_clear.c
> +++ b/src/mesa/state_tracker/st_cb_clear.c
> @@ -430,14 +430,20 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
>     st_validate_state( st );
>
>     if (mask & BUFFER_BITS_COLOR) {
> +      unsigned int color_idx = ~0u;
> +
>        for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
> -         GLuint b = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
> +         gl_buffer_index b = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
> +
> +         if (b == -1)
> +            continue;
>
> +         ++color_idx;
>           if (mask & (1 << b)) {
>              struct gl_renderbuffer *rb
>                 = ctx->DrawBuffer->Attachment[b].Renderbuffer;
>              struct st_renderbuffer *strb = st_renderbuffer(rb);
> -            int colormask_index = ctx->Extensions.EXT_draw_buffers2 ? i : 0;
> +            int colormask_index = ctx->Extensions.EXT_draw_buffers2 ? color_idx : 0;
>
>              if (!strb || !strb->surface)
>                 continue;
> @@ -447,9 +453,9 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
>
>              if (is_scissor_enabled(ctx, rb) ||
>                  is_color_masked(ctx, colormask_index))
> -               quad_buffers |= PIPE_CLEAR_COLOR0 << i;
> +               quad_buffers |= PIPE_CLEAR_COLOR0 << color_idx;
>              else
> -               clear_buffers |= PIPE_CLEAR_COLOR0 << i;
> +               clear_buffers |= PIPE_CLEAR_COLOR0 << color_idx;
>           }
>        }
>     }
> --
> 1.7.10.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev