[PATCH weston 1/2] config-parser: Add weston_config_section_get_color

Bryce Harrington bryce at osg.samsung.com
Thu Jul 14 18:47:45 UTC 2016


On Thu, Jul 14, 2016 at 09:16:41AM +0200, Quentin Glidic wrote:
> On 14/07/2016 04:01, Bryce Harrington wrote:
> >Previously weston_config_section_get_uint was serving dual purpose for
> >parsing both unsigned decimal integer values (ids, counts, seconds,
> >etc.)  and hexadecimal values (colors), by relying on strtoul's
> >auto-detection mechanism.
> >
> >However, this usage is unable to catch certain kinds of error
> >conditions, such as specifying a negative number where an unsigned
> >should be used.  And for colors in particular, it would misparse hex
> >values if the leading 0x was omitted.  E.g. "background-color=99999999"
> >would render a near-black background (effectively 0x05f5e0ff) instead of
> >medium grey, and "background-color=ffffffff" would be treated as an
> >error rather than white.  "background-color=0x01234567",
> >"background-color=01234567", and "background-color=1234567" each
> >resulted in the value being parsed as hexadecimal, octal, and decimal
> >respectively, resulting in colors 0x01234567, 0x00053977, and 0x0012d687
> >being displayed.
> >
> >This new routine forces hexadecimal to be used in all cases when parsing
> >color values, so "0x01234567", "01234567", and "1234567" all result in
> >the same color value, "99999999" is grey, and "ffffffff" is white.
> >
> >Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> >---
> 
> If we add a specific helper, I would use more specific parsing, i.e.
> fully support CSS-compatible # notation.
> 
> It is not that hard: check if the first char is #, then split by
> groups of 2 chars to be parsed as hex. Alpha is then the last part
> #RRGGBBAA and we can also support the one-char-per-colour version
> #RGB(A).
>
> I can make a patch quickly if you want, with backward compatibility
> (if prefix is 0x, use 0x(AA)RRGGBB).

Full support of CSS-compatible notation would be nice.  The one problem
though is that config-parser's file handling treats # as a comment
character and strips it out unconditionally.  Do you have ideas on how
to handle that?

Bryce
 
