[PATCH resend 2/3] Make GC clientClip always be a region.

[Jamey Sharp]

On Wed, Sep 28, 2011 at 11:48:38AM -0700, Aaron Plattner wrote:
> On 09/26/2011 10:53 PM, Jamey Sharp wrote:
> >-      for (i = nRects; i-->  0; ) {
> >...
> >+    for (i = nRects; i-->  0; ) {
> 
> Whitespace error: "i-->  0"

The patch got mangled somewhere in transit. In my git tree, and in
Patchwork, it is correct: "i-- > 0".

http://patchwork.freedesktop.org/patch/7362/

Your reply has more places where whitespace mysteriously moved around
operators, too...

> >+    free((char *) pRects);
> 
> While you're at it, could you remove this cast please?

I didn't really want to make unrelated changes, but I sympathize, so
I've amended this locally.

> >+  } else
> >+    XSetClipMask(xnestDisplay, xnestGC(pGC), None);
> 
> I'm wavering between asking you to fix the indentation since you're
> replacing the whole function anyway, but it would make the file
> inconsistent.  Better to change it all at one go, I guess.

Yeah, I don't think that belongs in this commit. Actually, I want to
delete Xnest entirely, along with all the other non-xfree86 DDXes. :-)

> >diff --git a/include/gcstruct.h b/include/gcstruct.h
> >index fb9ee0d..ed598fc 100644
> >--- a/include/gcstruct.h
> >+++ b/include/gcstruct.h
> >@@ -278,13 +278,12 @@ typedef struct _GC {
> >      unsigned int       arcMode : 1;
> >      unsigned int       subWindowMode : 1;
> >      unsigned int       graphicsExposures : 1;
> >-    unsigned int       clientClipType : 2; /* CT_<kind>  */
> 
> This leaves a misleading comment in gc.h:
> 
> /* clientClipType field in GC */
> #define CT_NONE                 0
> #define CT_PIXMAP               1
> #define CT_REGION               2
> [...]

True. I've amended the following patch locally; does it help?

diff --git a/include/gc.h b/include/gc.h
index 2079cfa..a28b419 100644
--- a/include/gc.h
+++ b/include/gc.h
@@ -54,7 +54,7 @@ SOFTWARE.
 #include "screenint.h"	/* for ScreenPtr */
 #include "pixmap.h"	/* for DrawablePtr */
 
-/* clientClipType field in GC */
+/* clip type argument for ChangeClip */
 #define CT_NONE			0
 #define CT_PIXMAP		1
 #define CT_REGION		2

The defines are also used for the clientClipType field in the Picture
structure, which might call for a similar patch someday, but I don't
think that calls for a mention in gc.h.

Thanks,
Jamey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110928/1f7a3b26/attachment-0001.pgp>