Re: [PATCH weston v2 1/2] config-parser: add support for more color formats

2016-11-21 Thread Daniel Stone
Hi Eric,

On 26 October 2016 at 23:53, Eric Engestrom  wrote:
> Supported colour formats are:
> - (0x)(AA)RRGGBB
> - #RGB(A)
> - #RRGGBB(AA)

If I'm honest, I don't entirely see the value in these. Accepting
12345678 as well as 0x12345678 implies, to me at least, that the
former will be interpreted as decimal and the latter as hexadecimal.
Obviously it would be better if we'd only accepted 0x for hex in the
first place, but here we are. I'd still rather not increase the
confusion by accepting both at this point.

I similarly don't think the # adds that much value (people can figure
it out easily enough), and the half-precision variant is rare enough
that I'm not sure it's worth supporting.

So, exceedingly long lines notwithstanding, it's a nice enough patch,
but I'd rather just leave it be, suboptimal though the current
situation is. Thanks and sorry.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v2 1/2] config-parser: add support for more color formats

2016-10-26 Thread Eric Engestrom
Supported colour formats are:
- (0x)(AA)RRGGBB
- #RGB(A)
- #RRGGBB(AA)

Cc: Bryce Harrington 
Cc: Quentin Glidic 
Signed-off-by: Eric Engestrom 
---
v2: reduce the number of formats supported
read #-prefixed values the same way CSS would
---
 shared/config-parser.c | 54 ++
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/shared/config-parser.c b/shared/config-parser.c
index e2b5fa2..b3848a2 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -215,6 +215,12 @@ weston_config_section_get_uint(struct 
weston_config_section *section,
return 0;
 }
 
+/*
+ * Supported colour formats:
+ * - (0x)(AA)RRGGBB
+ * - #RGB(A)
+ * - #RRGGBB(AA)
+ */
 WL_EXPORT
 int
 weston_config_section_get_color(struct weston_config_section *section,
@@ -224,6 +230,10 @@ weston_config_section_get_color(struct 
weston_config_section *section,
struct weston_config_entry *entry;
int len;
char *end;
+   const char alpha[] = "FF"; /* Default alpha when unspecified */
+   const char *val;
+   const char *color_string;
+   enum { hex, css } colour_type;
 
entry = config_section_get_entry(section, key);
if (entry == NULL) {
@@ -232,19 +242,47 @@ weston_config_section_get_color(struct 
weston_config_section *section,
return -1;
}
 
+   val = entry->value;
+   len = strlen(val);
-   len = strlen(entry->value);
+
-   if (len == 1 && entry->value[0] == '0') {
+   if (len == 1 && val[0] == '0') {
*color = 0;
return 0;
-   } else if (len != 8 && len != 10) {
-   *color = default_color;
-   errno = EINVAL;
-   return -1;
}
 
+   if (len > 2 && val[0] == '0' && val[1] == 'x') {
+   val += 2;
+   len -= 2;
+   colour_type = hex;
+   }
+   else if (len > 1 && val[0] == '#') {
+   val += 1;
+   len -= 1;
+   colour_type = css;
+   }
+   else
+   colour_type = hex;
+
+   if (colour_type == hex)
+   switch (len) {
+   case 8: color_string = (char[]){   val[0],   val[1], val[2], 
val[3], val[4], val[5], val[6], val[7], '\0' }; break; /* (0x)AARRGGBB */
+   case 6: color_string = (char[]){ alpha[0], alpha[1], val[0], 
val[1], val[2], val[3], val[4], val[5], '\0' }; break; /*   (0x)RRGGBB */
+   default: goto invalid;
+   }
+
+   if (colour_type == css)
+   switch (len) {
+   case 8: color_string = (char[]){   val[6],   val[7], val[0], 
val[1], val[2], val[3], val[4], val[5], '\0' }; break; /* #RRGGBBAA */
+   case 6: color_string = (char[]){ alpha[0], alpha[1], val[0], 
val[1], val[2], val[3], val[4], val[5], '\0' }; break; /* #RRGGBB   */
+   case 4: color_string = (char[]){   val[3],   val[3], val[0], 
val[0], val[1], val[1], val[2], val[2], '\0' }; break; /* # R G B A */
+   case 3: color_string = (char[]){ alpha[0], alpha[1], val[0], 
val[0], val[1], val[1], val[2], val[2], '\0' }; break; /* # R G B   */
+   default: goto invalid;
+   }
+
errno = 0;
-   *color = strtoul(entry->value, , 16);
+   *color = strtoul(color_string, , 16);
-   if (errno != 0 || end == entry->value || *end != '\0') {
+   if (errno != 0 || end == color_string || *end != '\0') {
+invalid:
*color = default_color;
errno = EINVAL;
return -1;
-- 
Cheers,
  Eric
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel