I think there are two different things here: one is the driver internally fakes BGRX with BGRA, and obviously it's the pipe driver that needs to fix up alpha channel blending to simulate it's one.

The other is the state tracker is faking BGRX with BGRA, and in that case, it's the state tracker that needs to do the fix up (as the pipe has no way to know if that's intentional or not).

Or are you saying that state trackers should never attempt to fulfill BGRX formats with BGRA, and that every driver needs to workaround this internally?

I don't fill too strongly about this, but it wouldn't sound consistent. Nowadays have a pipe cap for every single little thing out there. I don't see a reason to treat render target formats differently. Otherwise something that can be easily done once in the state tracker now needs to be replicated in every driver.


But you have a point about _NEW_BUFFERS. It could be avoided. We could introduce a new flag _NEW_BUFFERS_EMULATED, set when fbo's with emulated formats (e.g., backing BGRX with BGRA), so that drivers that support (or workaround) these things internally don't . On the other hand, _NEW_BUFFERS probably doesn't change that frequently, blend state probably changes much more often.


Jose


On 29/04/15 11:00, Marek Olšák wrote:
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 <bri...@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@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=M_k-NmK2vLhRMd22cxU3O0uEvSVQTY2Xka_BlVDLl9k&s=VQRn_ViicoFTo-Y5ISP66gJ-tE1eFmROcUxUaLudRxg&e=
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=M_k-NmK2vLhRMd22cxU3O0uEvSVQTY2Xka_BlVDLl9k&s=VQRn_ViicoFTo-Y5ISP66gJ-tE1eFmROcUxUaLudRxg&e=


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to