[PATCH] remove damagePostOp() from DamageDamageRegion()

[Michel Dänzer]

On Tue, 2008-08-26 at 23:58 +0200, Maarten Maathuis wrote:
> This is a patch based on a suggestion by Michael Danzer, because it's
> non-trivial i'm posting it first. The idea is that PostOp only needs
> to be called after doing an actual rendering operation. Please voice
> any concerns you might have.

Thanks for taking this on Maarten.


> diff --git a/damageext/damageext.c b/damageext/damageext.c
> index 7dd328a..cea4636 100755
> --- a/damageext/damageext.c
> +++ b/damageext/damageext.c
> @@ -232,6 +232,7 @@ ProcDamageCreate (ClientPtr client)
>      {
>         pRegion = &((WindowPtr) pDrawable)->borderClip;
>         DamageDamageRegion (pDrawable, pRegion);
> +       DamageDamagePostOp (pDrawable);
>      }
>  
>      return (client->noClientException);
> @@ -302,6 +303,7 @@ ProcDamageAdd (ClientPtr client)
>       */
>      REGION_TRANSLATE(pScreen, pRegion, pDrawable->x, pDrawable->y);
>      DamageDamageRegion(pDrawable, pRegion);
> +    DamageDamagePostOp(pDrawable);
>      REGION_TRANSLATE(pScreen, pRegion, -pDrawable->x, -pDrawable->y);
>  
>      return (client->noClientException);

I think these must not be added; they could mislead damage records
interested in damage post operation into thinking that the drawable
contents were modified when in fact they were not.

BTW, I thought a bit more about ExaDamageReport(), and I think it (and
pExaPixmap->pendingDamage) actually could be removed again with these
changes if damageDamageRegion() was also changed to use
REGION_COPY(pScreen, &pDamage->pendingDamage, pDamageRegion) instead of
REGION_UNION(pScreen, &pDamage->pendingDamage, &pDamage->pendingDamage,
pDamageRegion), so pending damage only ever becomes real post-op damage
when damageReportPostOp is called, which I suspect is desireable in
general.


> @@ -1083,10 +1083,8 @@ exaTrapezoids (CARD8 op, PicturePtr pSrc, PicturePtr pDst,
>  
>         exaFinishAccess(pDraw, EXA_PREPARE_DEST);
>  
> -       /* Damage manually, because Trapezoids expects to hit Composite normally. */
> -       /* Composite is wrapped by damage, but Trapezoids isn't. */
>         if (pExaPixmap->pDamage) {
> -           DamageDamageRegion(pDraw, &migration);
> +           DamageDamagePostOp(pDraw);
>             REGION_UNINIT(pScreen, &migration);
>         }
>      }

You could move up the REGION_UNINIT and maybe limit the scope of the
variable 'migration'.


> diff --git a/glx/glxdri.c b/glx/glxdri.c
> index 8ae56ed..eb3fbb4 100644
> --- a/glx/glxdri.c
> +++ b/glx/glxdri.c
> @@ -805,6 +805,7 @@ static void __glXReportDamage(__DRIdrawable *driDraw,
>      REGION_INIT(pDraw->pScreen, &region, (BoxPtr) rects, num_rects);
>      REGION_TRANSLATE(pScreen, &region, pDraw->x, pDraw->y);
>      DamageDamageRegion(pDraw, &region);
> +    DamageDamagePostOp(pDraw);
>      REGION_UNINIT(pDraw->pScreen, &region);
>  
>      __glXleaveServer(GL_FALSE);
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index 7c1f00e..17202bd 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -362,6 +362,7 @@ static void dri2PostDamage(__DRIdrawable *draw,
>      REGION_INIT(pDraw->pScreen, &region, (BoxPtr) rects, numRects);
>      REGION_TRANSLATE(pScreen, &region, pDraw->x, pDraw->y);
>      DamageDamageRegion(pDraw, &region);
> +    DamageDamagePostOp(pDraw);
>      REGION_UNINIT(pDraw->pScreen, &region);
>  }
>  

These aren't correct. DamageDamageRegion needs to be called before the
SwapBuffers operation and DamageDamagePostOp afterwards. (Yes, this was
always broken :)


I think Adam Jackson has put some though into this as well, so maybe he
has more comments.


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