[Bug 3486474]: Inconsistent color scaling

13 files changed, 115 insertions, 31 deletions

diff --git a/ChangeLog b/ChangeLog
index 49fffe4..ea77f36 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2012-02-15  Jan Nijtmans  <nijtmans@users.sf.net>
+
+	* xlib/xcolors.c: [Bug 3486474]: Inconsistent color scaling
+	* generic/tkColor.c: new internal function TkParseColor
+	* generic/tkInt.h:
+	* generic/tk*.c:   Change XParseColor() to TkParseColor() everywhere.
+
 2012-02-10  Donal K. Fellows  <dkf@users.sf.net>
 
 	* win/tkWinDialog.c (GetFileNameW): Ensure that we do not convert a
diff --git a/doc/GetColor.3 b/doc/GetColor.3
index b5416af..25fe16e 100644
--- a/doc/GetColor.3
+++ b/doc/GetColor.3
@@ -87,8 +87,9 @@ to use to display the color.  Each \fIR\fR, \fIG\fR, or \fIB\fR
 represents a single hexadecimal digit.  The four forms permit
 colors to be specified with 4-bit, 8-bit, 12-bit or 16-bit values.
 When fewer than 16 bits are provided for each color, they represent
-the most significant bits of the color.  For example, #3a7 is the
-same as #3000a0007000.
+the most significant bits of the color, while the lower unfilled
+bits will be repeatedly replicated from the available higher bits.
+For example, #3a7 is the same as #3333aaaa7777.
 .PP
 \fBTk_AllocColorFromObj\fR returns a pointer to
 an XColor structure;  the structure indicates the exact intensities of
diff --git a/generic/tkColor.c b/generic/tkColor.c
index 6836c52..ebfcf22 100644
--- a/generic/tkColor.c
+++ b/generic/tkColor.c
@@ -798,7 +798,61 @@ TkDebugColor(
     }
     return resultPtr;
 }
-
+
+#ifndef __WIN32__
+/* This function is not necessary for Win32,
+ * since XParseColor already does the right thing */
+Status
+TkParseColor(
+    Display * display,		/* The display */
+    Colormap map,			/* Color map */
+    _Xconst char* spec,     /* String to be parsed */
+    XColor * colorPtr)
+{
+    if (*spec == '#') {
+    char buf[14];
+    buf[0] = '#'; buf[13] = '\0';
+	if (!*(++spec) || !*(++spec) || !*(++spec)) {
+	/* Not at least 3 hex digits, so invalid */
+	return 0;
+	} else if (!*(++spec)) {
+	/* Exactly 3 hex digits */
+	buf[9] = buf[10] = buf[11] = buf[12] = *(--spec);
+	buf[5] = buf[6] = buf[7] = buf[8] = *(--spec);
+	buf[1] = buf[2] = buf[3] = buf[4] = *(--spec);
+	spec = buf;
+	} else if (!*(++spec)	|| !*(++spec)) {
+	/* Not at least 6 hex digits, so invalid */
+	return 0;
+	} else if (!*(++spec)) {
+	/* Exactly 6 hex digits */
+	buf[10] = buf[12] = *(--spec);
+	buf[9] = buf[11] = *(--spec);
+	buf[6] = buf[8] = *(--spec);
+	buf[5] = buf[7] = *(--spec);
+	buf[2] = buf[4] = *(--spec);
+	buf[1] = buf[3] = *(--spec);
+	spec = buf;
+	} else if (!*(++spec) || !*(++spec)) {
+	/* Not at least 9 hex digits, so invalid */
+	return 0;
+	} else if (!*(++spec)) {
+	/* Exactly 9 hex digits */
+	buf[11] = *(--spec);
+	buf[10] = *(--spec);
+	buf[9] = buf[12] = *(--spec);
+	buf[7] = *(--spec);
+	buf[6] = *(--spec);
+	buf[5] = buf[8] = *(--spec);
+	buf[3] = *(--spec);
+	buf[2] = *(--spec);
+	buf[1] = buf[4] = *(--spec);
+	spec = buf;
+	}
+    }
+    return XParseColor(display, map, spec, colorPtr);
+}
+#endif /* __WIN32__ */
 /*
  * Local Variables:
  * mode: c
diff --git a/generic/tkCursor.c b/generic/tkCursor.c
index 606a349..410aea9 100644
--- a/generic/tkCursor.c
+++ b/generic/tkCursor.c
@@ -349,11 +349,11 @@ Tk_GetCursorFromData(
      * add it to the database.
      */
 
