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

[Christian König]

Am 18.02.2013 20:11, schrieb 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...).

I agree alpha blending with a buffer format that doesn't have alpha is a 
bit strange, that should be catched by the upper layers.

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

Why should we do so? SRC_ALPHA_SATURATE should still work fine, even 
when the destination buffer doesn't have an alpha component.

Christian.

>
> Roland
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev