st/mesa: bind NULL colorbuffers as specified by glDrawBuffers

An example why it is required:

    Let's say there's a fragment shader writing to gl_FragData[0..1].
    The user calls: glDrawBuffers(2, {GL_NONE, GL_COLOR_ATTACHMENT0});

    That means gl_FragData[0] is unused and gl_FragData[1] is written
    to GL_COLOR_ATTACHMENT0.

st/mesa was skipping the GL_NONE draw buffer, therefore gl_FragData[0]
was written to GL_COLOR_ATTACHMENT0, which was wrong.

This commit fixes it, but drivers must also be fixed not to crash when
binding NULL colorbuffers. There is also a new set of piglit tests for this.

The MSAA state also had to be fixed not to crash when reading fb->cbufs[0].

Reviewed-by: Brian Paul <[email protected]>

6 files changed, 40 insertions, 14 deletions

diff --git a/src/gallium/auxiliary/util/u_framebuffer.c b/src/gallium/auxiliary/util/u_framebuffer.c
index 683967237e9..377b802b62a 100644
--- a/src/gallium/auxiliary/util/u_framebuffer.c
+++ b/src/gallium/auxiliary/util/u_framebuffer.c
@@ -171,3 +171,24 @@ util_framebuffer_get_num_layers(const struct pipe_framebuffer_state *fb)
 	}
 	return num_layers;
 }
+
+
+/**
+ * Return the number of MSAA samples.
+ */
+unsigned
+util_framebuffer_get_num_samples(const struct pipe_framebuffer_state *fb)
+{
+   unsigned i;
+
+   for (i = 0; i < fb->nr_cbufs; i++) {
+      if (fb->cbufs[i]) {
+         return MAX2(1, fb->cbufs[i]->texture->nr_samples);
+      }
+   }
+   if (fb->zsbuf) {
+      return MAX2(1, fb->zsbuf->texture->nr_samples);
+   }
+
+   return 1;
+}
diff --git a/src/gallium/auxiliary/util/u_framebuffer.h b/src/gallium/auxiliary/util/u_framebuffer.h
index 0e6c98363a8..c73942c9c14 100644
--- a/src/gallium/auxiliary/util/u_framebuffer.h
+++ b/src/gallium/auxiliary/util/u_framebuffer.h
@@ -60,6 +60,10 @@ extern unsigned
 util_framebuffer_get_num_layers(const struct pipe_framebuffer_state *fb);
 
 
+extern unsigned
+util_framebuffer_get_num_samples(const struct pipe_framebuffer_state *fb);
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c b/src/mesa/state_tracker/st_atom_framebuffer.c
index 51f079cdad5..a77fe54ae12 100644
--- a/src/mesa/state_tracker/st_atom_framebuffer.c
+++ b/src/mesa/state_tracker/st_atom_framebuffer.c
@@ -65,12 +65,14 @@ update_framebuffer_state( struct st_context *st )
    /* Examine Mesa's ctx->DrawBuffer->_ColorDrawBuffers state
     * to determine which surfaces to draw to
     */
-   framebuffer->nr_cbufs = 0;
+   framebuffer->nr_cbufs = fb->_NumColorDrawBuffers;
+
    for (i = 0; i < fb->_NumColorDrawBuffers; i++) {
+      pipe_surface_reference(&framebuffer->cbufs[i], NULL);
+
       strb = st_renderbuffer(fb->_ColorDrawBuffers[i]);
 
       if (strb) {
-         /*printf("--------- framebuffer surface rtt %p\n", strb->rtt);*/
          if (strb->is_rtt ||
              (strb->texture && util_format_is_srgb(strb->texture->format))) {
             /* rendering to a GL texture, may have to update surface */
@@ -78,13 +80,12 @@ update_framebuffer_state( struct st_context *st )
          }
 
          if (strb->surface) {
-            pipe_surface_reference(&framebuffer->cbufs[framebuffer->nr_cbufs],
-                                   strb->surface);
-            framebuffer->nr_cbufs++;
+            pipe_surface_reference(&framebuffer->cbufs[i], strb->surface);
          }
          strb->defined = GL_TRUE; /* we'll be drawing something */
       }
    }
+
    for (i = framebuffer->nr_cbufs; i < PIPE_MAX_COLOR_BUFS; i++) {
       pipe_surface_reference(&framebuffer->cbufs[i], NULL);
    }
@@ -113,7 +114,8 @@ update_framebuffer_state( struct st_context *st )
 #ifdef DEBUG
    /* Make sure the resource binding flags were set properly */
    for (i = 0; i < framebuffer->nr_cbufs; i++) {
-      assert(framebuffer->cbufs[i]->texture->bind & PIPE_BIND_RENDER_TARGET);
+      assert(!framebuffer->cbufs[i] ||
+             framebuffer->cbufs[i]->texture->bind & PIPE_BIND_RENDER_TARGET);
    }
    if (framebuffer->zsbuf) {
       assert(framebuffer->zsbuf->texture->bind & PIPE_BIND_DEPTH_STENCIL);
diff --git a/src/mesa/state_tracker/st_atom_msaa.c b/src/mesa/state_tracker/st_atom_msaa.c
index fb760460c57..2f3a42e0248 100644
--- a/src/mesa/state_tracker/st_atom_msaa.c
+++ b/src/mesa/state_tracker/st_atom_msaa.c
@@ -31,6 +31,7 @@
 #include "st_atom.h"
 
 #include "cso_cache/cso_context.h"
+#include "util/u_framebuffer.h"
 
 
 /* Second state atom for user clip planes:
@@ -38,14 +39,9 @@
 static void update_sample_mask( struct st_context *st )
 {
    unsigned sample_mask = 0xffffffff;
-   unsigned sample_count = 1;
    struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
-
    /* dependency here on bound surface (or rather, sample count) is worrying */
-   if (framebuffer->zsbuf)
-      sample_count = framebuffer->zsbuf->texture->nr_samples;
-   else if (framebuffer->cbufs[0])
-      sample_count = framebuffer->cbufs[0]->texture->nr_samples;
+   unsigned sample_count = util_framebuffer_get_num_samples(framebuffer);
 
    if (st->ctx->Multisample.Enabled && sample_count > 1) {
    /* unlike in gallium/d3d10 the mask is only active if msaa is enabled */
diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
index a84c9075f5e..87dccee1f49 100644
--- a/src/mesa/state_tracker/st_cb_clear.c
+++ b/src/mesa/state_tracker/st_cb_clear.c
@@ -446,7 +446,7 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
       for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
          GLuint b = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
 
-         if (mask & (1 << b)) {
+         if (b >= 0 && mask & (1 << b)) {
             struct gl_renderbuffer *rb
                = ctx->DrawBuffer->Attachment[b].Renderbuffer;
             struct st_renderbuffer *strb = st_renderbuffer(rb);
diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
index 70baa9965de..daa94df92b8 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -680,7 +680,10 @@ st_DrawBuffers(struct gl_context *ctx, GLsizei count, const GLenum *buffers)
    /* add the renderbuffers on demand */
    for (i = 0; i < fb->_NumColorDrawBuffers; i++) {
       gl_buffer_index idx = fb->_ColorDrawBufferIndexes[i];
-      st_manager_add_color_renderbuffer(st, fb, idx);
+
+      if (idx >= 0) {
+         st_manager_add_color_renderbuffer(st, fb, idx);
+      }
    }
 }