llvmpipe: fix blending with SRC_ALPHA_SATURATE with some formats without alpha

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.)

Reviewed-by: Jose Fonseca <jfonseca@vmware.com>
Reviewed-by: Zack Rusin <zackr@vmware.com>

diff --git a/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c b/src/gallium/drivers/llvmpipe/lp_bld_blend_aos.c
index c4d04a26873741882199ec65efe247da9962febb..377eaa5a6e09f69f6d7a1e72400854c8c8bc4d60 100644 (file)
--- 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 afd01e31725efd2c7ccd4e2889b8d5ad0902279a..a305109c010eb18b82ad05299e754be0ee62e07c 100644 (file)
--- 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;