dix: Always store GC client clip as a region (v2)

Again, this changes FixesCreateRegionFromGC to throw BadMatch when fed a
GC with no client clip.

v2: Fix Xnest and some variable names (Keith)

Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Adam Jackson <ajax@redhat.com>

dix/gc.c

@@ -495,7 +495,6 @@ NewGCObject(ScreenPtr pScreen, int depth)
     pGC->graphicsExposures = TRUE;
     pGC->clipOrg.x = 0;
     pGC->clipOrg.y = 0;
-    pGC->clientClipType = CT_NONE;
     pGC->clientClip = (void *) NULL;
     pGC->numInDashList = 2;
     pGC->dash = DefaultDash;
@@ -1067,7 +1066,7 @@ GetScratchGC(unsigned depth, ScreenPtr pScreen)
             pGC->graphicsExposures = FALSE;
             pGC->clipOrg.x = 0;
             pGC->clipOrg.y = 0;
-            if (pGC->clientClipType != CT_NONE)
+            if (pGC->clientClip)
                 (*pGC->funcs->ChangeClip) (pGC, CT_NONE, NULL, 0);
             pGC->stateChanges = GCAllBits;
             return pGC;

exa/exa_accel.c

@@ -413,7 +413,7 @@ exaHWCopyNtoN(DrawablePtr pSrcDrawable,
 
         if (!pGC || !exaGCReadsDestination(pDstDrawable, pGC->planemask,
                                            pGC->fillStyle, pGC->alu,
-                                           pGC->clientClipType)) {
+                                           pGC->clientClip != NULL)) {
             dstregion = RegionCreate(NullBox, 0);
             RegionCopy(dstregion, srcregion);
             RegionTranslate(dstregion, dst_off_x - dx - src_off_x,
@@ -771,7 +771,7 @@ exaPolySegment(DrawablePtr pDrawable, GCPtr pGC, int nseg, xSegment * pSeg)
 
 static Bool exaFillRegionSolid(DrawablePtr pDrawable, RegionPtr pRegion,
                                Pixel pixel, CARD32 planemask, CARD32 alu,
-                               unsigned int clientClipType);
+                               Bool hasClientClip);
 
 static void
 exaPolyFillRect(DrawablePtr pDrawable, GCPtr pGC, int nrect, xRectangle *prect)
@@ -816,11 +816,11 @@ exaPolyFillRect(DrawablePtr pDrawable, GCPtr pGC, int nrect, xRectangle *prect)
         if (((pGC->fillStyle == FillSolid || pGC->tileIsPixel) &&
              exaFillRegionSolid(pDrawable, pReg, pGC->fillStyle == FillSolid ?
                                 pGC->fgPixel : pGC->tile.pixel, pGC->planemask,
-                                pGC->alu, pGC->clientClipType)) ||
+                                pGC->alu, pGC->clientClip != NULL)) ||
             (pGC->fillStyle == FillTiled && !pGC->tileIsPixel &&
              exaFillRegionTiled(pDrawable, pReg, pGC->tile.pixmap, &pGC->patOrg,
                                 pGC->planemask, pGC->alu,
-                                pGC->clientClipType))) {
+                                pGC->clientClip != NULL))) {
             goto out;
         }
     }
@@ -990,7 +990,7 @@ exaCopyWindow(WindowPtr pWin, DDXPointRec ptOldOrg, RegionPtr prgnSrc)
 
 static Bool
 exaFillRegionSolid(DrawablePtr pDrawable, RegionPtr pRegion, Pixel pixel,
-                   CARD32 planemask, CARD32 alu, unsigned int clientClipType)
+                   CARD32 planemask, CARD32 alu, Bool hasClientClip)
 {
     ExaScreenPriv(pDrawable->pScreen);
     PixmapPtr pPixmap = exaGetDrawablePixmap(pDrawable);
@@ -1013,8 +1013,7 @@ exaFillRegionSolid(DrawablePtr pDrawable, RegionPtr pRegion, Pixel pixel,
         pixmaps[0].pPix = pPixmap;
         pixmaps[0].pReg = exaGCReadsDestination(pDrawable, planemask, FillSolid,
                                                 alu,
-                                                clientClipType) ? NULL :
+                                                hasClientClip) ? NULL : pRegion;
-            pRegion;
 
         exaDoMigration(pixmaps, 1, TRUE);
     }
