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 <thellst...@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.
>
>>
>> /*
>> * If there's no dst alpha channel, adjust the blend op so that
>> we'll treat
>> @@ -298,7 +284,8 @@ picture_format_fixups(struct xa_picture *src_pic,
>> ret |= mask ? FS_MASK_SET_ALPHA : FS_SRC_SET_ALPHA;
>>
>> if (src_hw_format == src_pic_format) {
>> - if (src->tex->format == PIPE_FORMAT_L8_UNORM)
>> + if (src->tex->format == PIPE_FORMAT_L8_UNORM ||
>> + src->tex->format == PIPE_FORMAT_R8_UNORM)
>> return ((mask) ? FS_MASK_LUMINANCE : FS_SRC_LUMINANCE);
>>
>> return ret;
>> @@ -372,7 +359,8 @@ bind_shaders(struct xa_context *ctx, const struct
>> xa_composite *comp)
>> fs_traits |= picture_format_fixups(mask_pic, 1);
>> }
>>
>> - if (ctx->srf->format == PIPE_FORMAT_L8_UNORM)
>> + if (ctx->srf->format == PIPE_FORMAT_L8_UNORM ||
>> + ctx->srf->format == PIPE_FORMAT_R8_UNORM)
>> fs_traits |= FS_DST_LUMINANCE;
>
> What exactly is FS_DST_LUMINANCE supposed to do? From xa_tgsi.c it
> seems to swizzle the alpha channel into the red channel, before
> outputing. But if we no longer want to emulate A8 with R8/L8 then it
> seems that's no longer necessary.
Yes, it swizzles the alpha channel to all color channels. What we
disable above is only those composite operations that use DEST_ALPHA
blending (which is only a few), together with a R8/L8 destination.
Probably a very odd combination, so there are definitely situations were
we can composite to A8 using other composite operations, and I've
verified that it works.
>
>
> BTW, isn't necessary to update
> src/gallium/state_trackers/xa/xa_renderer.c to apply
> FS_SRC/DST_LUMINANCE traits for R8_UNORM too?
Hmm. Good point. I have missed that. I'll take a look.
Thanks,
Thomas
>
>
>>
>> shader = xa_shaders_get(ctx->shaders, vs_traits, fs_traits);
>> diff --git a/src/gallium/state_trackers/xa/xa_tracker.c
>> b/src/gallium/state_trackers/xa/xa_tracker.c
>> index 2944b16..cd1394a 100644
>> --- a/src/gallium/state_trackers/xa/xa_tracker.c
>> +++ b/src/gallium/state_trackers/xa/xa_tracker.c
>> @@ -82,7 +82,7 @@ static const unsigned int
>> stype_bind[XA_LAST_SURFACE_TYPE] = { 0,
>> };
>>
>> static struct xa_format_descriptor
>> -xa_get_pipe_format(enum xa_formats xa_format)
>> +xa_get_pipe_format(struct xa_tracker *xa, enum xa_formats xa_format)
>> {
>> struct xa_format_descriptor fdesc;
>>
>> @@ -102,7 +102,13 @@ xa_get_pipe_format(enum xa_formats xa_format)
>> fdesc.format = PIPE_FORMAT_B5G5R5A1_UNORM;
>> break;
>> case xa_format_a8:
>> - fdesc.format = PIPE_FORMAT_L8_UNORM;
>> + if (xa->screen->is_format_supported(xa->screen,
>> PIPE_FORMAT_R8_UNORM,
>> + PIPE_TEXTURE_2D, 0,
>> + stype_bind[xa_type_a] |
>> + PIPE_BIND_RENDER_TARGET))
>> + fdesc.format = PIPE_FORMAT_R8_UNORM;
>> + else
>> + fdesc.format = PIPE_FORMAT_L8_UNORM;
>> break;
>> case xa_format_z24:
>> fdesc.format = PIPE_FORMAT_Z24X8_UNORM;
>> @@ -126,7 +132,12 @@ xa_get_pipe_format(enum xa_formats xa_format)
>> fdesc.format = PIPE_FORMAT_S8_UINT_Z24_UNORM;
>> break;
>> case xa_format_yuv8:
>> - fdesc.format = PIPE_FORMAT_L8_UNORM;
>> + if (xa->screen->is_format_supported(xa->screen,
>> PIPE_FORMAT_R8_UNORM,
>> + PIPE_TEXTURE_2D, 0,
>> +
>> stype_bind[xa_type_yuv_component]))
>> + fdesc.format = PIPE_FORMAT_R8_UNORM;
>> + else
>> + fdesc.format = PIPE_FORMAT_L8_UNORM;
>> break;
>> default:
>> fdesc.xa_format = xa_format_unknown;
>> @@ -184,7 +195,8 @@ xa_tracker_create(int drm_fd)
>> for (i = 0; i < num_preferred[stype]; ++i) {
>> xa_format = preferred[stype][i];
>>
>> - struct xa_format_descriptor fdesc =
>> xa_get_pipe_format(xa_format);
>> + struct xa_format_descriptor fdesc =
>> + xa_get_pipe_format(xa, xa_format);
>>
>> if (xa->screen->is_format_supported(xa->screen, fdesc.format,
>> PIPE_TEXTURE_2D, 0, bind)) {
>> @@ -259,7 +271,7 @@ xa_get_format_stype_depth(struct xa_tracker *xa,
>> int found = 0;
>>
>> for (i = xa->format_map[stype][0]; i <=
>> xa->format_map[stype][1]; ++i) {
>> - fdesc = xa_get_pipe_format(xa->supported_formats[i]);
>> + fdesc = xa_get_pipe_format(xa, xa->supported_formats[i]);
>> if (fdesc.xa_format != xa_format_unknown &&
>> xa_format_depth(fdesc.xa_format) == depth) {
>> found = 1;
>> @@ -277,7 +289,7 @@ XA_EXPORT int
>> xa_format_check_supported(struct xa_tracker *xa,
>> enum xa_formats xa_format, unsigned int flags)
>> {
>> - struct xa_format_descriptor fdesc = xa_get_pipe_format(xa_format);
>> + struct xa_format_descriptor fdesc = xa_get_pipe_format(xa,
>> xa_format);
>> unsigned int bind;
>>
>> if (fdesc.xa_format == xa_format_unknown)
>> @@ -328,7 +340,7 @@ surface_create(struct xa_tracker *xa,
>> if (xa_format == xa_format_unknown)
>> fdesc = xa_get_format_stype_depth(xa, stype, depth);
>> else
>> - fdesc = xa_get_pipe_format(xa_format);
>> + fdesc = xa_get_pipe_format(xa, xa_format);
>>
>> if (fdesc.xa_format == xa_format_unknown)
>> return NULL;
>> @@ -440,7 +452,7 @@ xa_surface_redefine(struct xa_surface *srf,
>> if (xa_format == xa_format_unknown)
>> fdesc = xa_get_format_stype_depth(xa, stype, depth);
>> else
>> - fdesc = xa_get_pipe_format(xa_format);
>> + fdesc = xa_get_pipe_format(xa, xa_format);
>>
>> if (width == template->width0 && height == template->height0 &&
>> template->format == fdesc.format &&
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