> Cheers,
> 
> 
> > clients/desktop-shell.c            |  8 ++--
> > clients/ivi-shell-user-interface.c |  2 +-
> > shared/config-parser.c             | 27 +++++++++++++
> > shared/config-parser.h             |  4 ++
> > tests/config-parser-test.c         | 80 ++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 116 insertions(+), 5 deletions(-)
> >
> >diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> >index f5e0ba2..18b8f9e 100644
> >--- a/clients/desktop-shell.c
> >+++ b/clients/desktop-shell.c
> >@@ -571,8 +571,8 @@ panel_create(struct desktop *desktop)
> > 	free (clock_format_option);
> >
> > 	s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
> >-	weston_config_section_get_uint(s, "panel-color",
> >-				       &panel->color, 0xaa000000);
> >+	weston_config_section_get_color(s, "panel-color",
> >+					&panel->color, 0xaa000000);
> >
> > 	panel_add_launchers(panel, desktop);
> >
> >@@ -1068,8 +1068,8 @@ background_create(struct desktop *desktop)
> > 	s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
> > 	weston_config_section_get_string(s, "background-image",
> > 					 &background->image, NULL);
> >-	weston_config_section_get_uint(s, "background-color",
> >-				       &background->color, 0);
> >+	weston_config_section_get_color(s, "background-color",
> >+					&background->color, 0x00000000);
> >
> > 	weston_config_section_get_string(s, "background-type",
> > 					 &type, "tile");
> >diff --git a/clients/ivi-shell-user-interface.c b/clients/ivi-shell-user-interface.c
> >index 06b79a2..465def1 100644
> >--- a/clients/ivi-shell-user-interface.c
> >+++ b/clients/ivi-shell-user-interface.c
> >@@ -1143,7 +1143,7 @@ hmi_homescreen_setting_create(void)
> > 	weston_config_section_get_uint(
> > 		shellSection, "home-id", &setting->home.id, 1007);
> >
> >-	weston_config_section_get_uint(
> >+	weston_config_section_get_color(
> > 		shellSection, "workspace-background-color",
> > 		&setting->workspace_background.color, 0x99000000);
> >
> >diff --git a/shared/config-parser.c b/shared/config-parser.c
> >index 1e08759..f040380 100644
> >--- a/shared/config-parser.c
> >+++ b/shared/config-parser.c
> >@@ -209,6 +209,33 @@ weston_config_section_get_uint(struct weston_config_section *section,
> >
> > WL_EXPORT
> > int
> >+weston_config_section_get_color(struct weston_config_section *section,
> >+				const char *key,
> >+				uint32_t *color, uint32_t default_color)
> >+{
> >+	struct weston_config_entry *entry;
> >+	char *end;
> >+
> >+	entry = config_section_get_entry(section, key);
> >+	if (entry == NULL) {
> >+		*color = default_color;
> >+		errno = ENOENT;
> >+		return -1;
> >+	}
> >+
> >+	errno = 0;
> >+	*color = strtoul(entry->value, &end, 16);
> >+	if (errno != 0 || end == entry->value || *end != '\0') {
> >+		*color = default_color;
> >+		errno = EINVAL;
> >+		return -1;
> >+	}
> >+
> >+	return 0;
> >+}
> >+
> >+WL_EXPORT
> >+int
> > weston_config_section_get_double(struct weston_config_section *section,
> > 				 const char *key,
> > 				 double *value, double default_value)
> >diff --git a/shared/config-parser.h b/shared/config-parser.h
> >index b8462a7..17ef5d7 100644
> >--- a/shared/config-parser.h
> >+++ b/shared/config-parser.h
> >@@ -85,6 +85,10 @@ weston_config_section_get_uint(struct weston_config_section *section,
> > 			       const char *key,
> > 			       uint32_t *value, uint32_t default_value);
> > int
> >+weston_config_section_get_color(struct weston_config_section *section,
> >+				const char *key,
> >+				uint32_t *color, uint32_t default_color);
> >+int
> > weston_config_section_get_double(struct weston_config_section *section,
> > 				 const char *key,
> > 				 double *value, double default_value);
> >diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c
> >index 735da4e..f9495b3 100644
> >--- a/tests/config-parser-test.c
> >+++ b/tests/config-parser-test.c
> >@@ -118,6 +118,9 @@ static struct zuc_fixture config_test_t1 = {
> > 	"number=5252\n"
> > 	"zero=0\n"
> > 	"flag=false\n"
> >+	"color-none=0x000000\n"
> >+	"color-low=0x11223344\n"
> >+	"color-high=0xff00ff00\n"
> > 	"\n"
> > 	"[stuff]\n"
> > 	"flag=     true \n"
> >@@ -461,6 +464,83 @@ ZUC_TEST_F(config_test_t1, test019, data)
> > 	ZUC_ASSERT_EQ(0, errno);
> > }
> >
> >+ZUC_TEST_F(config_test_t1, test020, data)
> >+{
> >+	int r;
> >+	uint32_t n;
> >+	struct weston_config_section *section;
> >+	struct weston_config *config = data;
> >+
> >+	section = weston_config_get_section(config, "bar", NULL, NULL);
> >+	r = weston_config_section_get_color(section, "color-none", &n, 0xff336699);
> >+
> >+	ZUC_ASSERT_EQ(0, r);
> >+	ZUC_ASSERT_EQ(0x000000, n);
> >+	ZUC_ASSERT_EQ(0, errno);
> >+}
> >+
> >+ZUC_TEST_F(config_test_t1, test021, data)
> >+{
> >+	int r;
> >+	uint32_t n;
> >+	struct weston_config_section *section;
> >+	struct weston_config *config = data;
> >+
> >+	section = weston_config_get_section(config, "bar", NULL, NULL);
> >+	r = weston_config_section_get_color(section, "color-low", &n, 0xff336699);
> >+
> >+	ZUC_ASSERT_EQ(0, r);
> >+	ZUC_ASSERT_EQ(0x11223344, n);
> >+	ZUC_ASSERT_EQ(0, errno);
> >+}
> >+
> >+ZUC_TEST_F(config_test_t1, test022, data)
> >+{
> >+	int r;
> >+	uint32_t n;
> >+	struct weston_config_section *section;
> >+	struct weston_config *config = data;
> >+
> >+	section = weston_config_get_section(config, "bar", NULL, NULL);
> >+	r = weston_config_section_get_color(section, "color-high", &n, 0xff336699);
> >+
> >+	ZUC_ASSERT_EQ(0, r);
> >+	ZUC_ASSERT_EQ(0xff00ff00, n);
> >+	ZUC_ASSERT_EQ(0, errno);
> >+}
> >+
> >+ZUC_TEST_F(config_test_t1, test023, data)
> >+{
> >+	int r;
> >+	uint32_t n;
> >+	struct weston_config_section *section;
> >+	struct weston_config *config = data;
> >+
> >+	// Treat colors as hex values even if missing the leading 0x
> >+	section = weston_config_get_section(config, "bar", NULL, NULL);
> >+	r = weston_config_section_get_color(section, "number", &n, 0xff336699);
> >+
> >+	ZUC_ASSERT_EQ(0, r);
> >+	ZUC_ASSERT_EQ(0x5252, n);
> >+	ZUC_ASSERT_EQ(0, errno);
> >+}
> >+
> >+ZUC_TEST_F(config_test_t1, test024, data)
> >+{
> >+	int r;
> >+	uint32_t n;
> >+	struct weston_config_section *section;
> >+	struct weston_config *config = data;
> >+
> >+	// String color names are unsupported
> >+	section = weston_config_get_section(config, "bucket", NULL, NULL);
> >+	r = weston_config_section_get_color(section, "color", &n, 0xff336699);
> >+
> >+	ZUC_ASSERT_EQ(-1, r);
> >+	ZUC_ASSERT_EQ(0xff336699, n);
> >+	ZUC_ASSERT_EQ(EINVAL, errno);
> >+}
> >+
> > ZUC_TEST_F(config_test_t2, doesnt_parse, data)
> > {
> > 	struct weston_config *config = data;
> >
> 
> 
> -- 
> 
> Quentin “Sardem FF7” Glidic


More information about the wayland-devel mailing list