@@ -1074,7 +1073,7 @@ exaFillRegionSolid(DrawablePtr pDrawable, RegionPtr pRegion, Pixel pixel,
 Bool
 exaFillRegionTiled(DrawablePtr pDrawable, RegionPtr pRegion, PixmapPtr pTile,
                    DDXPointPtr pPatOrg, CARD32 planemask, CARD32 alu,
-                   unsigned int clientClipType)
+                   Bool hasClientClip)
 {
     ExaScreenPriv(pDrawable->pScreen);
     PixmapPtr pPixmap;
@@ -1096,7 +1095,7 @@ exaFillRegionTiled(DrawablePtr pDrawable, RegionPtr pRegion, PixmapPtr pTile,
     if (tileWidth == 1 && tileHeight == 1)
         return exaFillRegionSolid(pDrawable, pRegion,
                                   exaGetPixmapFirstPixel(pTile), planemask,
-                                  alu, clientClipType);
+                                  alu, hasClientClip);
 
     pPixmap = exaGetDrawablePixmap(pDrawable);
     pExaPixmap = ExaGetPixmapPriv(pPixmap);
@@ -1113,8 +1112,7 @@ exaFillRegionTiled(DrawablePtr pDrawable, RegionPtr pRegion, PixmapPtr pTile,
         pixmaps[0].pPix = pPixmap;
         pixmaps[0].pReg = exaGCReadsDestination(pDrawable, planemask, FillTiled,
                                                 alu,
-                                                clientClipType) ? NULL :
+                                                hasClientClip) ? NULL : pRegion;
-            pRegion;
         pixmaps[1].as_dst = FALSE;
         pixmaps[1].as_src = TRUE;
         pixmaps[1].pPix = pTile;

exa/exa_priv.h

@@ -455,12 +455,11 @@ ExaCheckAddTraps(PicturePtr pPicture,
 static _X_INLINE Bool
 exaGCReadsDestination(DrawablePtr pDrawable, unsigned long planemask,
                       unsigned int fillStyle, unsigned char alu,
-                      unsigned int clientClipType)
+                      Bool clientClip)
 {
     return ((alu != GXcopy && alu != GXclear && alu != GXset &&
              alu != GXcopyInverted) || fillStyle == FillStippled ||
-            clientClipType != CT_NONE ||
+            clientClip != FALSE || !EXA_PM_IS_SOLID(pDrawable, planemask));
-            !EXA_PM_IS_SOLID(pDrawable, planemask));
 }
 
 void
@@ -470,7 +469,7 @@ Bool
 
 exaFillRegionTiled(DrawablePtr pDrawable, RegionPtr pRegion, PixmapPtr pTile,
                    DDXPointPtr pPatOrg, CARD32 planemask, CARD32 alu,
-                   unsigned int clientClipType);
+                   Bool clientClip);
 
 void
 

exa/exa_unaccel.c

