[PATCH] miext/damage: Only wrap into the GC ops chain if there's a listener (v3)

[Aaron Plattner]

On 09/20/2012 02:57 PM, Adam Jackson wrote:
>   before           after          Operation
> --------    -----------------    -----------------
> 1148346.9   1191807.5 (  1.04)   PutImage 10x10 square
> 2091666.1   2180983.0 (  1.04)   ShmPutImage 10x10 square
>
> v3: In miDamage{R,Unr}egister, bump the serial number of the affected
> drawable (and all children if it's a window) so subsequent drawing
> against the damage will trigger another ValidateGC pass and we wrap
> in/out correctly.  Spotted by Aaron Plattner.
>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>   miext/damage/damage.c |   30 +++++++++++++++++++++++++++++-
>   1 files changed, 29 insertions(+), 1 deletions(-)
>
> diff --git a/miext/damage/damage.c b/miext/damage/damage.c
> index 2d2b422..a98c20e 100644
> --- a/miext/damage/damage.c
> +++ b/miext/damage/damage.c
> @@ -436,9 +436,13 @@ damageCreateGC(GCPtr pGC)
>   static void
>   damageValidateGC(GCPtr pGC, unsigned long changes, DrawablePtr pDrawable)
>   {
> +    drawableDamage(pDrawable);
>       DAMAGE_GC_FUNC_PROLOGUE(pGC);
>       (*pGC->funcs->ValidateGC) (pGC, changes, pDrawable);
> -    pGCPriv->ops = pGC->ops;    /* just so it's not NULL */
> +    if (pDamage)
> +        pGCPriv->ops = pGC->ops; /* so it's not NULL, so FUNC_EPILOGUE does work */
> +    else
> +        pGCPriv->ops = NULL;
>       DAMAGE_GC_FUNC_EPILOGUE(pGC);
>   }
>
> @@ -1663,14 +1667,38 @@ miDamageCreate(DamagePtr pDamage)
>   {
>   }
>
> +/*
> + * We only wrap into the GC when there's a registered listener.  For windows,
> + * damage includes damage to children.  So if there's a GC validated against
> + * a subwindow and we then register a damage on the parent, we need to bump
> + * the serial numbers of the children to re-trigger validation.
> + *
> + * Since we can't know if a GC has been validated against one of the affected
> + * children, just bump them all to be safe.
> + */
> +static int
> +damageRegisterVisit(WindowPtr pWin, void *data)
> +{
> +    pWin->drawable.serialNumber = NEXT_SERIAL_NUMBER;
> +    return WT_WALKCHILDREN;
> +}
> +
>   void
>   miDamageRegister(DrawablePtr pDrawable, DamagePtr pDamage)
>   {
> +    if (pDrawable->type == DRAWABLE_WINDOW)
> +        TraverseTree((WindowPtr)pDrawable, damageRegisterVisit, NULL);
> +    else
> +        pDrawable->serialNumber = NEXT_SERIAL_NUMBER;
>   }
>
>   void
>   miDamageUnregister(DrawablePtr pDrawable, DamagePtr pDamage)
>   {
> +    if (pDrawable->type == DRAWABLE_WINDOW)
> +        TraverseTree((WindowPtr)pDrawable, damageRegisterVisit, NULL);
> +    else
> +        pDrawable->serialNumber = NEXT_SERIAL_NUMBER;
>   }
>
>   void
>

I think this looks good now.

Reviewed-by: Aaron Plattner <aplattner at nvidia.com>