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

2012-02-08 Thread Aaron Plattner 

On 01/09/2012 10:21 AM, Aaron Plattner wrote:

On 01/06/2012 10:46 AM, Adam Jackson wrote:

Before:
4000 trep @   0.0009 msec (1148346.9/sec): PutImage 10x10 square
6000 trep @   0.0005 msec (2091666.1/sec): ShmPutImage 10x10 square

After:
4000 trep @   0.0008 msec (1191807.5/sec): PutImage 10x10 square
6000 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 Anholte...@anholt.net
Signed-off-by: Adam Jacksona...@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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

2012-01-09 Thread Aaron Plattner 

On 01/06/2012 10:46 AM, Adam Jackson wrote:

Before:
4000 trep @   0.0009 msec (1148346.9/sec): PutImage 10x10 square
6000 trep @   0.0005 msec (2091666.1/sec): ShmPutImage 10x10 square

After:
4000 trep @   0.0008 msec (1191807.5/sec): PutImage 10x10 square
6000 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 Anholte...@anholt.net
Signed-off-by: Adam Jacksona...@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.



  void miDamageDestroy (DamagePtr pDamage)


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

2012-01-09 Thread Adam Jackson 
On Mon, 2012-01-09 at 10:21 -0800, Aaron Plattner wrote:

 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.

D'oh, forgot that was global.  Excellent point again, thanks for
catching it.

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

2012-01-06 Thread Adam Jackson 
Before:
4000 trep @   0.0009 msec (1148346.9/sec): PutImage 10x10 square
6000 trep @   0.0005 msec (2091666.1/sec): ShmPutImage 10x10 square

After:
4000 trep @   0.0008 msec (1191807.5/sec): PutImage 10x10 square
6000 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 e...@anholt.net
Signed-off-by: Adam Jackson a...@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++;
 }
 
 void miDamageDestroy (DamagePtr pDamage)
-- 
1.7.7.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

2012-01-06 Thread Chris Wilson 
On Fri,  6 Jan 2012 13:46:38 -0500, Adam Jackson a...@redhat.com wrote:
 Before:
 4000 trep @   0.0009 msec (1148346.9/sec): PutImage 10x10 square
 6000 trep @   0.0005 msec (2091666.1/sec): ShmPutImage 10x10 square
 
 After:
 4000 trep @   0.0008 msec (1191807.5/sec): PutImage 10x10 square
 6000 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 e...@anholt.net
 Signed-off-by: Adam Jackson a...@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;

That comment is quite annoying and does not make those macros any less
opaque. How about:

  pGCPriv-ops = pDamage; /* a non-NULL value to enable damage tracking
 and fixed up in the epilogue to the appropriate
 wrapped ops */

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel