On 09/28/2011 12:58 PM, Jamey Sharp wrote:
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...
Aargh, you're right. I blame MS Exchange -- it converts incoming utf-8
messages to base64 and breaks them in the process, so I wonder if it did
something similar to your patch. Did you send it with
Content-Transfer-Encoding: quoted-printable, or is that Exchange's dirty
work too?
I really need to subscribe from my GMail account 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. :-)
Yeah, if xf86-video-nested actually works, I'm on board with that. (I
haven't tested it).
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
Yes, thanks.
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.
Agreed.
Reviewed-by: Aaron Plattner <[email protected]>
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
