From 69e7838461e8b465762d9217c6038667a4136e4d Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 17 Aug 2024 15:43:42 +0200 Subject: [PATCH 1/5] rendernodeparser: Don't have unnecessary forward declarations Reorders functions to not have them. No functional changes. --- gsk/gskrendernodeparser.c | 225 +++++++++++++++++++------------------- 1 file changed, 111 insertions(+), 114 deletions(-) diff --git a/gsk/gskrendernodeparser.c b/gsk/gskrendernodeparser.c index 7526a729a5..521ef39e5d 100644 --- a/gsk/gskrendernodeparser.c +++ b/gsk/gskrendernodeparser.c @@ -712,9 +712,117 @@ parse_float4 (GtkCssParser *parser, return TRUE; } -static gboolean parse_color2 (GtkCssParser *parser, - Context *context, - gpointer color); +static gboolean +parse_color_state (GtkCssParser *parser, + Context *context, + gpointer color_state) +{ + GdkColorState *cs = NULL; + + if (gtk_css_parser_try_ident (parser, "srgb")) + cs = gdk_color_state_get_srgb (); + else if (gtk_css_parser_try_ident (parser, "srgb-linear")) + cs = gdk_color_state_get_srgb_linear (); + else if (gtk_css_parser_try_ident (parser, "rec2100-pq")) + cs = gdk_color_state_get_rec2100_pq (); + else if (gtk_css_parser_try_ident (parser, "rec2100-linear")) + cs = gdk_color_state_get_rec2100_linear (); + else if (gtk_css_token_is (gtk_css_parser_get_token (parser), GTK_CSS_TOKEN_STRING)) + { + char *name = gtk_css_parser_consume_string (parser); + + if (context->named_color_states) + cs = g_hash_table_lookup (context->named_color_states, name); + + if (!cs) + { + gtk_css_parser_error_value (parser, "No color state named \"%s\"", name); + g_free (name); + return FALSE; + } + + g_free (name); + } + else + { + gtk_css_parser_error_syntax (parser, "Expected a valid color state"); + return FALSE; + } + + *(GdkColorState **) color_state = gdk_color_state_ref (cs); + return TRUE; +} + +typedef struct { + Context *context; + GdkColor *color; +} ColorArgData; + +static guint +parse_color_arg (GtkCssParser *parser, + guint arg, + gpointer data) +{ + ColorArgData *d = data; + GdkColorState *color_state; + float values[4]; + + if (!parse_color_state (parser, d->context, &color_state)) + return 0; + + for (int i = 0; i < 3; i++) + { + double number; + + if (!gtk_css_parser_consume_number_or_percentage (parser, 0, 1, &number)) + return 0; + + values[i] = number; + } + + if (gtk_css_parser_try_delim (parser, '/')) + { + double number; + + if (!gtk_css_parser_consume_number_or_percentage (parser, 0, 1, &number)) + return 0; + + values[3] = number; + } + else + { + values[3] = 1; + } + + gdk_color_init (d->color, color_state, values); + return 1; +} + +static gboolean +parse_color2 (GtkCssParser *parser, + Context *context, + gpointer color) +{ + GdkRGBA rgba; + + if (gtk_css_parser_has_function (parser, "color")) + { + ColorArgData data = { context, color }; + + if (!gtk_css_parser_consume_function (parser, 1, 1, parse_color_arg, &data)) + return FALSE; + + return TRUE; + } + else if (gdk_rgba_parser_parse (parser, &rgba)) + { + gdk_color_init_from_rgba ((GdkColor *) color, &rgba); + return TRUE; + } + + return FALSE; +} + static gboolean parse_shadows (GtkCssParser *parser, @@ -1565,117 +1673,6 @@ parse_color_state_rule (GtkCssParser *parser, return TRUE; } -static gboolean -parse_color_state (GtkCssParser *parser, - Context *context, - gpointer color_state) -{ - GdkColorState *cs = NULL; - - if (gtk_css_parser_try_ident (parser, "srgb")) - cs = gdk_color_state_get_srgb (); - else if (gtk_css_parser_try_ident (parser, "srgb-linear")) - cs = gdk_color_state_get_srgb_linear (); - else if (gtk_css_parser_try_ident (parser, "rec2100-pq")) - cs = gdk_color_state_get_rec2100_pq (); - else if (gtk_css_parser_try_ident (parser, "rec2100-linear")) - cs = gdk_color_state_get_rec2100_linear (); - else if (gtk_css_token_is (gtk_css_parser_get_token (parser), GTK_CSS_TOKEN_STRING)) - { - char *name = gtk_css_parser_consume_string (parser); - - if (context->named_color_states) - cs = g_hash_table_lookup (context->named_color_states, name); - - if (!cs) - { - gtk_css_parser_error_value (parser, "No color state named \"%s\"", name); - g_free (name); - return FALSE; - } - - g_free (name); - } - else - { - gtk_css_parser_error_syntax (parser, "Expected a valid color state"); - return FALSE; - } - - *(GdkColorState **) color_state = gdk_color_state_ref (cs); - return TRUE; -} - -typedef struct { - Context *context; - GdkColor *color; -} ColorArgData; - -static guint -parse_color_arg (GtkCssParser *parser, - guint arg, - gpointer data) -{ - ColorArgData *d = data; - GdkColorState *color_state; - float values[4]; - - if (!parse_color_state (parser, d->context, &color_state)) - return 0; - - for (int i = 0; i < 3; i++) - { - double number; - - if (!gtk_css_parser_consume_number_or_percentage (parser, 0, 1, &number)) - return 0; - - values[i] = number; - } - - if (gtk_css_parser_try_delim (parser, '/')) - { - double number; - - if (!gtk_css_parser_consume_number_or_percentage (parser, 0, 1, &number)) - return 0; - - values[3] = number; - } - else - { - values[3] = 1; - } - - gdk_color_init (d->color, color_state, values); - return 1; -} - -static gboolean -parse_color2 (GtkCssParser *parser, - Context *context, - gpointer color) -{ - GdkRGBA rgba; - - if (gtk_css_parser_has_function (parser, "color")) - { - ColorArgData data = { context, color }; - - if (!gtk_css_parser_consume_function (parser, 1, 1, parse_color_arg, &data)) - return FALSE; - - return TRUE; - } - else if (gdk_rgba_parser_parse (parser, &rgba)) - { - gdk_color_init_from_rgba ((GdkColor *) color, &rgba); - return TRUE; - } - - return FALSE; -} - static gboolean parse_colors4 (GtkCssParser *parser, Context *context, From 585f31fa2ed1afe5102f2c76fbb57f6c83686874 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 17 Aug 2024 15:55:37 +0200 Subject: [PATCH 2/5] rendernodeparser: Rename function There's no need to name a function parse_color2() when parse_color() doesn't exist. --- gsk/gskrendernodeparser.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gsk/gskrendernodeparser.c b/gsk/gskrendernodeparser.c index 521ef39e5d..bacb5f1475 100644 --- a/gsk/gskrendernodeparser.c +++ b/gsk/gskrendernodeparser.c @@ -799,9 +799,9 @@ parse_color_arg (GtkCssParser *parser, } static gboolean -parse_color2 (GtkCssParser *parser, - Context *context, - gpointer color) +parse_color (GtkCssParser *parser, + Context *context, + gpointer color) { GdkRGBA rgba; @@ -837,7 +837,7 @@ parse_shadows (GtkCssParser *parser, GdkColor color = GDK_COLOR_SRGB (0, 0, 0, 1); double dx = 0, dy = 0, radius = 0; - if (!parse_color2 (parser, context, &color)) + if (!parse_color (parser, context, &color)) gtk_css_parser_error_value (parser, "Expected shadow color"); if (!gtk_css_parser_consume_number (parser, &dx)) @@ -1683,7 +1683,7 @@ parse_colors4 (GtkCssParser *parser, for (i = 0; i < 4 && !gtk_css_parser_has_token (parser, GTK_CSS_TOKEN_EOF); i ++) { - if (!parse_color2 (parser, context, &colors[i])) + if (!parse_color (parser, context, &colors[i])) return FALSE; } if (i == 0) @@ -1709,7 +1709,7 @@ parse_color_node (GtkCssParser *parser, GdkColor color = GDK_COLOR_SRGB (1, 0, 0.8, 1); const Declaration declarations[] = { { "bounds", parse_rect, NULL, &bounds }, - { "color", parse_color2, NULL, &color }, + { "color", parse_color, NULL, &color }, }; GskRenderNode *node; @@ -1887,7 +1887,7 @@ parse_inset_shadow_node (GtkCssParser *parser, double dx = 1, dy = 1, blur = 0, spread = 0; const Declaration declarations[] = { { "outline", parse_rounded_rect, NULL, &outline }, - { "color", parse_color2, NULL, &color }, + { "color", parse_color, NULL, &color }, { "dx", parse_double, NULL, &dx }, { "dy", parse_double, NULL, &dy }, { "spread", parse_double, NULL, &spread }, @@ -2301,7 +2301,7 @@ parse_outset_shadow_node (GtkCssParser *parser, double dx = 1, dy = 1, blur = 0, spread = 0; const Declaration declarations[] = { { "outline", parse_rounded_rect, NULL, &outline }, - { "color", parse_color2, NULL, &color }, + { "color", parse_color, NULL, &color }, { "dx", parse_double, NULL, &dx }, { "dy", parse_double, NULL, &dy }, { "spread", parse_double, NULL, &spread }, @@ -2612,7 +2612,7 @@ parse_text_node (GtkCssParser *parser, const Declaration declarations[] = { { "font", parse_font, clear_font, &font }, { "offset", parse_point, NULL, &offset }, - { "color", parse_color2, NULL, &color }, + { "color", parse_color, NULL, &color }, { "glyphs", parse_glyphs, clear_glyphs, &glyphs }, { "hint-style", parse_hint_style, NULL, &hint_style }, { "antialias", parse_antialias, NULL, &antialias }, From 4e9ebb5299b697d4590d9a622adf43a45ba72066 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 6 Aug 2024 16:15:53 -0400 Subject: [PATCH 3/5] gdk: Add gdk_color_state_clamp() Allows clamping values into the correct range to construct valid colors. --- gdk/gdkcolorstate.c | 86 ++++++++++++++++++++++++++++++++++++++ gdk/gdkcolorstateprivate.h | 11 ++++- 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/gdk/gdkcolorstate.c b/gdk/gdkcolorstate.c index f6493de173..35f5c23402 100644 --- a/gdk/gdkcolorstate.c +++ b/gdk/gdkcolorstate.c @@ -331,6 +331,68 @@ gdk_default_color_state_get_cicp (GdkColorState *color_state) return &self->cicp; } +static gboolean +gdk_color_state_check_inf_nan (const float src[4], + float dest[4]) +{ + if (isnan (src[0]) || + isnan (src[1]) || + isnan (src[2]) || + isnan (src[3])) + { + dest = (float[4]) { 1.0, 0.0, 0.8, 1.0 }; + return TRUE; + } + if (isinf (src[0]) || + isinf (src[1]) || + isinf (src[2]) || + isinf (src[3])) + { + dest = (float[4]) { 0.0, 0.8, 1.0, 1.0 }; + return TRUE; + } + + return FALSE; +} + +static void +gdk_color_state_clamp_0_1 (GdkColorState *self, + const float src[4], + float dest[4]) +{ + if (gdk_color_state_check_inf_nan (src, dest)) + return; + + dest[0] = CLAMP (src[0], 0.0f, 1.0f); + dest[1] = CLAMP (src[1], 0.0f, 1.0f); + dest[2] = CLAMP (src[2], 0.0f, 1.0f); + dest[3] = CLAMP (src[3], 0.0f, 1.0f); +} + +static void +gdk_color_state_clamp_unbounded (GdkColorState *self, + const float src[4], + float dest[4]) +{ + if (gdk_color_state_check_inf_nan (src, dest)) + return; + + dest[0] = src[0]; + dest[1] = src[1]; + dest[2] = src[2]; + dest[3] = CLAMP (src[3], 0.0f, 1.0f); +} + +static void +gdk_default_color_state_clamp (GdkColorState *color_state, + const float in[4], + float out[4]) +{ + GdkDefaultColorState *self = (GdkDefaultColorState *) color_state; + + self->clamp (color_state, in, out); +} + /* }}} */ static const @@ -342,6 +404,7 @@ GdkColorStateClass GDK_DEFAULT_COLOR_STATE_CLASS = { .get_convert_to = gdk_default_color_state_get_convert_to, .get_convert_from = gdk_default_color_state_get_convert_from, .get_cicp = gdk_default_color_state_get_cicp, + .clamp = gdk_default_color_state_clamp, }; GdkDefaultColorState gdk_default_color_states[] = { @@ -360,6 +423,7 @@ GdkDefaultColorState gdk_default_color_states[] = { [GDK_COLOR_STATE_ID_REC2100_PQ] = gdk_default_srgb_to_rec2100_pq, [GDK_COLOR_STATE_ID_REC2100_LINEAR] = gdk_default_srgb_to_rec2100_linear, }, + .clamp = gdk_color_state_clamp_0_1, .cicp = { 1, 13, 0, 1 }, }, [GDK_COLOR_STATE_ID_SRGB_LINEAR] = { @@ -377,6 +441,7 @@ GdkDefaultColorState gdk_default_color_states[] = { [GDK_COLOR_STATE_ID_REC2100_PQ] = gdk_default_srgb_linear_to_rec2100_pq, [GDK_COLOR_STATE_ID_REC2100_LINEAR] = gdk_default_srgb_linear_to_rec2100_linear, }, + .clamp = gdk_color_state_clamp_0_1, .cicp = { 1, 8, 0, 1 }, }, [GDK_COLOR_STATE_ID_REC2100_PQ] = { @@ -394,6 +459,7 @@ GdkDefaultColorState gdk_default_color_states[] = { [GDK_COLOR_STATE_ID_SRGB_LINEAR] = gdk_default_rec2100_pq_to_srgb_linear, [GDK_COLOR_STATE_ID_REC2100_LINEAR] = gdk_default_rec2100_pq_to_rec2100_linear, }, + .clamp = gdk_color_state_clamp_0_1, .cicp = { 9, 16, 0, 1 }, }, [GDK_COLOR_STATE_ID_REC2100_LINEAR] = { @@ -411,6 +477,7 @@ GdkDefaultColorState gdk_default_color_states[] = { [GDK_COLOR_STATE_ID_SRGB_LINEAR] = gdk_default_rec2100_linear_to_srgb_linear, [GDK_COLOR_STATE_ID_REC2100_PQ] = gdk_default_rec2100_linear_to_rec2100_pq, }, + .clamp = gdk_color_state_clamp_unbounded, .cicp = { 9, 8, 0, 1 }, }, }; @@ -566,6 +633,7 @@ GdkColorStateClass GDK_CICP_COLOR_STATE_CLASS = { .get_convert_to = gdk_cicp_color_state_get_convert_to, .get_convert_from = gdk_cicp_color_state_get_convert_from, .get_cicp = gdk_cicp_color_state_get_cicp, + .clamp = gdk_color_state_clamp_0_1, }; static inline float * @@ -770,6 +838,24 @@ gdk_color_state_get_no_srgb_tf (GdkColorState *self) return self->klass->get_no_srgb_tf (self); } +/*< private > + * gdk_color_state_clamp: + * @self: a `GdkColorState` + * @src: the values to clamp + * @dest: (out): location to store the result, may be identical to + * the src argument + * + * Clamps the values to be within the allowed ranges for the given + * color state. + */ +void +gdk_color_state_clamp (GdkColorState *self, + const float src[4], + float dest[4]) +{ + self->klass->clamp (self, src, dest); +} + /* }}} */ /* vim:set foldmethod=marker expandtab: */ diff --git a/gdk/gdkcolorstateprivate.h b/gdk/gdkcolorstateprivate.h index f2bfb93bf6..be98807f0c 100644 --- a/gdk/gdkcolorstateprivate.h +++ b/gdk/gdkcolorstateprivate.h @@ -46,6 +46,9 @@ struct _GdkColorStateClass GdkFloatColorConvert (* get_convert_from) (GdkColorState *self, GdkColorState *source); const GdkCicp * (* get_cicp) (GdkColorState *self); + void (* clamp) (GdkColorState *self, + const float src[4], + float dest[4]); }; typedef struct _GdkDefaultColorState GdkDefaultColorState; @@ -57,6 +60,9 @@ struct _GdkDefaultColorState const char *name; GdkColorState *no_srgb; GdkFloatColorConvert convert_to[GDK_COLOR_STATE_N_IDS]; + void (* clamp) (GdkColorState *self, + const float src[4], + float dest[4]); GdkCicp cicp; }; @@ -78,6 +84,10 @@ GdkColorState * gdk_color_state_get_no_srgb_tf (GdkColorState GdkColorState * gdk_color_state_new_for_cicp (const GdkCicp *cicp, GError **error); +void gdk_color_state_clamp (GdkColorState *self, + const float src[4], + float dest[4]); + static inline GdkColorState * gdk_color_state_get_rendering_color_state (GdkColorState *self) { @@ -214,4 +224,3 @@ gdk_color_state_from_rgba (GdkColorState *self, self, out_color); } - From a6233ac852a0082cfc805492305e6393dec73f52 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 17 Aug 2024 16:01:40 +0200 Subject: [PATCH 4/5] rendernodeparser: Check color values aren't out of range Use the clamp() API from the previous commit to: 1. Clamp values into range 2. Emit an error if values were out of range Unlike CSS, which just clamps and doesn't emit an error, we do want to emit one because we care about colors being correct in our node files. --- gsk/gskrendernodeparser.c | 17 +++++++++++++++-- testsuite/gsk/nodeparser/color4.errors | 3 ++- testsuite/gsk/nodeparser/color4.node | 6 +++++- testsuite/gsk/nodeparser/color4.ref.node | 4 ++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/gsk/gskrendernodeparser.c b/gsk/gskrendernodeparser.c index bacb5f1475..2b8b11932a 100644 --- a/gsk/gskrendernodeparser.c +++ b/gsk/gskrendernodeparser.c @@ -765,7 +765,7 @@ parse_color_arg (GtkCssParser *parser, { ColorArgData *d = data; GdkColorState *color_state; - float values[4]; + float values[4], clamped[4]; if (!parse_color_state (parser, d->context, &color_state)) return 0; @@ -794,7 +794,20 @@ parse_color_arg (GtkCssParser *parser, values[3] = 1; } - gdk_color_init (d->color, color_state, values); + gdk_color_state_clamp (color_state, values, clamped); + if (values[0] != clamped[0] || + values[1] != clamped[1] || + values[2] != clamped[2] || + values[3] != clamped[3]) + { + gtk_css_parser_error (parser, + GTK_CSS_PARSER_ERROR_UNKNOWN_VALUE, + gtk_css_parser_get_block_location (parser), + gtk_css_parser_get_end_location (parser), + "Color values out of range for color state"); + } + + gdk_color_init (d->color, color_state, clamped); return 1; } diff --git a/testsuite/gsk/nodeparser/color4.errors b/testsuite/gsk/nodeparser/color4.errors index f6574d0778..dda06456e4 100644 --- a/testsuite/gsk/nodeparser/color4.errors +++ b/testsuite/gsk/nodeparser/color4.errors @@ -1 +1,2 @@ -:2:27-28: error: GTK_CSS_PARSER_ERROR_SYNTAX +:2:10-27: error: GTK_CSS_PARSER_ERROR_UNKNOWN_VALUE +:6:27-28: error: GTK_CSS_PARSER_ERROR_SYNTAX diff --git a/testsuite/gsk/nodeparser/color4.node b/testsuite/gsk/nodeparser/color4.node index a2bbe4b7d4..9a43739bf1 100644 --- a/testsuite/gsk/nodeparser/color4.node +++ b/testsuite/gsk/nodeparser/color4.node @@ -1,3 +1,7 @@ color { - color: color(srgb 1 2 3 4 5 6); + color: color(srgb 1 2 3); +} + +color { + color: color(srgb 1 1 1 1 1 1); } diff --git a/testsuite/gsk/nodeparser/color4.ref.node b/testsuite/gsk/nodeparser/color4.ref.node index 15a0f7553a..902ae198b1 100644 --- a/testsuite/gsk/nodeparser/color4.ref.node +++ b/testsuite/gsk/nodeparser/color4.ref.node @@ -2,3 +2,7 @@ color { bounds: 0 0 50 50; color: rgb(255,255,255); } +color { + bounds: 0 0 50 50; + color: rgb(255,255,255); +} From 96139a902da1ab4df9d4329370f7a5149a857ad1 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 16 Aug 2024 06:11:39 +0200 Subject: [PATCH 5/5] gsk: Don't print any sRGB color as rgb() or rgba() If the color value is not inside the proper range for rgb() or rgba() values being integers, use color() instead. Tests added/adapted. --- gsk/gskrendernodeparser.c | 5 ++++- testsuite/gsk/meson.build | 1 + testsuite/gsk/nodeparser/color.ref.node | 2 +- testsuite/gsk/nodeparser/srgb-high-accuracy.node | 4 ++++ 4 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 testsuite/gsk/nodeparser/srgb-high-accuracy.node diff --git a/gsk/gskrendernodeparser.c b/gsk/gskrendernodeparser.c index 2b8b11932a..428adee670 100644 --- a/gsk/gskrendernodeparser.c +++ b/gsk/gskrendernodeparser.c @@ -3678,7 +3678,10 @@ static void print_color (Printer *p, const GdkColor *color) { - if (gdk_color_state_equal (color->color_state, GDK_COLOR_STATE_SRGB)) + if (gdk_color_state_equal (color->color_state, GDK_COLOR_STATE_SRGB) && + round (CLAMP (color->red, 0, 1) * 255) == color->red * 255 && + round (CLAMP (color->green, 0, 1) * 255) == color->green * 255 && + round (CLAMP (color->blue, 0, 1) * 255) == color->blue * 255) { gdk_rgba_print ((const GdkRGBA *) color->values, p->str); } diff --git a/testsuite/gsk/meson.build b/testsuite/gsk/meson.build index 8cf9d5a1dc..2f5e0ce04b 100644 --- a/testsuite/gsk/meson.build +++ b/testsuite/gsk/meson.build @@ -431,6 +431,7 @@ node_parser_tests = [ 'shadow-fail.node', 'shadow-fail.ref.node', 'shadow-fail.errors', + 'srgb-high-accuracy.node', 'string-error.errors', 'string-error.node', 'string-error.ref.node', diff --git a/testsuite/gsk/nodeparser/color.ref.node b/testsuite/gsk/nodeparser/color.ref.node index 9b50e72cb7..056e485cdd 100644 --- a/testsuite/gsk/nodeparser/color.ref.node +++ b/testsuite/gsk/nodeparser/color.ref.node @@ -4,7 +4,7 @@ color { } color { bounds: 100 100 200 300; - color: rgb(1,1,0); + color: color(srgb 0.00392157 0.00196078 0.00117647); } color { bounds: 100 100 200 300; diff --git a/testsuite/gsk/nodeparser/srgb-high-accuracy.node b/testsuite/gsk/nodeparser/srgb-high-accuracy.node new file mode 100644 index 0000000000..1ab70c5ce2 --- /dev/null +++ b/testsuite/gsk/nodeparser/srgb-high-accuracy.node @@ -0,0 +1,4 @@ +color { + bounds: 0 0 50 50; + color: color(srgb 0.999 0 0); +}