On Apr 12, 2016 1:42 PM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: > > Color clears should respect each drawbuffer's color mask state. > > Previously, we tried to leave the color mask untouched. However, > _mesa_meta_drawbuffers_from_bitfield() ended up rebinding all the > color drawbuffers in a different order, so we ended up pairing > drawbuffers with the wrong color mask state. > > The new _mesa_meta_drawbuffers_and_colormask() function does the > same job as the old _mesa_meta_drawbuffers_from_bitfield(), but > also rearranges the color mask state to match the new drawbuffer > configuration. Arrrrrrggggghhhh.... I don't like this patch either. I really wish we could let meta_begin stomp color masks. Unfortunately, the only way to do that would be to split up the save and re-arrange code across the meta_begin which also seems terrible. I don't know what say. Oh well... Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> Feel free to put a Naked-by on there too if you'd like. > This code was largely ripped off from Gallium's st_Clear code. > > This fixes ES31-CTS.draw_buffers_indexed.color_masks, which binds > up to 8 drawbuffers, sets color masks for each, and then calls > glClearBufferfv to clear each buffer individually. ClearBuffer > causes us to rebind only one drawbuffer, at which point we used > ctx->Color.ColorMask[0] (draw buffer 0's state) for everything. > > We could probably delete _mesa_meta_drawbuffers_from_bitfield(), > but I'd rather not think about the i965 fast clear code. Topi is > rewriting a bunch of that soon anyway, so let's delete it then. > > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=94847">https://bugs.freedesktop.org/show_bug.cgi?id=94847</a> > Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > --- > src/mesa/drivers/common/meta.c | 82 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 75 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c > index eedfb7c..6dcbc8b 100644 > --- a/src/mesa/drivers/common/meta.c > +++ b/src/mesa/drivers/common/meta.c > @@ -1618,6 +1618,79 @@ _mesa_meta_drawbuffers_from_bitfield(GLbitfield bits) > } > > /** > + * Return if all of the color channels are masked. > + */ > +static inline GLboolean > +is_color_disabled(struct gl_context *ctx, int i) > +{ > + return !ctx->Color.ColorMask[i][0] && > + !ctx->Color.ColorMask[i][1] && > + !ctx->Color.ColorMask[i][2] && > + !ctx->Color.ColorMask[i][3]; > +} > + > +/** > + * Given a bitfield of BUFFER_BIT_x draw buffers, call glDrawBuffers to > + * set GL to only draw to those buffers. Also, update color masks to > + * reflect the new draw buffer ordering. > + */ > +static void > +_mesa_meta_drawbuffers_and_colormask(struct gl_context *ctx, GLbitfield mask) > +{ > + GLenum enums[MAX_DRAW_BUFFERS]; > + GLubyte colormask[MAX_DRAW_BUFFERS][4]; > + int num_bufs = 0; > + > + /* This function is only legal for color buffer bitfields. */ > + assert((mask & ~BUFFER_BITS_COLOR) == 0); > + > + /* Make sure we don't overflow any arrays. */ > + assert(_mesa_bitcount(mask) <= MAX_DRAW_BUFFERS); > + > + enums[0] = GL_NONE; > + > + for (int i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) { > + int b = ctx->DrawBuffer->_ColorDrawBufferIndexes[i]; > + int colormask_idx = ctx->Extensions.EXT_draw_buffers2 ? i : 0; > + > + if (b < 0 || !(mask & (1 << b)) || is_color_disabled(ctx, colormask_idx)) > + continue; > + > + switch (b) { > + case BUFFER_FRONT_LEFT: > + enums[num_bufs] = GL_FRONT_LEFT; > + break; > + case BUFFER_FRONT_RIGHT: > + enums[num_bufs] = GL_FRONT_RIGHT; > + break; > + case BUFFER_BACK_LEFT: > + enums[num_bufs] = GL_BACK_LEFT; > + break; > + case BUFFER_BACK_RIGHT: > + enums[num_bufs] = GL_BACK_RIGHT; > + break; > + default: > + assert(b >= BUFFER_COLOR0 && b <= BUFFER_COLOR7); > + enums[num_bufs] = GL_COLOR_ATTACHMENT0 + (b - BUFFER_COLOR0); > + break; > + } > + > + for (int k = 0; k < 4; k++) > + colormask[num_bufs][k] = ctx->Color.ColorMask[colormask_idx][k]; > + > + num_bufs++; > + } > + > + _mesa_DrawBuffers(num_bufs, enums); > + > + for (int i = 0; i < num_bufs; i++) { > + _mesa_ColorMaski(i, colormask[i][0], colormask[i][1], > + colormask[i][2], colormask[i][3]); > + } > +} > + > + > +/** > * Meta implementation of ctx->Driver.Clear() in terms of polygon rendering. > */ > static void > @@ -1633,6 +1706,7 @@ meta_clear(struct gl_context *ctx, GLbitfield buffers, bool glsl) > > metaSave = (MESA_META_ALPHA_TEST | > MESA_META_BLEND | > + MESA_META_COLOR_MASK | > MESA_META_DEPTH_TEST | > MESA_META_RASTERIZATION | > MESA_META_SHADER | > @@ -1655,11 +1729,6 @@ meta_clear(struct gl_context *ctx, GLbitfield buffers, bool glsl) > > if (buffers & BUFFER_BITS_COLOR) { > metaSave |= MESA_META_DRAW_BUFFERS; > - } else { > - /* We'll use colormask to disable color writes. Otherwise, > - * respect color mask > - */ > - metaSave |= MESA_META_COLOR_MASK; > } > > _mesa_meta_begin(ctx, metaSave); > @@ -1695,7 +1764,7 @@ meta_clear(struct gl_context *ctx, GLbitfield buffers, bool glsl) > /* GL_COLOR_BUFFER_BIT */ > if (buffers & BUFFER_BITS_COLOR) { > /* Only draw to the buffers we were asked to clear. */ > - _mesa_meta_drawbuffers_from_bitfield(buffers & BUFFER_BITS_COLOR); > + _mesa_meta_drawbuffers_and_colormask(ctx, buffers & BUFFER_BITS_COLOR); > > /* leave colormask state as-is */ > > @@ -1704,7 +1773,6 @@ meta_clear(struct gl_context *ctx, GLbitfield buffers, bool glsl) > _mesa_ClampColor(GL_CLAMP_FRAGMENT_COLOR, GL_FALSE); > } > else { > - assert(metaSave & MESA_META_COLOR_MASK); > _mesa_ColorMask(GL_FALSE, GL_FALSE, GL_FALSE, GL_FALSE); > } > > -- > 2.8.0 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>