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 <brianp@vmware.com>
tags/mesa-10.1-rc1
| @@ -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; | |||
| } | |||
| @@ -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 | |||
| @@ -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); | |||
| @@ -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 */ | |||
| @@ -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); | |||
| @@ -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); | |||
| } | |||
| } | |||
| } | |||