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

[Aaron Plattner]

On 01/09/2012 10:21 AM, Aaron Plattner wrote:
> On 01/06/2012 10:46 AM, Adam Jackson wrote:
>> Before:
>> 40000000 trep @   0.0009 msec (1148346.9/sec): PutImage 10x10 square
>> 60000000 trep @   0.0005 msec (2091666.1/sec): ShmPutImage 10x10 square
>>
>> After:
>> 40000000 trep @   0.0008 msec (1191807.5/sec): PutImage 10x10 square
>> 60000000 trep @   0.0005 msec (2180983.0/sec): ShmPutImage 10x10 square
>>
>> v2: Bump drawable serial number on damage register/unregister to trigger
>>       ValidateGC, otherwise a Damage created after the GC was validated
>>       would never be called, and a Damage destroyed on a validated GC
>>       would probably crash the next GC draw.  Spotted by Aaron Plattner.
>> v2.1: Actually include the above change.
>>
>> Reviewed-by: Eric Anholt<eric at anholt.net>
>> Signed-off-by: Adam Jackson<ajax at redhat.com>
>> ---
>>    miext/damage/damage.c |    9 ++++++++-
>>    1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/miext/damage/damage.c b/miext/damage/damage.c
>> index d791211..37599dd 100644
>> --- a/miext/damage/damage.c
>> +++ b/miext/damage/damage.c
>> @@ -437,9 +437,13 @@ 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;  /* just so it's not NULL */
>> +    else
>> +	pGCPriv->ops = NULL;
>>        DAMAGE_GC_FUNC_EPILOGUE (pGC);
>>    }
>>
>> @@ -1766,12 +1770,15 @@ void miDamageCreate (DamagePtr pDamage)
>>    {
>>    }
>>
>> +/* Bump serial on register/unregister to trigger ValidateGC */
>>    void miDamageRegister (DrawablePtr pDrawable, DamagePtr pDamage)
>>    {
>> +    pDrawable->serialNumber++;
>>    }
>>
>>    void miDamageUnregister (DrawablePtr pDrawable, DamagePtr pDamage)
>>    {
>> +    pDrawable->serialNumber++;
>>    }
>
> I think these need to be pDrawable->serialNumber = NEXT_SERIAL_NUMBER.
> Otherwise, creating two pixmaps, validating a GC for the second one,
> creating a damage object on the first, and then trying to use the second
> GC on the first drawable might cause similar problems.

I was thinking about this change again today for some reason and I think 
this will still be a problem if the drawable is a window and there's a 
GC validated against one of its descendant windows.

We probably need a TraverseTree in miDamageRegister.

-- Aaron