-    if (XParseColor(dataKey.display, Tk_Colormap(tkwin), fg, &fgColor) == 0) {
+    if (TkParseColor(dataKey.display, Tk_Colormap(tkwin), fg, &fgColor) == 0) {
 	Tcl_AppendResult(interp, "invalid color name \"", fg, "\"", NULL);
 	goto error;
     }
-    if (XParseColor(dataKey.display, Tk_Colormap(tkwin), bg, &bgColor) == 0) {
+    if (TkParseColor(dataKey.display, Tk_Colormap(tkwin), bg, &bgColor) == 0) {
 	Tcl_AppendResult(interp, "invalid color name \"", bg, "\"", NULL);
 	goto error;
     }
diff --git a/generic/tkImgBmap.c b/generic/tkImgBmap.c
index 88c96d5..8598e88 100644
--- a/generic/tkImgBmap.c
+++ b/generic/tkImgBmap.c
@@ -1247,7 +1247,7 @@ ImgBmapPostscript(
     if ((masterPtr->bgUid != NULL) && (masterPtr->bgUid[0] != '\000')) {
 	XColor color;
 
-	XParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin), masterPtr->bgUid,
+	TkParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin), masterPtr->bgUid,
 		&color);
 	if (Tk_PostscriptColor(interp, psinfo, &color) != TCL_OK) {
 	    return TCL_ERROR;
@@ -1269,7 +1269,7 @@ ImgBmapPostscript(
     if ((masterPtr->fgUid != NULL) && (masterPtr->data != NULL)) {
 	XColor color;
 
-	XParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin), masterPtr->fgUid,
+	TkParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin), masterPtr->fgUid,
 		&color);
 	if (Tk_PostscriptColor(interp, psinfo, &color) != TCL_OK) {
 	    return TCL_ERROR;
diff --git a/generic/tkImgPhoto.c b/generic/tkImgPhoto.c
index 6075da8..ffff4fc 100644
--- a/generic/tkImgPhoto.c
+++ b/generic/tkImgPhoto.c
@@ -1160,7 +1160,7 @@ ImgPhotoCmd(
 		    }
 		}
 
-		if (!XParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin),
+		if (!TkParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin),
 			colorString, &color)) {
 		    Tcl_AppendResult(interp, "can't parse color \"",
 			    colorString, "\"", NULL);
diff --git a/generic/tkInt.h b/generic/tkInt.h
index bd40a88..42ef495 100644
--- a/generic/tkInt.h
+++ b/generic/tkInt.h
@@ -1179,6 +1179,13 @@ MODULE_SCOPE void	TkUnderlineCharsInContext(Display *display,
 			    int firstByte, int lastByte);
 MODULE_SCOPE void	TkpGetFontAttrsForChar(Tk_Window tkwin, Tk_Font tkfont,
 			    Tcl_UniChar c, struct TkFontAttributes *faPtr);
+#ifdef __WIN32__
+#define TkParseColor XParseColor
+#else
+MODULE_SCOPE Status TkParseColor (Display * display,
+				Colormap map, _Xconst char* spec,
+				XColor * colorPtr);
+#endif
 
 /*
  * Unsupported commands.
diff --git a/generic/tkStubInit.c b/generic/tkStubInit.c
index 9c154e8..170ada7 100644
--- a/generic/tkStubInit.c
+++ b/generic/tkStubInit.c
@@ -45,6 +45,12 @@ MODULE_SCOPE TkIntXlibStubs tkIntXlibStubs;
 MODULE_SCOPE TkPlatStubs tkPlatStubs;
 MODULE_SCOPE TkStubs tkStubs;
 
+#ifndef __WIN32__
+/* Make sure that extensions which call XParseColor through
+ * the stub table, call TkParseColor in stead. See bug #3486474 */
+#   define XParseColor TkParseColor
+#endif
+
 /*
  * WARNING: The contents of this file is automatically generated by the
  * tools/genStubs.tcl script. Any modifications to the function declarations
diff --git a/macosx/tkMacOSXColor.c b/macosx/tkMacOSXColor.c
index b6b1d18..359c8a6 100644
--- a/macosx/tkMacOSXColor.c
+++ b/macosx/tkMacOSXColor.c
@@ -591,7 +591,7 @@ TkpGetColor(
 	}
     }
 
-    if (XParseColor(display, colormap, name, &color) == 0) {
+    if (TkParseColor(display, colormap, name, &color) == 0) {
 	return (TkColor *) NULL;
     }
 
diff --git a/unix/tkUnixColor.c b/unix/tkUnixColor.c
index e9fdf11..d3dba23 100644
--- a/unix/tkUnixColor.c
+++ b/unix/tkUnixColor.c
@@ -166,7 +166,7 @@ TkpGetColor(
 	    FindClosestColor(tkwin, &screen, &color);
 	}
     } else {
-	if (XParseColor(display, colormap, buf, &color) == 0) {
+	if (TkParseColor(display, colormap, buf, &color) == 0) {
 	    return NULL;
 	}
 	if (XAllocColor(display, colormap, &color) != 0) {
diff --git a/unix/tkUnixCursor.c b/unix/tkUnixCursor.c
index 2169d1c..1ab238e 100644
--- a/unix/tkUnixCursor.c
+++ b/unix/tkUnixCursor.c
@@ -274,7 +274,7 @@ TkGetCursorByName(
 	    fg.red = fg.green = fg.blue = 0;
 	    bg.red = bg.green = bg.blue = 65535;
 	} else {
-	    if (XParseColor(display, Tk_Colormap(tkwin), argv[1], &fg) == 0) {
+	    if (TkParseColor(display, Tk_Colormap(tkwin), argv[1], &fg) == 0) {
 		Tcl_AppendResult(interp, "invalid color name \"", argv[1],
 			"\"", NULL);
 		goto cleanup;
@@ -282,7 +282,7 @@ TkGetCursorByName(
 	    if (argc == 2) {
 		bg.red = bg.green = bg.blue = 0;
 		maskIndex = namePtr->shape;
-	    } else if (XParseColor(display, Tk_Colormap(tkwin), argv[2],
+	    } else if (TkParseColor(display, Tk_Colormap(tkwin), argv[2],
 		    &bg) == 0) {
 		Tcl_AppendResult(interp, "invalid color name \"", argv[2],
 			"\"", NULL);
@@ -456,7 +456,7 @@ CreateCursorFromTableOrFile(
 	bg.red = bg.green = bg.blue = 65535;
     } else if (argc == 2) {
 	fgColor = argv[1];
-	if (XParseColor(display, Tk_Colormap(tkwin), fgColor, &fg) == 0) {
+	if (TkParseColor(display, Tk_Colormap(tkwin), fgColor, &fg) == 0) {
 	    Tcl_AppendResult(interp, "invalid color name \"",
 		    fgColor, "\"", NULL);
 	    goto cleanup;
@@ -475,12 +475,12 @@ CreateCursorFromTableOrFile(
 	    fgColor = argv[2];
 	    bgColor = argv[3];
 	}
-	if (XParseColor(display, Tk_Colormap(tkwin), fgColor, &fg) == 0) {
+	if (TkParseColor(display, Tk_Colormap(tkwin), fgColor, &fg) == 0) {
 	    Tcl_AppendResult(interp, "invalid color name \"",
 		    fgColor, "\"", NULL);
 	    goto cleanup;
 	}
-	if (XParseColor(display, Tk_Colormap(tkwin), bgColor, &bg) == 0) {
+	if (TkParseColor(display, Tk_Colormap(tkwin), bgColor, &bg) == 0) {
 	    Tcl_AppendResult(interp, "invalid color name \"",
 		    bgColor, "\"", NULL);
 	    goto cleanup;
diff --git a/win/tkWinColor.c b/win/tkWinColor.c
index 234c736..f52f6ed 100644
--- a/win/tkWinColor.c
+++ b/win/tkWinColor.c
@@ -200,7 +200,7 @@ TkpGetColor(
 
     if (((strncasecmp(name, "system", 6) == 0)
 	    && FindSystemColor(name+6, &color, &index))
-	    || XParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin), name,
+	    || TkParseColor(Tk_Display(tkwin), Tk_Colormap(tkwin), name,
 		    &color)) {
 	winColPtr = (WinColor *) ckalloc(sizeof(WinColor));
 	winColPtr->info.color = color;
diff --git a/xlib/xcolors.c b/xlib/xcolors.c
index ad33b7b..b1d1f27 100644
--- a/xlib/xcolors.c
+++ b/xlib/xcolors.c
@@ -879,24 +879,33 @@ XParseColor(
     XColor *colorPtr)
 {
     if (spec[0] == '#') {
-	char fmt[16];
-	int i, red, green, blue;
+	char *p;
+	Tcl_WideInt value = _strtoi64(++spec, &p, 16);
 
-	if ((i = (int) strlen(spec+1))%3) {
-	    return 0;
-	}
-	i /= 3;
-
-	sprintf(fmt, "%%%dx%%%dx%%%dx", i, i, i);
-	if (sscanf(spec+1, fmt, &red, &green, &blue) != 3) {
+	switch ((int)(p-spec)) {
+	case 3:
+	    colorPtr->red = (unsigned short) (((value >> 8) & 0xf) * 0x1111);
+	    colorPtr->green = (unsigned short) (((value >> 4) & 0xf) * 0x1111);
+	    colorPtr->blue = (unsigned short) ((value & 0xf) * 0x1111);
+	    break;
+	case 6:
+	    colorPtr->red = (unsigned short) (((value >> 16) & 0xff) | ((value >> 8) & 0xff00));
+	    colorPtr->green = (unsigned short) (((value >> 8) & 0xff) | (value & 0xff00));
+	    colorPtr->blue = (unsigned short) ((value & 0xff) | (value << 8));
+	    break;
+	case 9:
+	    colorPtr->red = (unsigned short) (((value >> 32) & 0xf) | ((value >> 20) & 0xfff0));
+	    colorPtr->green = (unsigned short) (((value >> 20) & 0xf) | ((value >> 8) & 0xfff0));
+	    colorPtr->blue = (unsigned short) (((value >> 8) & 0xf) | (value << 4));
+	    break;
+	case 12:
+	    colorPtr->red = (unsigned short) (value >> 32);
+	    colorPtr->green = (unsigned short) (value >> 16);
+	    colorPtr->blue = (unsigned short) value;
+	    break;
+	default:
 	    return 0;
 	}
-	colorPtr->red = (((unsigned short) red) << (4 * (4 - i)))
-		| ((unsigned short) red);
-	colorPtr->green = (((unsigned short) green) << (4 * (4 - i)))
-		| ((unsigned short) green);
-	colorPtr->blue = (((unsigned short) blue) << (4 * (4 - i)))
-		| ((unsigned short) blue);
     } else {
 	if (!FindColor(spec, colorPtr)) {
 	    return 0;