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:[email protected]]
Sent: 2010?7?6? 14:57
To: Huang, FrankR
Cc: [email protected]
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 <[email protected]>
>
> *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 <[email protected]>
> Signed-off-by: Frank Huang <[email protected]>
> ---
> 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;
render.tgz
Description: render.tgz
_______________________________________________ Xorg-driver-geode mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-driver-geode
