From: Marek Olšák Date: Wed, 8 Jan 2014 00:09:15 +0000 (+0100) Subject: st/mesa: bind NULL colorbuffers as specified by glDrawBuffers X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=9baa45f78b8ca7d66280e36009b6a685055d7cd6;p=mesa.git 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 --- 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); + } } }