From: Mattias Engdegård Date: Fri, 12 Jun 2020 16:12:37 +0000 (+0200) Subject: Consolidate #RGB string parsers X-Git-Tag: emacs-28.0.90~7120 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=9fe2bdb88a4ebd4b2286c1c2a2a2ba7411af01b6;p=emacs.git Consolidate #RGB string parsers Use a single parser of color strings in the #RGB, rgb:R/G/B and rgbi:R/G/B formats, replacing four existing ones. Previously, error-checking was spotty, handling of the rgbi: format not always present, and normalization of the result was sometimes incorrect. * src/dispextern.h: New prototype. * src/xfaces.c (parse_hex_color_comp, parse_float_color_comp) (parse_color_spec, Finternal-color_values_from_color_spec): New functions. * test/src/xfaces-tests.el (xfaces-internal-color-values-from-color-spec): New test. * lisp/term/tty-colors.el (tty-color-standard-values): Use internal-color-values-from-color-spec, replacing old parser. * src/nsterm.m (ns_get_color): * src/w32fns.c (x_to_w32_color): * src/xterm.c (x_parse_color): Use parse_color_spec, replacing old parsers. (HEX_COLOR_NAME_LENGTH): Remove #define. --- diff --git a/lisp/term/tty-colors.el b/lisp/term/tty-colors.el index 39ca2d36276..73e2431822e 100644 --- a/lisp/term/tty-colors.el +++ b/lisp/term/tty-colors.el @@ -923,62 +923,8 @@ The returned value reflects the standard Emacs definition of COLOR (see the info node `(emacs) Colors'), regardless of whether the terminal can display it, so the return value should be the same regardless of what display is being used." - (let ((len (length color))) - (cond ((and (>= len 4) ;; HTML/CSS/SVG-style "#XXYYZZ" color spec - (eq (aref color 0) ?#) - (member (aref color 1) - '(?0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9 - ?a ?b ?c ?d ?e ?f - ?A ?B ?C ?D ?E ?F))) - ;; Translate the string "#XXYYZZ" into a list of numbers - ;; (XX YY ZZ), scaling each to the {0..65535} range. This - ;; follows the HTML color convention, where both "#fff" and - ;; "#ffffff" represent the same color, white. - (let* ((ndig (/ (- len 1) 3)) - (maxval (1- (ash 1 (* 4 ndig)))) - (i1 1) - (i2 (+ i1 ndig)) - (i3 (+ i2 ndig)) - (i4 (+ i3 ndig))) - (list - (/ (* (string-to-number - (substring color i1 i2) 16) - 65535) - maxval) - (/ (* (string-to-number - (substring color i2 i3) 16) - 65535) - maxval) - (/ (* (string-to-number - (substring color i3 i4) 16) - 65535) - maxval)))) - ((and (>= len 9) ;; X-style rgb:xx/yy/zz color spec - (string= (substring color 0 4) "rgb:")) - ;; Translate the string "rgb:XX/YY/ZZ" into a list of - ;; numbers (XX YY ZZ), scaling each to the {0..65535} - ;; range. "rgb:F/F/F" is white. - (let* ((ndig (/ (- len 3) 3)) - (maxval (1- (ash 1 (* 4 (- ndig 1))))) - (i1 4) - (i2 (+ i1 ndig)) - (i3 (+ i2 ndig)) - (i4 (+ i3 ndig))) - (list - (/ (* (string-to-number - (substring color i1 (- i2 1)) 16) - 65535) - maxval) - (/ (* (string-to-number - (substring color i2 (- i3 1)) 16) - 65535) - maxval) - (/ (* (string-to-number - (substring color i3 (1- i4)) 16) - 65535) - maxval)))) - (t - (cdr (assoc color color-name-rgb-alist)))))) + (or (internal-color-values-from-color-spec color) + (cdr (assoc color color-name-rgb-alist)))) (defun tty-color-translate (color &optional frame) "Given a color COLOR, return the index of the corresponding TTY color. diff --git a/src/dispextern.h b/src/dispextern.h index 0b1f3d14aeb..e1d6eddc419 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -3514,6 +3514,8 @@ void update_face_from_frame_parameter (struct frame *, Lisp_Object, Lisp_Object); extern bool tty_defined_color (struct frame *, const char *, Emacs_Color *, bool, bool); +bool parse_color_spec (const char *, + unsigned short *, unsigned short *, unsigned short *); Lisp_Object tty_color_name (struct frame *, int); void clear_face_cache (bool); diff --git a/src/nsterm.m b/src/nsterm.m index 3dc7e1db7c9..0e405fc0175 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -2341,9 +2341,6 @@ ns_get_color (const char *name, NSColor **col) See https://lists.gnu.org/r/emacs-devel/2009-07/msg01203.html. */ { NSColor *new = nil; - static char hex[20]; - int scaling = 0; - float r = -1.0, g, b; NSString *nsname = [NSString stringWithUTF8String: name]; NSTRACE ("ns_get_color(%s, **)", name); @@ -2386,51 +2383,31 @@ ns_get_color (const char *name, NSColor **col) } /* First, check for some sort of numeric specification. */ - hex[0] = '\0'; - - if (name[0] == '0' || name[0] == '1' || name[0] == '.') /* RGB decimal */ + unsigned short r16, g16, b16; + if (parse_color_spec (name, &r16, &g16, &b16)) { - NSScanner *scanner = [NSScanner scannerWithString: nsname]; - [scanner scanFloat: &r]; - [scanner scanFloat: &g]; - [scanner scanFloat: &b]; - } - else if (!strncmp(name, "rgb:", 4)) /* A newer X11 format -- rgb:r/g/b */ - scaling = (snprintf (hex, sizeof hex, "%s", name + 4) - 2) / 3; - else if (name[0] == '#') /* An old X11 format; convert to newer */ - { - int len = 0; - while (isxdigit (name[len + 1])) - len++; - if (name[len + 1] == '\0' && len >= 1 && len <= 12 && len % 3 == 0) - { - scaling = len / 3; - for (int i = 0; i < 3; i++) - sprintf (hex + i * (scaling + 1), "%.*s/", scaling, - name + 1 + i * scaling); - hex[3 * (scaling + 1) - 1] = '\0'; - } + *col = [NSColor colorForEmacsRed: r16 / 65535.0 + green: g16 / 65535.0 + blue: b16 / 65535.0 + alpha: 1.0]; + unblock_input (); + return 0; } - - if (hex[0]) + else if (name[0] == '0' || name[0] == '1' || name[0] == '.') { - unsigned int rr, gg, bb; - float fscale = (1 << (scaling * 4)) - 1; - if (sscanf (hex, "%x/%x/%x", &rr, &gg, &bb)) + /* RGB decimal */ + NSScanner *scanner = [NSScanner scannerWithString: nsname]; + float r, g, b; + if ( [scanner scanFloat: &r] && r >= 0 && r <= 1 + && [scanner scanFloat: &g] && g >= 0 && g <= 1 + && [scanner scanFloat: &b] && b >= 0 && b <= 1) { - r = rr / fscale; - g = gg / fscale; - b = bb / fscale; + *col = [NSColor colorForEmacsRed: r green: g blue: b alpha: 1.0]; + unblock_input (); + return 0; } } - if (r >= 0.0F) - { - *col = [NSColor colorForEmacsRed: r green: g blue: b alpha: 1.0]; - unblock_input (); - return 0; - } - /* Otherwise, color is expected to be from a list */ { NSEnumerator *lenum, *cenum; diff --git a/src/w32fns.c b/src/w32fns.c index e595b0285a7..ab864332e78 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -864,161 +864,14 @@ x_to_w32_color (const char * colorname) block_input (); - if (colorname[0] == '#') + unsigned short r, g, b; + if (parse_color_spec (colorname, &r, &g, &b)) { - /* Could be an old-style RGB Device specification. */ - int size = strlen (colorname + 1); - char *color = alloca (size + 1); - - strcpy (color, colorname + 1); - if (size == 3 || size == 6 || size == 9 || size == 12) - { - UINT colorval; - int i, pos; - pos = 0; - size /= 3; - colorval = 0; - - for (i = 0; i < 3; i++) - { - char *end; - char t; - unsigned long value; - - /* The check for 'x' in the following conditional takes into - account the fact that strtol allows a "0x" in front of - our numbers, and we don't. */ - if (!isxdigit (color[0]) || color[1] == 'x') - break; - t = color[size]; - color[size] = '\0'; - value = strtoul (color, &end, 16); - color[size] = t; - if (errno == ERANGE || end - color != size) - break; - switch (size) - { - case 1: - value = value * 0x10; - break; - case 2: - break; - case 3: - value /= 0x10; - break; - case 4: - value /= 0x100; - break; - } - colorval |= (value << pos); - pos += 0x8; - if (i == 2) - { - unblock_input (); - XSETINT (ret, colorval); - return ret; - } - color = end; - } - } - } - else if (strnicmp (colorname, "rgb:", 4) == 0) - { - const char *color; - UINT colorval; - int i, pos; - pos = 0; - - colorval = 0; - color = colorname + 4; - for (i = 0; i < 3; i++) - { - char *end; - unsigned long value; - - /* The check for 'x' in the following conditional takes into - account the fact that strtol allows a "0x" in front of - our numbers, and we don't. */ - if (!isxdigit (color[0]) || color[1] == 'x') - break; - value = strtoul (color, &end, 16); - if (errno == ERANGE) - break; - switch (end - color) - { - case 1: - value = value * 0x10 + value; - break; - case 2: - break; - case 3: - value /= 0x10; - break; - case 4: - value /= 0x100; - break; - default: - value = ULONG_MAX; - } - if (value == ULONG_MAX) - break; - colorval |= (value << pos); - pos += 0x8; - if (i == 2) - { - if (*end != '\0') - break; - unblock_input (); - XSETINT (ret, colorval); - return ret; - } - if (*end != '/') - break; - color = end + 1; - } + unblock_input (); + /* Throw away the low 8 bits and return 0xBBGGRR. */ + return make_fixnum ((b & 0xff00) << 8 | (g & 0xff00) | r >> 8); } - else if (strnicmp (colorname, "rgbi:", 5) == 0) - { - /* This is an RGB Intensity specification. */ - const char *color; - UINT colorval; - int i, pos; - pos = 0; - - colorval = 0; - color = colorname + 5; - for (i = 0; i < 3; i++) - { - char *end; - double value; - UINT val; - value = strtod (color, &end); - if (errno == ERANGE) - break; - if (value < 0.0 || value > 1.0) - break; - val = (UINT)(0x100 * value); - /* We used 0x100 instead of 0xFF to give a continuous - range between 0.0 and 1.0 inclusive. The next statement - fixes the 1.0 case. */ - if (val == 0x100) - val = 0xFF; - colorval |= (val << pos); - pos += 0x8; - if (i == 2) - { - if (*end != '\0') - break; - unblock_input (); - XSETINT (ret, colorval); - return ret; - } - if (*end != '/') - break; - color = end + 1; - } - } /* I am not going to attempt to handle any of the CIE color schemes or TekHVC, since I don't know the algorithms for conversion to RGB. */ diff --git a/src/xfaces.c b/src/xfaces.c index cf155288bd1..308509a0267 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -220,6 +220,7 @@ along with GNU Emacs. If not, see . */ #include "sysstdio.h" #include #include +#include #include "lisp.h" #include "character.h" @@ -819,6 +820,120 @@ load_pixmap (struct frame *f, Lisp_Object name) Color Handling ***********************************************************************/ +/* Parse hex color component at S ending right before E. + Set *DST to the value normalized so that the maximum for the + number of digits given becomes 65535, and return true on success, + false otherwise. */ +static bool +parse_hex_color_comp (const char *s, const char *e, unsigned short *dst) +{ + int n = e - s; + if (n <= 0 || n > 4) + return false; + int val = 0; + for (; s < e; s++) + { + int digit; + if (*s >= '0' && *s <= '9') + digit = *s - '0'; + else if (*s >= 'A' && *s <= 'F') + digit = *s - 'A' + 10; + else if (*s >= 'a' && *s <= 'f') + digit = *s - 'a' + 10; + else + return false; + val = (val << 4) | digit; + } + int maxval = (1 << (n * 4)) - 1; + *dst = (unsigned)val * 65535 / maxval; + return true; +} + +/* Parse floating-point color component at S ending right before E. + Return the number if in the range [0,1]; otherwise -1. */ +static double +parse_float_color_comp (const char *s, const char *e) +{ + char *end; + double x = strtod (s, &end); + return (end == e && x >= 0 && x <= 1) ? x : -1; +} + +/* Parse S as a numeric color specification and set *R, *G and *B. + Return true on success, false on failure. + Recognized formats: + + "#RGB", with R, G and B hex strings of equal length, 1-4 digits each + "rgb:R/G/B", with R, G and B hex strings, 1-4 digits each + "rgbi:R/G/B", with R, G and B numbers in [0,1] + + The result is normalized to a maximum value of 65535 per component. */ +bool +parse_color_spec (const char *s, + unsigned short *r, unsigned short *g, unsigned short *b) +{ + int len = strlen (s); + if (s[0] == '#') + { + if ((len - 1) % 3 == 0) + { + int n = (len - 1) / 3; + return ( parse_hex_color_comp (s + 1 + 0 * n, s + 1 + 1 * n, r) + && parse_hex_color_comp (s + 1 + 1 * n, s + 1 + 2 * n, g) + && parse_hex_color_comp (s + 1 + 2 * n, s + 1 + 3 * n, b)); + } + } + else if (strncmp (s, "rgb:", 4) == 0) + { + char *sep1, *sep2; + return ((sep1 = strchr (s + 4, '/')) != NULL + && (sep2 = strchr (sep1 + 1, '/')) != NULL + && parse_hex_color_comp (s + 4, sep1, r) + && parse_hex_color_comp (sep1 + 1, sep2, g) + && parse_hex_color_comp (sep2 + 1, s + len, b)); + } + else if (strncmp (s, "rgbi:", 5) == 0) + { + char *sep1, *sep2; + double red, green, blue; + if ((sep1 = strchr (s + 5, '/')) != NULL + && (sep2 = strchr (sep1 + 1, '/')) != NULL + && (red = parse_float_color_comp (s + 5, sep1)) >= 0 + && (green = parse_float_color_comp (sep1 + 1, sep2)) >= 0 + && (blue = parse_float_color_comp (sep2 + 1, s + len)) >= 0) + { + *r = lrint (red * 65535); + *g = lrint (green * 65535); + *b = lrint (blue * 65535); + return true; + } + } + return false; +} + +DEFUN ("internal-color-values-from-color-spec", + Finternal_color_values_from_color_spec, + Sinternal_color_values_from_color_spec, + 1, 1, 0, + doc: /* Parse STRING as a numeric color and return (RED GREEN BLUE). +Recognised formats for STRING are: + + #RGB, where R, G and B are hex numbers of equal length, 1-4 digits each + rgb:R/G/B, where R, G, and B are hex numbers, 1-4 digits each + rgbi:R/G/B, where R, G and B are floating-point numbers in [0,1] + +The result is normalized to a maximum value of 65535 per component, +forming a list of three integers in [0,65535]. +If STRING is not in one of the above forms, return nil. */) + (Lisp_Object string) +{ + CHECK_STRING (string); + unsigned short r, g, b; + return (parse_color_spec (SSDATA (string), &r, &g, &b) + ? list3i (r, g, b) + : Qnil); +} + /* Parse RGB_LIST, and fill in the RGB fields of COLOR. RGB_LIST should contain (at least) 3 lisp integers. Return true iff RGB_LIST is OK. */ @@ -7018,4 +7133,5 @@ clear the face cache, see `clear-face-cache'. */); defsubr (&Sinternal_face_x_get_resource); defsubr (&Sx_family_fonts); #endif + defsubr (&Sinternal_color_values_from_color_spec); } diff --git a/src/xterm.c b/src/xterm.c index 7989cecec7f..6340700cb89 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -2376,8 +2376,6 @@ x_query_frame_background_color (struct frame *f, XColor *bgcolor) x_query_colors (f, bgcolor, 1); } -#define HEX_COLOR_NAME_LENGTH 32 - /* On frame F, translate the color name to RGB values. Use cached information, if possible. @@ -2389,44 +2387,23 @@ x_query_frame_background_color (struct frame *f, XColor *bgcolor) Status x_parse_color (struct frame *f, const char *color_name, XColor *color) { + /* Don't pass #RGB strings directly to XParseColor, because that + follows the X convention of zero-extending each channel + value: #f00 means #f00000. We want the convention of scaling + channel values, so #f00 means #ff0000, just as it does for + HTML, SVG, and CSS. */ + unsigned short r, g, b; + if (parse_color_spec (color_name, &r, &g, &b)) + { + color->red = r; + color->green = g; + color->blue = b; + return 1; + } + Display *dpy = FRAME_X_DISPLAY (f); Colormap cmap = FRAME_X_COLORMAP (f); struct color_name_cache_entry *cache_entry; - - if (color_name[0] == '#') - { - /* Don't pass #RGB strings directly to XParseColor, because that - follows the X convention of zero-extending each channel - value: #f00 means #f00000. We want the convention of scaling - channel values, so #f00 means #ff0000, just as it does for - HTML, SVG, and CSS. - - So we translate #f00 to rgb:f/0/0, which X handles - differently. */ - char rgb_color_name[HEX_COLOR_NAME_LENGTH]; - int len = strlen (color_name); - int digits_per_channel; - if (len == 4) - digits_per_channel = 1; - else if (len == 7) - digits_per_channel = 2; - else if (len == 10) - digits_per_channel = 3; - else if (len == 13) - digits_per_channel = 4; - else - return 0; - - snprintf (rgb_color_name, sizeof rgb_color_name, "rgb:%.*s/%.*s/%.*s", - digits_per_channel, color_name + 1, - digits_per_channel, color_name + digits_per_channel + 1, - digits_per_channel, color_name + 2 * digits_per_channel + 1); - - /* The rgb form is parsed directly by XParseColor without - talking to the X server. No need for caching. */ - return XParseColor (dpy, cmap, rgb_color_name, color); - } - for (cache_entry = FRAME_DISPLAY_INFO (f)->color_names; cache_entry; cache_entry = cache_entry->next) { diff --git a/test/src/xfaces-tests.el b/test/src/xfaces-tests.el index 5ed16c9e51d..34cda07e5b4 100644 --- a/test/src/xfaces-tests.el +++ b/test/src/xfaces-tests.el @@ -24,4 +24,27 @@ (should (equal (color-distance "#222222" "#ffffff") (color-distance "#ffffff" "#222222")))) +(ert-deftest xfaces-internal-color-values-from-color-spec () + (should (equal (internal-color-values-from-color-spec "#f05") + '(#xffff #x0000 #x5555))) + (should (equal (internal-color-values-from-color-spec "#1fb0C5") + '(#x1f1f #xb0b0 #xc5c5))) + (should (equal (internal-color-values-from-color-spec "#1f8b0AC5e") + '(#x1f81 #xb0aa #xc5eb))) + (should (equal (internal-color-values-from-color-spec "#1f83b0ADC5e2") + '(#x1f83 #xb0ad #xc5e2))) + (should (equal (internal-color-values-from-color-spec "#1f83b0ADC5e2g") nil)) + (should (equal (internal-color-values-from-color-spec "#1f83b0ADC5e20") nil)) + (should (equal (internal-color-values-from-color-spec "#12345") nil)) + (should (equal (internal-color-values-from-color-spec "rgb:f/23/28a") + '(#xffff #x2323 #x28a2))) + (should (equal (internal-color-values-from-color-spec "rgb:1234/5678/09ab") + '(#x1234 #x5678 #x09ab))) + (should (equal (internal-color-values-from-color-spec "rgb:0//0") nil)) + (should (equal (internal-color-values-from-color-spec "rgbi:0/0.5/0.1") + '(0 32768 6554))) + (should (equal (internal-color-values-from-color-spec "rgbi:1e-3/1.0e-2/1e0") + '(66 655 65535))) + (should (equal (internal-color-values-from-color-spec "rgbi:0/0.5/10") nil))) + (provide 'xfaces-tests)