This fuses two loops in each function, eliminating an intermediate
MAXSCREENS-sized array from each.

Aside from being more efficient, I believe this is equivalent to the
previous implementation, since
- each per-screen GC has the graphicsExposures flag set the same way,
  and
- the REGION_* macros ignore their screen argument.

Signed-off-by: Jamey Sharp <[email protected]>
---
After sending the last version I noticed that CopyPlane had the same
pattern, so this version applies the same transformation to both.

On Fri, Apr 23, 2010 at 8:26 PM, Keith Packard <[email protected]> wrote:
> I've reviewed this as best I can,

Thanks for the review! Besides adding CopyPlane to the patch, I revised
the commit message to be more certain since you confirmed my
assumptions. I'd appreciate review of the new changes; the original
hunks are unchanged.

> but I'd like to see someone actually give it a try.

I'd appreciate that too. I'd prefer testing on this version of the patch
since it's more comprehensive.


 Xext/panoramiXprocs.c |   62 ++++++++++++++++++++----------------------------
 1 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/Xext/panoramiXprocs.c b/Xext/panoramiXprocs.c
index 3baa8f9..2bb1542 100644
--- a/Xext/panoramiXprocs.c
+++ b/Xext/panoramiXprocs.c
@@ -1092,10 +1092,12 @@ int PanoramiXCopyArea(ClientPtr client)
     } else {
        DrawablePtr pDst = NULL, pSrc = NULL;
        GCPtr pGC = NULL;
-       RegionPtr pRgn[MAXSCREENS];
+       RegionRec totalReg;
        int rc;
 
+       REGION_NULL(unusedScreen, &totalReg);
        FOR_NSCREENS_BACKWARD(j) {
+           RegionPtr pRgn;
            stuff->dstDrawable = dst->info[j].id;
            stuff->srcDrawable = src->info[j].id;
            stuff->gc          = gc->info[j].id;
@@ -1124,37 +1126,30 @@ int PanoramiXCopyArea(ClientPtr client)
            } else
                pSrc = pDst;
 
-           pRgn[j] = (*pGC->ops->CopyArea)(pSrc, pDst, pGC, 
+           pRgn = (*pGC->ops->CopyArea)(pSrc, pDst, pGC,
                                stuff->srcX, stuff->srcY,
                                stuff->width, stuff->height, 
                                stuff->dstX, stuff->dstY);
+           if(pGC->graphicsExposures && pRgn) {
+              if(srcIsRoot) {
+                  REGION_TRANSLATE(unusedScreen, pRgn,
+                           panoramiXdataPtr[j].x, panoramiXdataPtr[j].y);
+              }
+              REGION_APPEND(unusedScreen, &totalReg, pRgn);
+              REGION_DESTROY(unusedScreen, pRgn);
+           }
 
-           if(dstShared) {
-               while(j--) pRgn[j] = NULL;
+           if(dstShared)
                break;
-           }
        }
 
        if(pGC->graphicsExposures) {
            ScreenPtr pScreen = pDst->pScreen;
-           RegionRec totalReg;
            Bool overlap;
-
-           REGION_NULL(pScreen, &totalReg);
-           FOR_NSCREENS_BACKWARD(j) {
-               if(pRgn[j]) {
-                  if(srcIsRoot) {
-                      REGION_TRANSLATE(pScreen, pRgn[j], 
-                               panoramiXdataPtr[j].x, panoramiXdataPtr[j].y);
-                  }
-                  REGION_APPEND(pScreen, &totalReg, pRgn[j]);
-                  REGION_DESTROY(pScreen, pRgn[j]);
-               }
-           }
-           REGION_VALIDATE(pScreen, &totalReg, &overlap);
+           REGION_VALIDATE(unusedScreen, &totalReg, &overlap);
            (*pScreen->SendGraphicsExpose)(
                client, &totalReg, stuff->dstDrawable, X_CopyArea, 0);
-           REGION_UNINIT(pScreen, &totalReg);
+           REGION_UNINIT(unusedScreen, &totalReg);
        }
        
        result = client->noClientException;
@@ -1173,7 +1168,7 @@ int PanoramiXCopyPlane(ClientPtr client)
     Bool               srcShared, dstShared;
     DrawablePtr        psrcDraw, pdstDraw = NULL;
     GCPtr              pGC = NULL;
-    RegionPtr          pRgn[MAXSCREENS];
+    RegionRec          totalReg;
     REQUEST(xCopyPlaneReq);
 
     REQUEST_SIZE_MATCH(xCopyPlaneReq);
@@ -1208,7 +1203,9 @@ int PanoramiXCopyPlane(ClientPtr client)
     srcx = stuff->srcX; srcy = stuff->srcY;
     dstx = stuff->dstX; dsty = stuff->dstY;
  
+    REGION_NULL(unusedScreen, &totalReg);
     FOR_NSCREENS_BACKWARD(j) {
+       RegionPtr pRgn;
        stuff->dstDrawable = dst->info[j].id;
        stuff->srcDrawable = src->info[j].id;
        stuff->gc          = gc->info[j].id;
@@ -1241,33 +1238,26 @@ int PanoramiXCopyPlane(ClientPtr client)
            return(BadValue);
        }
 
-       pRgn[j] = (*pGC->ops->CopyPlane)(psrcDraw, pdstDraw, pGC, 
+       pRgn = (*pGC->ops->CopyPlane)(psrcDraw, pdstDraw, pGC,
                                stuff->srcX, stuff->srcY,
                                stuff->width, stuff->height, 
                                stuff->dstX, stuff->dstY, stuff->bitPlane);
+       if(pGC->graphicsExposures && pRgn) {
+           REGION_APPEND(unusedScreen, &totalReg, pRgn);
+           REGION_DESTROY(unusedScreen, pRgn);
+       }
 
-       if(dstShared) {
-           while(j--) pRgn[j] = NULL;
+       if(dstShared)
            break;
-       }
     }
 
     if(pGC->graphicsExposures) {
        ScreenPtr pScreen = pdstDraw->pScreen;
-       RegionRec totalReg;
        Bool overlap;
-
-       REGION_NULL(pScreen, &totalReg);
-       FOR_NSCREENS_BACKWARD(j) {
-           if(pRgn[j]) {
-               REGION_APPEND(pScreen, &totalReg, pRgn[j]);
-               REGION_DESTROY(pScreen, pRgn[j]);
-           }
-       }
-       REGION_VALIDATE(pScreen, &totalReg, &overlap);
+       REGION_VALIDATE(unusedScreen, &totalReg, &overlap);
        (*pScreen->SendGraphicsExpose)(
                client, &totalReg, stuff->dstDrawable, X_CopyPlane, 0);
-       REGION_UNINIT(pScreen, &totalReg);
+       REGION_UNINIT(unusedScreen, &totalReg);
     }
 
     return (client->noClientException);
-- 
1.7.0

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to