xserver: Branch 'master' - 4 commits

Michel Dänzer michel at tungstengraphics.com
Tue Aug 26 11:21:02 PDT 2008


On Tue, 2008-08-26 at 08:57 -0700, Maarten Maathuis wrote:
> 
> commit 7c14fdbacfcd2f4d56a346e6c72e44e4ba9909c1
> Author: Maarten Maathuis <madman2003 at gmail.com>
> Date:   Tue Aug 26 17:21:43 2008 +0200
> 
>     exa: some minor cleanup
>     - Fix compile warning
>     - Order exa.h by source file that exports the function.
>     - Move the function i created earlier to private headers.

I would prefer such unrelated changes to be in separate commits. (You
could have even fixed up the location of the new function prototype in
the commit where it was added before pushing these changes - something
like stgit/guilt makes that easy)


> commit 988725f32e082aee9392a71464125157a83d1e67
> Author: Maarten Maathuis <madman2003 at gmail.com>
> Date:   Tue Aug 26 16:54:29 2008 +0200
> 
>     exa: move destination damage for internal calls to a special function
>     - This should improve clarity for someone who isn't familiar with the code.

This change is incorrect:

>  /**
> + * Returns TRUE if the pixmap has damage.
> + * EXA only migrates the parts of a destination 
> + * that are affected by rendering.
> + * It uses the current damage as indication.
> + * So anything that does not need to be updated won't be.
> + * For clarity this seperate function was made.
> + * Note that some situations don't use this, 
> + * because their calls are wrapped by the damage layer.
> + */
> +Bool
> +exaDamageDestForMigration(PixmapPtr pPix, RegionPtr region)
> +{
> +    ScreenPtr pScreen = pPix->drawable.pScreen;
> +    (void) pScreen; /* the macros don't use pScreen currently */
> +    ExaPixmapPriv (pPix);
> +    int x_offset, y_offset;
> +    RegionPtr pending_damage;
> +
> +    if (!pExaPixmap->pDamage)
> +	return FALSE;
> +
> +    exaGetDrawableDeltas(&pPix->drawable, pPix, &x_offset, &y_offset);
> +
> +    REGION_TRANSLATE(pScreen, region, x_offset, y_offset);
> +    pending_damage = DamagePendingRegion(pExaPixmap->pDamage);
> +    REGION_UNION(pScreen, pending_damage, pending_damage, region);
> +    /* Restore region as we got it. */
> +    REGION_TRANSLATE(pScreen, region, -x_offset, -y_offset);
> +
> +    return TRUE;
> +}

If you look at exaGetDrawableDeltas(), you'll see that x/y_offset will
always be 0 here. At the very least, exaDamageDestForMigration() will
need to take a separate DrawablePtr argument like and for
exaGetDrawableDeltas().

Though I think a cleaner long term solution would be to move the
damageReportPostOp() call from DamageDamageRegion() to a separate
function like DamageDamagePostOp(), so we could just use
DamageDamageRegion() instead of exaDamageDestForMigration() and
DamageDamagePostOp() after the corresponding rendering operation.


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer




More information about the xorg mailing list