From e57b98bad33b13ba02b11144709dd09af2ea8c95 Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Thu, 18 Jul 2013 02:05:34 +0200 Subject: [PATCH] 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 Reviewed-by: Zack Rusin --- .../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 c4d04a26873..377eaa5a6e0 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 afd01e31725..a305109c010 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; -- 2.30.2