[Mesa-dev] [PATCH 2/2] meta: Don't botch color masks when changing drawbuffers.
Jason Ekstrand
jason at jlekstrand.net
Tue Apr 12 23:20:59 UTC 2016
On Apr 12, 2016 1:42 PM, "Kenneth Graunke" <kenneth at whitecape.org> 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 <jason at jlekstrand.net>
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: https://bugs.freedesktop.org/show_bug.cgi?id=94847
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
> 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
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160412/3ff1b4a8/attachment.html>
More information about the mesa-dev
mailing list