@@ -107,7 +107,7 @@ ExaCheckPutImage(DrawablePtr pDrawable, GCPtr pGC, int depth,
     EXA_FALLBACK(("to %p (%c)\n", pDrawable, exaDrawableLocation(pDrawable)));
     if (!pExaScr->prepare_access_reg || !pExaPixmap->pDamage ||
         exaGCReadsDestination(pDrawable, pGC->planemask, pGC->fillStyle,
-                              pGC->alu, pGC->clientClipType))
+                              pGC->alu, pGC->clientClip != NULL))
         exaPrepareAccess(pDrawable, EXA_PREPARE_DEST);
     else
         pExaScr->prepare_access_reg(pPixmap, EXA_PREPARE_DEST,
@@ -143,7 +143,7 @@ ExaCheckCopyNtoN(DrawablePtr pSrc, DrawablePtr pDst, GCPtr pGC,
 
     if (pExaScr->prepare_access_reg &&
         !exaGCReadsDestination(pDst, pGC->planemask, pGC->fillStyle,
-                               pGC->alu, pGC->clientClipType) &&
+                               pGC->alu, pGC->clientClip != NULL) &&
         RegionInitBoxes(&reg, pbox, nbox)) {
         PixmapPtr pPixmap = exaGetDrawablePixmap(pDst);
 
@@ -179,10 +179,9 @@ ExaFallbackPrepareReg(DrawablePtr pDrawable,
     ExaScreenPriv(pScreen);
 
     if (pExaScr->prepare_access_reg &&
-        !(checkReads && exaGCReadsDestination(pDrawable,
+        !(checkReads && exaGCReadsDestination(pDrawable, pGC->planemask,
-                                              pGC->planemask,
+                                              pGC->fillStyle, pGC->alu,
-                                              pGC->fillStyle,
+                                              pGC->clientClip != NULL))) {
-                                              pGC->alu, pGC->clientClipType))) {
         BoxRec box;
         RegionRec reg;
         int xoff, yoff;

hw/dmx/dmxgc.c

@@ -391,13 +391,10 @@ dmxChangeClip(GCPtr pGC, int type, void *pvalue, int nrects)
     pGC->funcs->ChangeClip(pGC, type, pvalue, nrects);
 
     /* Set the client clip on the back-end server */
-    switch (pGC->clientClipType) {
+    if (!pGC->clientClip) {
-    case CT_NONE:
         if (dmxScreen->beDisplay)
             XSetClipMask(dmxScreen->beDisplay, pGCPriv->gc, None);
-        break;
+    } else {
-
-    case CT_REGION:
         if (dmxScreen->beDisplay) {
             nRects = RegionNumRects((RegionPtr) pGC->clientClip);
             pRects = malloc(nRects * sizeof(*pRects));
@@ -416,11 +413,6 @@ dmxChangeClip(GCPtr pGC, int type, void *pvalue, int nrects)
 
             free(pRects);
         }
-        break;
-
-    case CT_PIXMAP:
-        /* Condensed down to REGION in the mi code */
-        break;
     }
 
     DMX_GC_FUNC_EPILOGUE(pGC);

hw/kdrive/src/kxv.c

@@ -492,7 +492,7 @@ static void
 KdXVCopyClip(XvPortRecPrivatePtr portPriv, GCPtr pGC)
 {
     /* copy the new clip if it exists */
-    if ((pGC->clientClipType == CT_REGION) && pGC->clientClip) {
+    if (pGC->clientClip) {
         if (!portPriv->clientClip)
             portPriv->clientClip = RegionCreate(NullBox, 1);
         /* Note: this is in window coordinates */

hw/xfree86/common/xf86xv.c

@@ -599,7 +599,7 @@ static void
 xf86XVCopyClip(XvPortRecPrivatePtr portPriv, GCPtr pGC)
 {
     /* copy the new clip if it exists */
-    if ((pGC->clientClipType == CT_REGION) && pGC->clientClip) {
+    if (pGC->clientClip) {
         if (!portPriv->clientClip)
             portPriv->clientClip = RegionCreate(NullBox, 1);
         /* Note: this is in window coordinates */

hw/xnest/GC.c

@@ -194,11 +194,12 @@ xnestChangeClip(GCPtr pGC, int type, void *pValue, int nRects)
     BoxPtr pBox;
     XRectangle *pRects;
 
-    xnestDestroyClipHelper(pGC);
+    xnestDestroyClip(pGC);
 
     switch (type) {
     case CT_NONE:
         XSetClipMask(xnestDisplay, xnestGC(pGC), None);
+        pValue = NULL;
         break;
 
     case CT_REGION:
@@ -224,11 +225,9 @@ xnestChangeClip(GCPtr pGC, int type, void *pValue, int nRects)
          * Need to change into region, so subsequent uses are with
          * current pixmap contents.
          */
-        pGC->clientClip =
+        pGC->clientClip = (*pGC->pScreen->BitmapToRegion) ((PixmapPtr) pValue);
-            (void *) (*pGC->pScreen->BitmapToRegion) ((PixmapPtr) pValue);
         (*pGC->pScreen->DestroyPixmap) ((PixmapPtr) pValue);
         pValue = pGC->clientClip;
-        type = CT_REGION;
         break;
 
     case CT_UNSORTED:
@@ -264,65 +263,34 @@ xnestChangeClip(GCPtr pGC, int type, void *pValue, int nRects)
     case CT_YSORTED:
     case CT_YXSORTED:
     case CT_YXBANDED:
-
+        /* server clip representation is a region */
-        /*
+        pGC->clientClip = RegionFromRects(nRects, (xRectangle *) pValue, type);
-         * other parts of server can only deal with CT_NONE,
-         * CT_PIXMAP and CT_REGION client clips.
-         */
-        pGC->clientClip = (void *) RegionFromRects(nRects,
-                                                    (xRectangle *) pValue,
-                                                    type);
         free(pValue);
         pValue = pGC->clientClip;
-        type = CT_REGION;
-
         break;
     }
 
-    pGC->clientClipType = type;
     pGC->clientClip = pValue;
 }
 
 void
 xnestDestroyClip(GCPtr pGC)
 {
-    xnestDestroyClipHelper(pGC);
+    if (pGC->clientClip) {
-
-    XSetClipMask(xnestDisplay, xnestGC(pGC), None);
-
-    pGC->clientClipType = CT_NONE;
-    pGC->clientClip = NULL;
-}
-
-void
-xnestDestroyClipHelper(GCPtr pGC)
-{
-    switch (pGC->clientClipType) {
-    default:
-    case CT_NONE:
-        break;
-
-    case CT_REGION:
         RegionDestroy(pGC->clientClip);
-        break;
+        XSetClipMask(xnestDisplay, xnestGC(pGC), None);
+        pGC->clientClip = NULL;
     }
 }
 
 void
 xnestCopyClip(GCPtr pGCDst, GCPtr pGCSrc)
 {
-    RegionPtr pRgn;
+    if (pGCSrc->clientClip) {
-
+        RegionPtr pRgn = RegionCreate(NULL, 1);
-    switch (pGCSrc->clientClipType) {
-    default:
-    case CT_NONE:
-        xnestDestroyClip(pGCDst);
-        break;
-
-    case CT_REGION:
-        pRgn = RegionCreate(NULL, 1);
         RegionCopy(pRgn, pGCSrc->clientClip);
         xnestChangeClip(pGCDst, CT_REGION, pRgn, 0);
-        break;
+    } else {
+        xnestDestroyClip(pGCDst);
     }
 }

hw/xnest/XNGC.h

@@ -37,7 +37,6 @@ void xnestCopyGC(GCPtr pGCSrc, unsigned long mask, GCPtr pGCDst);
 void xnestDestroyGC(GCPtr pGC);
 void xnestChangeClip(GCPtr pGC, int type, void *pValue, int nRects);
 void xnestDestroyClip(GCPtr pGC);
-void xnestDestroyClipHelper(GCPtr pGC);
 void xnestCopyClip(GCPtr pGCDst, GCPtr pGCSrc);
 
 #endif                          /* XNESTGC_H */

include/gcstruct.h

@@ -254,13 +254,12 @@ typedef struct _GC {
     unsigned int arcMode:1;
     unsigned int subWindowMode:1;
     unsigned int graphicsExposures:1;
-    unsigned int clientClipType:2;      /* CT_<kind> */
     unsigned int miTranslate:1; /* should mi things translate? */
     unsigned int tileIsPixel:1; /* tile is solid pixel */
     unsigned int fExpose:1;     /* Call exposure handling */
     unsigned int freeCompClip:1;        /* Free composite clip */
     unsigned int scratch_inuse:1;       /* is this GC in a pool for reuse? */
-    unsigned int unused:13;     /* see comment above */
+    unsigned int unused:15;     /* see comment above */
     unsigned long planemask;
     unsigned long fgPixel;
     unsigned long bgPixel;
@@ -273,7 +272,7 @@ typedef struct _GC {
     DDXPointRec patOrg;         /* origin for (tile, stipple) */
     struct _Font *font;
     DDXPointRec clipOrg;
-    void *clientClip;
+    RegionPtr clientClip;
     unsigned long stateChanges; /* masked with GC_<kind> */
     unsigned long serialNumber;
     const GCFuncs *funcs;

mi/micopy.c

@@ -167,7 +167,7 @@ miDoCopy(DrawablePtr pSrcDrawable,
 
     /* Compute source clip region */
     if (pSrcDrawable->type == DRAWABLE_PIXMAP) {
-        if ((pSrcDrawable == pDstDrawable) && (pGC->clientClipType == CT_NONE))
+        if ((pSrcDrawable == pDstDrawable) && (!pGC->clientClip))
             prgnSrcClip = miGetCompositeClip(pGC);
         else
             fastSrc = TRUE;
@@ -186,8 +186,7 @@ miDoCopy(DrawablePtr pSrcDrawable,
                  */
                 fastSrc = TRUE;
             }
-            else if ((pSrcDrawable == pDstDrawable) &&
+            else if ((pSrcDrawable == pDstDrawable) && (!pGC->clientClip)) {
-                     (pGC->clientClipType == CT_NONE)) {
                 prgnSrcClip = miGetCompositeClip(pGC);
             }
             else {

mi/miexpose.c

@@ -230,7 +230,7 @@ miHandleExposures(DrawablePtr pSrcDrawable, DrawablePtr pDstDrawable,
     RegionIntersect(&rgnExposed, &rgnExposed, prgnDstClip);
 
     /* intersect with client clip region. */
-    if (pGC->clientClipType == CT_REGION)
+    if (pGC->clientClip)
         RegionIntersect(&rgnExposed, &rgnExposed, pGC->clientClip);
 
     /*

mi/migc.c

@@ -55,20 +55,9 @@ miDestroyGC(GCPtr pGC)
 void
 miDestroyClip(GCPtr pGC)
 {
-    if (pGC->clientClipType == CT_NONE)
+    if (pGC->clientClip)
-        return;
-    else if (pGC->clientClipType == CT_PIXMAP) {
-        (*pGC->pScreen->DestroyPixmap) ((PixmapPtr) (pGC->clientClip));
-    }
-    else {
-        /*
-         * we know we'll never have a list of rectangles, since ChangeClip
-         * immediately turns them into a region
-         */
         RegionDestroy(pGC->clientClip);
-    }
     pGC->clientClip = NULL;
-    pGC->clientClipType = CT_NONE;
 }
 
 void
@@ -77,8 +66,7 @@ miChangeClip(GCPtr pGC, int type, void *pvalue, int nrects)
     (*pGC->funcs->DestroyClip) (pGC);
     if (type == CT_PIXMAP) {
         /* convert the pixmap to a region */
-        pGC->clientClip = (void *) BitmapToRegion(pGC->pScreen,
+        pGC->clientClip = BitmapToRegion(pGC->pScreen, (PixmapPtr) pvalue);
-                                                   (PixmapPtr) pvalue);
         (*pGC->pScreen->DestroyPixmap) (pvalue);
     }
     else if (type == CT_REGION) {
@@ -86,34 +74,21 @@ miChangeClip(GCPtr pGC, int type, void *pvalue, int nrects)
         pGC->clientClip = pvalue;
     }
     else if (type != CT_NONE) {
-        pGC->clientClip = (void *) RegionFromRects(nrects,
+        pGC->clientClip = RegionFromRects(nrects, (xRectangle *) pvalue, type);
-                                                    (xRectangle *) pvalue,
-                                                    type);
         free(pvalue);
     }
-    pGC->clientClipType = (type != CT_NONE &&
-                           pGC->clientClip) ? CT_REGION : CT_NONE;
     pGC->stateChanges |= GCClipMask;
 }
 
 void
 miCopyClip(GCPtr pgcDst, GCPtr pgcSrc)
 {
-    RegionPtr prgnNew;
+    if (pgcSrc->clientClip) {
-
+        RegionPtr prgnNew = RegionCreate(NULL, 1);
-    switch (pgcSrc->clientClipType) {
-    case CT_PIXMAP:
-        ((PixmapPtr) pgcSrc->clientClip)->refcnt++;
-        /* Fall through !! */
-    case CT_NONE:
-        (*pgcDst->funcs->ChangeClip) (pgcDst, (int) pgcSrc->clientClipType,
-                                      pgcSrc->clientClip, 0);
-        break;
-    case CT_REGION:
-        prgnNew = RegionCreate(NULL, 1);
         RegionCopy(prgnNew, (RegionPtr) (pgcSrc->clientClip));
-        (*pgcDst->funcs->ChangeClip) (pgcDst, CT_REGION, (void *) prgnNew, 0);
+        (*pgcDst->funcs->ChangeClip) (pgcDst, CT_REGION, prgnNew, 0);
-        break;
+    } else {
+        (*pgcDst->funcs->ChangeClip) (pgcDst, CT_NONE, NULL, 0);
     }
 }
 
@@ -149,7 +124,7 @@ miComputeCompositeClip(GCPtr pGC, DrawablePtr pDrawable)
          * regions.  (this wins especially if many clients clip by children
          * and have no client clip.)
          */
-        if (pGC->clientClipType == CT_NONE) {
+        if (!pGC->clientClip) {
             if (freeCompClip)
                 RegionDestroy(pGC->pCompositeClip);
             pGC->pCompositeClip = pregWin;
@@ -206,7 +181,7 @@ miComputeCompositeClip(GCPtr pGC, DrawablePtr pDrawable)
             pGC->pCompositeClip = RegionCreate(&pixbounds, 1);
         }
 
-        if (pGC->clientClipType == CT_REGION) {
+        if (pGC->clientClip) {
             if (pDrawable->x || pDrawable->y) {
                 RegionTranslate(pGC->clientClip,
                                 pDrawable->x + pGC->clipOrg.x,

mi/mioverlay.c

@@ -1667,7 +1667,7 @@ miOverlayComputeCompositeClip(GCPtr pGC, WindowPtr pWin)
         freeTmpClip = FALSE;
     }
     freeCompClip = pGC->freeCompClip;
-    if (pGC->clientClipType == CT_NONE) {
+    if (!pGC->clientClip) {
         if (freeCompClip)
             RegionDestroy(pGC->pCompositeClip);
         pGC->pCompositeClip = pregWin;

xfixes/region.c

@@ -222,20 +222,13 @@ ProcXFixesCreateRegionFromGC(ClientPtr client)
     if (rc != Success)
         return rc;
 
-    switch (pGC->clientClipType) {
+    if (pGC->clientClip) {
-    case CT_PIXMAP:
-        pRegion = BitmapToRegion(pGC->pScreen, (PixmapPtr) pGC->clientClip);
-        if (!pRegion)
-            return BadAlloc;
-        break;
-    case CT_REGION:
         pClip = (RegionPtr) pGC->clientClip;
         pRegion = XFixesRegionCopy(pClip);
         if (!pRegion)
             return BadAlloc;
-        break;
+    } else {
-    default:
+        return BadMatch;
-        return BadImplementation;       /* assume sane server bits */
     }
 
     if (!AddResource(stuff->region, RegionResType, (void *) pRegion))