[Mesa-dev] [PATCH] st/xa: Use PIPE_FORMAT_R8_UNORM when available

[Thomas Hellstrom]

On 09/17/2015 11:53 AM, Thomas Hellstrom wrote:
> On 09/17/2015 11:34 AM, Jose Fonseca wrote:
>> On 16/09/15 14:04, Thomas Hellstrom wrote:
>>> XA has been using L8_UNORM for a8 and yuv component surfaces.
>>> This commit instead makes XA prefer R8_UNORM since it's assumed to
>>> have a
>>> higher availability.
>>>
>>> Also neither of these formats are suitable as destination formats using
>>> destination alpha blending, so reject those operations.
>>>
>>> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
>>> ---
>>>   src/gallium/state_trackers/xa/xa_composite.c | 40
>>> ++++++++++------------------
>>>   src/gallium/state_trackers/xa/xa_tracker.c   | 28 +++++++++++++------
>>>   2 files changed, 34 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/xa/xa_composite.c
>>> b/src/gallium/state_trackers/xa/xa_composite.c
>>> index 7cfd1e1..e81eeba 100644
>>> --- a/src/gallium/state_trackers/xa/xa_composite.c
>>> +++ b/src/gallium/state_trackers/xa/xa_composite.c
>>> @@ -78,26 +78,6 @@ static const struct xa_composite_blend xa_blends[]
>>> = {
>>>         0, 0, PIPE_BLENDFACTOR_ONE, PIPE_BLENDFACTOR_ONE},
>>>   };
>>>
>>> -
>>> -/*
>>> - * The alpha value stored in a luminance texture is read by the
>>> - * hardware as color.
>>> - */
>>> -static unsigned
>>> -xa_convert_blend_for_luminance(unsigned factor)
>>> -{
>>> -    switch(factor) {
>>> -    case PIPE_BLENDFACTOR_DST_ALPHA:
>>> -    return PIPE_BLENDFACTOR_DST_COLOR;
>>> -    case PIPE_BLENDFACTOR_INV_DST_ALPHA:
>>> -    return PIPE_BLENDFACTOR_INV_DST_COLOR;
>>> -    default:
>>> -    break;
>>> -    }
>>> -    return factor;
>>> -}
>>> -
>>> -
>>>   static boolean
>>>   blend_for_op(struct xa_composite_blend *blend,
>>>            enum xa_composite_op op,
>>> @@ -131,10 +111,16 @@ blend_for_op(struct xa_composite_blend *blend,
>>>       if (!dst_pic->srf)
>>>       return supported;
>>>
>>> -    if (dst_pic->srf->tex->format == PIPE_FORMAT_L8_UNORM) {
>>> -    blend->rgb_src = xa_convert_blend_for_luminance(blend->rgb_src);
>>> -    blend->rgb_dst = xa_convert_blend_for_luminance(blend->rgb_dst);
>>> -    }
>>> +    /*
>>> +     * None of the hardware formats we might use for dst A8 are
>>> +     * suitable for dst_alpha blending, since they present the
>>> +     * alpha channel either in all color channels (L8_UNORM) or
>>> +     * in the red channel only (R8_UNORM)
>>> +     */
>>> +    if ((dst_pic->srf->tex->format == PIPE_FORMAT_L8_UNORM ||
>>> +         dst_pic->srf->tex->format == PIPE_FORMAT_R8_UNORM) &&
>>> +        blend->alpha_dst)
>>> +        return FALSE;
>> I'm not very familiar with this code, but wasn't
>> xa_convert_blend_for_luminance compensating for the A8 <-> L8
>> mismatch?  It looks like it was doing the right thing.   And
>> xa_convert_blend_for_luminance would work both for L8 and R8.
> In these operations since the destination format is A8, we're only
> interested in the resulting alpha channel. With
> PIPE_BLENDFACTOR_DST_ALPHA, we would use the destination alpha channel
> for blending, which would pick up 1 for L8 and R8 which is obviously
> wrong. Now the removed function instead set PIPE_BLENDFACTOR_DST_COLOR,
> which would, if I read the specs correctly blend component-wise and
> yield the correct results for the R, G and B channels if L8 is used. and
> for the R channel only if R8 is used. But a prerequisite here is that
> the destination logical format is A8 and the alpha channel is never
> correct AFAICT.
>
Hmm, or are you making the point that blending takes place when we've
already swizzled the alpha value to the red channel? Yes, then you're
obviously right, and I need to revert that "fix" ;).

/Thomas