[Mesa-dev] [PATCH 1/2] st/mesa: adjust blending modes if we don't have destination alpha

[Marek Olšák]

I can't say I like this. Ever since Radeon - Northern Islands, our
hardware can do this DST_ALPHA->ONE adjustment automatically, so the
new state dependency on _NEW_BUFFERS is unnecessary for us here.

Marek

On Wed, Apr 29, 2015 at 2:19 AM, Brian Paul <brianp at vmware.com> wrote:
> If the user requested a GL_RGB texture but the driver actually allocated
> an RGBA texture, the alpha values in the texture may not be defined.
>
> If we later bind the texture as a color target and try to blend into
> it with GL_DST_ALPHA or GL_ONE_MINUS_DST_ALPHA we may blend with
> undefined alpha values when, in fact, the dest alpha value should be one.
> So replace GL_DST_ALPHA/GL_ONE_MINUS_DST_ALPHA with GL_ONE/GL_ZERO.
>
> Fixes the piglit fbo-blending-formats test for some GL_RGB formats
> with the VMware driver.  Also tested with llvmpipe.
>
> v2: use the i-th (not j-th) render buffer's base format
> ---
>  src/mesa/state_tracker/st_atom_blend.c | 38 +++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_blend.c b/src/mesa/state_tracker/st_atom_blend.c
> index 6bb4077..6337e1c 100644
> --- a/src/mesa/state_tracker/st_atom_blend.c
> +++ b/src/mesa/state_tracker/st_atom_blend.c
> @@ -44,10 +44,21 @@
>  /**
>   * Convert GLenum blend tokens to pipe tokens.
>   * Both blend factors and blend funcs are accepted.
> + * \param destBaseFormat  the base format of the render target, such as
> + *                        GL_RGBA, GL_RGB, GL_RED, GL_ALPHA, etc.
>   */
>  static GLuint
> -translate_blend(GLenum blend)
> +translate_blend(GLenum blend, GLenum destBaseFormat)
>  {
> +   /* If we don't have destination alpha and the blend factor is either
> +    * GL_DST_ALPHA or GL_ONE_MINUS_DST_ALPHA then we use
> +    * PIPE_BLENDFACTOR_ONE or _ZERO instead.
> +    */
> +   const bool haveDstA = (destBaseFormat == GL_RGBA ||
> +                          destBaseFormat == GL_ALPHA ||
> +                          destBaseFormat == GL_INTENSITY ||
> +                          destBaseFormat == GL_LUMINANCE_ALPHA);
> +
>     switch (blend) {
>     /* blend functions */
>     case GL_FUNC_ADD:
> @@ -69,7 +80,7 @@ translate_blend(GLenum blend)
>     case GL_SRC_ALPHA:
>        return PIPE_BLENDFACTOR_SRC_ALPHA;
>     case GL_DST_ALPHA:
> -      return PIPE_BLENDFACTOR_DST_ALPHA;
> +      return haveDstA ? PIPE_BLENDFACTOR_DST_ALPHA : PIPE_BLENDFACTOR_ONE;
>     case GL_DST_COLOR:
>        return PIPE_BLENDFACTOR_DST_COLOR;
>     case GL_SRC_ALPHA_SATURATE:
> @@ -91,7 +102,7 @@ translate_blend(GLenum blend)
>     case GL_ONE_MINUS_DST_COLOR:
>        return PIPE_BLENDFACTOR_INV_DST_COLOR;
>     case GL_ONE_MINUS_DST_ALPHA:
> -      return PIPE_BLENDFACTOR_INV_DST_ALPHA;
> +      return haveDstA ? PIPE_BLENDFACTOR_INV_DST_ALPHA : PIPE_BLENDFACTOR_ZERO;
>     case GL_ONE_MINUS_CONSTANT_COLOR:
>        return PIPE_BLENDFACTOR_INV_CONST_COLOR;
>     case GL_ONE_MINUS_CONSTANT_ALPHA:
> @@ -208,14 +219,21 @@ update_blend( struct st_context *st )
>     else if (ctx->Color.BlendEnabled) {
>        /* blending enabled */
>        for (i = 0, j = 0; i < num_state; i++) {
> +         const struct gl_renderbuffer *rb;
> +         GLenum baseFormat;
>
>           blend->rt[i].blend_enable = (ctx->Color.BlendEnabled >> i) & 0x1;
>
>           if (ctx->Extensions.ARB_draw_buffers_blend)
>              j = i;
>
> +         /* _NEW_BUFFERS */
> +         /* Get the base format of the i-th render target */
> +         rb = ctx->DrawBuffer->_ColorDrawBuffers[i];
> +         baseFormat = rb ? rb->_BaseFormat : GL_RGBA;
> +
>           blend->rt[i].rgb_func =
> -            translate_blend(ctx->Color.Blend[j].EquationRGB);
> +            translate_blend(ctx->Color.Blend[j].EquationRGB, baseFormat);
>
>           if (ctx->Color.Blend[i].EquationRGB == GL_MIN ||
>               ctx->Color.Blend[i].EquationRGB == GL_MAX) {
> @@ -225,13 +243,13 @@ update_blend( struct st_context *st )
>           }
>           else {
>              blend->rt[i].rgb_src_factor =
> -               translate_blend(ctx->Color.Blend[j].SrcRGB);
> +               translate_blend(ctx->Color.Blend[j].SrcRGB, baseFormat);
>              blend->rt[i].rgb_dst_factor =
> -               translate_blend(ctx->Color.Blend[j].DstRGB);
> +               translate_blend(ctx->Color.Blend[j].DstRGB, baseFormat);
>           }
>
>           blend->rt[i].alpha_func =
> -            translate_blend(ctx->Color.Blend[j].EquationA);
> +            translate_blend(ctx->Color.Blend[j].EquationA, baseFormat);
>
>           if (ctx->Color.Blend[i].EquationA == GL_MIN ||
>               ctx->Color.Blend[i].EquationA == GL_MAX) {
> @@ -241,9 +259,9 @@ update_blend( struct st_context *st )
>           }
>           else {
>              blend->rt[i].alpha_src_factor =
> -               translate_blend(ctx->Color.Blend[j].SrcA);
> +               translate_blend(ctx->Color.Blend[j].SrcA, baseFormat);
>              blend->rt[i].alpha_dst_factor =
> -               translate_blend(ctx->Color.Blend[j].DstA);
> +               translate_blend(ctx->Color.Blend[j].DstA, baseFormat);
>           }
>        }
>     }
> @@ -285,7 +303,7 @@ update_blend( struct st_context *st )
>  const struct st_tracked_state st_update_blend = {
>     "st_update_blend",                                  /* name */
>     {                                                   /* dirty */
> -      (_NEW_COLOR | _NEW_MULTISAMPLE),  /* XXX _NEW_BLEND someday? */  /* mesa */
> +      (_NEW_BUFFERS | _NEW_COLOR | _NEW_MULTISAMPLE),          /* mesa */
>        0,                                               /* st */
>     },
>     update_blend,                                       /* update */
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev