[Mesa-dev] [PATCH] radeonsi: Fix blending using destination alpha factor but non-alpha destination

[Roland Scheidegger]

Am 18.02.2013 19:14, schrieb Michel Dänzer:
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> 11 more little piglits.
> 
> NOTE: This is a candidate for the 9.1 branch.
> 
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
> 
> Any ideas why this seems necessary with radeonsi but not with r600g?
Maybe the hw uses an implicit 1 if the format has no alpha (though I'm
not sure if it can always know with bgrx formats and the like).
I'm wondering if there should be a helper for those fixups. Looks to me
like quite some drivers need it (though well so far I think just
non-gallium i965 does this plus llvmpipe, but for some of the others I'm
skeptical if not doing it is really correct...).

> 
>  src/gallium/drivers/radeonsi/si_state.c | 116 +++++++++++++++++---------------
>  src/gallium/drivers/radeonsi/si_state.h |   3 +-
>  2 files changed, 61 insertions(+), 58 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
> index d20e3ff..144a29d 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -36,33 +36,6 @@
>  #include "si_state.h"
>  #include "sid.h"
>  
> -/*
> - * inferred framebuffer and blender state
> - */
> -static void si_update_fb_blend_state(struct r600_context *rctx)
> -{
> -	struct si_pm4_state *pm4;
> -	struct si_state_blend *blend = rctx->queued.named.blend;
> -	uint32_t mask;
> -
> -	if (blend == NULL)
> -		return;
> -
> -	pm4 = CALLOC_STRUCT(si_pm4_state);
> -	if (pm4 == NULL)
> -		return;
> -
> -	mask = (1ULL << ((unsigned)rctx->framebuffer.nr_cbufs * 4)) - 1;
> -	mask &= blend->cb_target_mask;
> -	si_pm4_set_reg(pm4, R_028238_CB_TARGET_MASK, mask);
> -
> -	si_pm4_set_state(rctx, fb_blend, pm4);
> -}
> -
> -/*
> - * Blender functions
> - */
> -
>  static uint32_t si_translate_blend_function(int blend_func)
>  {
>  	switch (blend_func) {
> @@ -84,7 +57,7 @@ static uint32_t si_translate_blend_function(int blend_func)
>  	return 0;
>  }
>  
> -static uint32_t si_translate_blend_factor(int blend_fact)
> +static uint32_t si_translate_blend_factor(int blend_fact, bool dst_alpha)
>  {
>  	switch (blend_fact) {
>  	case PIPE_BLENDFACTOR_ONE:
> @@ -94,7 +67,7 @@ static uint32_t si_translate_blend_factor(int blend_fact)
>  	case PIPE_BLENDFACTOR_SRC_ALPHA:
>  		return V_028780_BLEND_SRC_ALPHA;
>  	case PIPE_BLENDFACTOR_DST_ALPHA:
> -		return V_028780_BLEND_DST_ALPHA;
> +		return dst_alpha ? V_028780_BLEND_DST_ALPHA : V_028780_BLEND_ONE;
>  	case PIPE_BLENDFACTOR_DST_COLOR:
>  		return V_028780_BLEND_DST_COLOR;
>  	case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
> @@ -110,7 +83,7 @@ static uint32_t si_translate_blend_factor(int blend_fact)
>  	case PIPE_BLENDFACTOR_INV_SRC_ALPHA:
>  		return V_028780_BLEND_ONE_MINUS_SRC_ALPHA;
>  	case PIPE_BLENDFACTOR_INV_DST_ALPHA:
> -		return V_028780_BLEND_ONE_MINUS_DST_ALPHA;
> +		return dst_alpha ? V_028780_BLEND_ONE_MINUS_DST_ALPHA : V_028780_BLEND_ZERO;
>  	case PIPE_BLENDFACTOR_INV_DST_COLOR:
>  		return V_028780_BLEND_ONE_MINUS_DST_COLOR;
>  	case PIPE_BLENDFACTOR_INV_CONST_COLOR:
> @@ -133,30 +106,25 @@ static uint32_t si_translate_blend_factor(int blend_fact)
>  	return 0;
>  }

I think you might also need to patch up SRC_ALPHA_SATURATE (to zero).

Can't comment on the hw stuff but at least llvmpipe does the same
otherwise :-).

Roland