[Mesa-dev] [PATCH 1/2] llvmpipe: fix blending with SRC_ALPHA_SATURATE with some formats without alpha

[sroland at vmware.com]

From: Roland Scheidegger <sroland at vmware.com>

We were fixing up the blend factor to ZERO, however this only works correctly
with fixed point render buffers where the input values are clamped to 0/1
(because src_alpha_saturate is min(As, 1-Ad) so can be negative with unclamped
inputs). Haven't seen any failure anywhere due to that with fixed point SNORM
buffers (which clamp inputs to -1/1) but it should apply there as well (snorm
blending is rare, even opengl 4.3 doesn't require snorm rendertargets at all,
d3d10 requires them but they are not blendable).
Doesn't look like piglit hits this though (some internal testing hits the
float case at least). (With legacy OpenGL we could theoretically still use the
fixup to zero if the fragment color clamp is enabled, but we can't detect that
easily since we don't support native clamping hence it gets baked into the
shader.)
---
 src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c |   18 ++++++++++++++----
 src/gallium/drivers/llvmpipe/lp_state_fs.c      |   16 ++++++++++++----
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c b/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
index c4d04a2..377eaa5 100644
--- a/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
+++ b/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
@@ -114,10 +114,20 @@ lp_build_blend_factor_unswizzled(struct lp_build_blend_aos_context *bld,
       if(alpha)
          return bld->base.one;
       else {
-         if(!bld->inv_dst)
-            bld->inv_dst = lp_build_comp(&bld->base, bld->dst);
-         if(!bld->saturate)
-            bld->saturate = lp_build_min(&bld->base, src_alpha, bld->inv_dst);
+         /*
+          * if there's separate src_alpha there's no dst alpha hence the complement
+          * is zero but for unclamped float inputs min can be non-zero (negative).
+          */
+         if (bld->src_alpha) {
+            if (!bld->saturate)
+               bld->saturate = lp_build_min(&bld->base, src_alpha, bld->base.zero);
+         }
+         else {
+            if(!bld->inv_dst)
+               bld->inv_dst = lp_build_comp(&bld->base, bld->dst);
+            if(!bld->saturate)
+               bld->saturate = lp_build_min(&bld->base, src_alpha, bld->inv_dst);
+         }
          return bld->saturate;
       }
    case PIPE_BLENDFACTOR_CONST_COLOR:
diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c b/src/gallium/drivers/llvmpipe/lp_state_fs.c
index afd01e3..a305109 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
@@ -2607,7 +2607,7 @@ llvmpipe_set_constant_buffer(struct pipe_context *pipe,
  * Return the blend factor equivalent to a destination alpha of one.
  */
 static INLINE unsigned
-force_dst_alpha_one(unsigned factor)
+force_dst_alpha_one(unsigned factor, boolean clamped_zero)
 {
    switch(factor) {
    case PIPE_BLENDFACTOR_DST_ALPHA:
@@ -2615,7 +2615,10 @@ force_dst_alpha_one(unsigned factor)
    case PIPE_BLENDFACTOR_INV_DST_ALPHA:
       return PIPE_BLENDFACTOR_ZERO;
    case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
-      return PIPE_BLENDFACTOR_ZERO;
+      if (clamped_zero)
+         return PIPE_BLENDFACTOR_ZERO;
+      else
+         return PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE;
    }
 
    return factor;
@@ -2735,8 +2738,13 @@ make_variant_key(struct llvmpipe_context *lp,
        */
       if (format_desc->swizzle[3] > UTIL_FORMAT_SWIZZLE_W ||
           format_desc->swizzle[3] == format_desc->swizzle[0]) {
-         blend_rt->rgb_src_factor   = force_dst_alpha_one(blend_rt->rgb_src_factor);
-         blend_rt->rgb_dst_factor   = force_dst_alpha_one(blend_rt->rgb_dst_factor);
+         /* Doesn't cover mixed snorm/unorm but can't render to them anyway */
+         boolean clamped_zero = !util_format_is_float(format) &&
+                                !util_format_is_snorm(format);
+         blend_rt->rgb_src_factor   = force_dst_alpha_one(blend_rt->rgb_src_factor,
+                                                          clamped_zero);
+         blend_rt->rgb_dst_factor   = force_dst_alpha_one(blend_rt->rgb_dst_factor,
+                                                          clamped_zero);
          blend_rt->alpha_func       = blend_rt->rgb_func;
          blend_rt->alpha_src_factor = blend_rt->rgb_src_factor;
          blend_rt->alpha_dst_factor = blend_rt->rgb_dst_factor;
-- 
1.7.9.5