[Xorg-driver-geode] [PATCH 2/5] Put the one pixel renderingwork back to the server to handle if the pMsk is not zero

[Huang, FrankR]

Good review.
Have you imagine the condition below:
	Although the source must be one pixel, but when the mask is not zero, there is no variable to record the source size(1x1). Because the exaScratch.srcWidth and exaScratch.srcHeight are the mask's width and mask's height if the mask is not zero(I have given the note in the patch you push in).
	So the rendering region will be enlarged if you don't fallback.
	Please try the application I give you in the attached file. With my patch 2 and without my patch 2, you can see the difference.


Thanks,
Frank

	

-----Original Message-----
From: Mart Raudsepp [mailto:leio at gentoo.org] 
Sent: 2010?7?6? 14:57
To: Huang, FrankR
Cc: xorg-driver-geode at lists.x.org
Subject: Re: [Xorg-driver-geode] [PATCH 2/5] Put the one pixel renderingwork back to the server to handle if the pMsk is not zero

On Fri, 2010-07-02 at 15:59 +0800, Huang, FrankR wrote:
> From: Frank Huang <frankr.huang at amd.com>
> 
> *With mask, our driver can only do one pixel source with repeat
> rendering. That is a solid source picture. Otherwise, we must
> return it to server.

Why is this fallback addition necessary? I mean, I think that we already
fallback in case of non-one pixel source in there, by this existing
code:

    if (pMsk && op != PictOpClear) {
	// ... some other fallback code not relevant here ...
	/* The pSrc should be 1x1 pixel if the pMsk is not zero */
	if (pSrc->pDrawable->width != 1 || pSrc->pDrawable->height != 1)
	    return FALSE;
    }

So your added fallback code would only be necessary only if either:

a) PictOpClear of any size source with repeat flag is an issue as well
b) We don't handle non-repeating 1x1 source pixel correctly, but we seem
to have code for that - the while loop in lx_do_composite will just do
one cycle if !repeat


So I believe this patch is redundant and not necessary, do you agree?

Meanwhile this patch is
Nacked-by: Mart Raudsepp <leio at gentoo.org>

> Signed-off-by: Frank Huang <frankr.huang at amd.com>
> ---
>  src/lx_exa.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/src/lx_exa.c b/src/lx_exa.c
> index e0def8f..fbb62a0 100644
> --- a/src/lx_exa.c
> +++ b/src/lx_exa.c
> @@ -589,6 +589,13 @@ lx_check_composite(int op, PicturePtr pSrc, PicturePtr pMsk, PicturePtr pDst)
>      if (pSrc->format == PICT_a8 || pDst->format == PICT_a8)
>  	return FALSE;
>  
> +    /* When the mask is not zero, we can only do one pixel source(1x1)
> +     * with repeatable parameter rendering, because we need a solid source
> +     * picture. If that is not so, return to Xserver to handle it */
> +
> +    if (pMsk && (!pSrc->repeat))
> +	return FALSE;
> +
>      if (pMsk && op != PictOpClear) {
>  	struct blend_ops_t *opPtr = &lx_alpha_ops[op * 2];
>  	int direction = (opPtr->channel == CIMGP_CHANNEL_A_SOURCE) ? 0 : 1;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: render.tgz
Type: application/x-compressed
Size: 21951 bytes
Desc: render.tgz
URL: <http://lists.x.org/archives/xorg-driver-geode/attachments/20100706/94106f4d/attachment-0001.bin>