[Mesa-dev] [PATCH 2/7] st/mesa: bind NULL colorbuffers as specified by glDrawBuffers
Brian Paul
brianp at vmware.com
Wed Jan 8 11:21:06 PST 2014
On 01/08/2014 10:23 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> 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.
> ---
> src/gallium/auxiliary/util/u_framebuffer.c | 21 +++++++++++++++++++++
> src/gallium/auxiliary/util/u_framebuffer.h | 4 ++++
> src/mesa/state_tracker/st_atom_framebuffer.c | 14 ++++++++------
> src/mesa/state_tracker/st_atom_msaa.c | 8 ++------
> src/mesa/state_tracker/st_cb_clear.c | 2 +-
> src/mesa/state_tracker/st_cb_fbo.c | 5 ++++-
It looks like there's some MSAA changes mixed in here. AFAICT, that's a
separate piece of work.
-Brian
> 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 6839672..377b802 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 0e6c983..c73942c 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 51f079c..a77fe54 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 fb76046..2f3a42e 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 a84c907..87dccee 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 70baa99..daa94df 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);
> + }
> }
> }
>
>
More information about the mesa-dev
mailing list