From f458951745443ed7ed6258570ccf8f2908d65746 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 15 Apr 2023 00:02:01 +0200 Subject: [PATCH 1/3] inspector: Don't randomly emit application signals When the variant-editor emits a callback, it might not actually have edited the value in question. Try to detect that by only emitting signals if the value changed. --- gtk/inspector/action-editor.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/gtk/inspector/action-editor.c b/gtk/inspector/action-editor.c index ba838fbaf7..2065a48fd7 100644 --- a/gtk/inspector/action-editor.c +++ b/gtk/inspector/action-editor.c @@ -38,6 +38,7 @@ struct _GtkInspectorActionEditor gboolean enabled; const GVariantType *parameter_type; GVariantType *state_type; + GVariant *state; GtkWidget *activate_button; GtkWidget *parameter_entry; GtkWidget *state_entry; @@ -101,6 +102,11 @@ state_changed (GtkWidget *editor, return; g_variant_ref_sink (value); + if (g_variant_equal (value, r->state)) + { + g_variant_unref (value); + return; + } if (G_IS_ACTION_GROUP (r->owner)) g_action_group_change_action_state (G_ACTION_GROUP (r->owner), r->name, value); @@ -147,35 +153,35 @@ gtk_inspector_action_editor_init (GtkInspectorActionEditor *r) static void update_widgets (GtkInspectorActionEditor *r) { - GVariant *state; + g_clear_pointer (&r->state, g_variant_unref); if (G_IS_ACTION_GROUP (r->owner)) { if (!g_action_group_query_action (G_ACTION_GROUP (r->owner), r->name, &r->enabled, &r->parameter_type, NULL, NULL, - &state)) + &r->state)) { r->enabled = FALSE; r->parameter_type = NULL; - state = NULL; + r->state = NULL; } } else if (GTK_IS_ACTION_MUXER (r->owner)) { if (!gtk_action_muxer_query_action (GTK_ACTION_MUXER (r->owner), r->name, &r->enabled, &r->parameter_type, NULL, NULL, - &state)) + &r->state)) { r->enabled = FALSE; r->parameter_type = NULL; - state = NULL; + r->state = NULL; } } else { r->enabled = FALSE; r->parameter_type = NULL; - state = NULL; + r->state = NULL; } gtk_widget_set_sensitive (r->activate_button, r->enabled); @@ -184,15 +190,13 @@ update_widgets (GtkInspectorActionEditor *r) if (r->parameter_type) gtk_inspector_variant_editor_set_type (r->parameter_entry, r->parameter_type); - gtk_widget_set_visible (r->state_editor, state != NULL); - if (state) + gtk_widget_set_visible (r->state_editor, r->state != NULL); + if (r->state) { if (r->state_type) g_variant_type_free (r->state_type); - r->state_type = g_variant_type_copy (g_variant_get_type (state)); - gtk_inspector_variant_editor_set_value (r->state_entry, state); - - g_variant_unref (state); + r->state_type = g_variant_type_copy (g_variant_get_type (r->state)); + gtk_inspector_variant_editor_set_value (r->state_entry, r->state); } } @@ -207,6 +211,7 @@ dispose (GObject *object) g_clear_pointer (&r->name, g_free); g_clear_pointer (&r->state_type, g_variant_type_free); + g_clear_pointer (&r->state, g_variant_unref); G_OBJECT_CLASS (gtk_inspector_action_editor_parent_class)->dispose (object); } From 41454b63b41a05e1cb593a77d67fb20a26ca5510 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 15 Apr 2023 05:24:11 +0200 Subject: [PATCH 2/3] testsuite: Add some more exhaustive testing to listlistmodel --- testsuite/gtk/listlistmodel.c | 86 ++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/testsuite/gtk/listlistmodel.c b/testsuite/gtk/listlistmodel.c index 7d1f25aa35..d1898e23f8 100644 --- a/testsuite/gtk/listlistmodel.c +++ b/testsuite/gtk/listlistmodel.c @@ -71,7 +71,7 @@ free_changes (gpointer data) } static void -test_list_list_model (void) +test_change (void) { GtkWidget *box; GListModel *model; @@ -111,6 +111,87 @@ test_list_list_model (void) g_object_unref (box); } +static void +test_exhaustive (void) +{ + GtkBox *box; + GListModel *model, *compare; + guint i; + + box = GTK_BOX (gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 0)); + g_object_ref_sink (box); + + model = gtk_widget_observe_children (GTK_WIDGET (box)); + compare = G_LIST_MODEL (g_list_store_new (GTK_TYPE_WIDGET)); + + for (i = 0; i < 500; i++) + { + switch (g_test_rand_int_range (0, 4)) + { + case 0: + /* compare */ + g_assert_cmpint (g_list_model_get_n_items (model), ==, g_list_model_get_n_items (compare)); + if (g_list_model_get_n_items (compare) > 0) + { + guint n = g_list_model_get_n_items (compare); + guint step = n == 1 ? 1 : g_test_rand_int_range (1, n); + guint j = 0; + do + { + gpointer o1 = g_list_model_get_item (model, j); + gpointer o2 = g_list_model_get_item (compare, j); + g_assert_cmphex (GPOINTER_TO_SIZE (o1), ==, GPOINTER_TO_SIZE (o2)); + g_object_unref (o1); + g_object_unref (o2); + j = (j + step) % n; + } + while (j != 0); + } + break; + + case 1: + /* remove a widget */ + if (g_list_model_get_n_items (compare) > 0) + { + guint position = g_test_rand_int_range (0, g_list_model_get_n_items (compare)); + GtkWidget *child = g_list_model_get_item (compare, position); + gtk_box_remove (box, child); + g_list_store_remove (G_LIST_STORE (compare), position); + g_object_unref (child); + } + break; + + case 2: + /* add a widget */ + { + guint position = g_test_rand_int_range (0, g_list_model_get_n_items (compare) + 1); + GtkWidget *child = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 0); + GtkWidget *sibling; + if (position == 0) + sibling = NULL; + else + sibling = g_list_model_get_item (compare, position - 1); + gtk_box_insert_child_after (box, child, sibling); + g_list_store_insert (G_LIST_STORE (compare), position, child); + g_clear_object (&sibling); + } + break; + + case 3: + /* move a widget (FIXME) */ + break; + + default: + g_assert_not_reached (); + break; + } + } + + g_object_unref (compare); + g_object_unref (box); + g_object_unref (model); +} + int main (int argc, char *argv[]) { @@ -120,7 +201,8 @@ main (int argc, char *argv[]) changes_quark = g_quark_from_static_string ("What did I see? Can I believe what I saw?"); - g_test_add_func ("/listlistmodel/change", test_list_list_model); + g_test_add_func ("/listlistmodel/change", test_change); + g_test_add_func ("/listlistmodel/exhaustive", test_exhaustive); return g_test_run (); } From e010cd242c577fa4da3b05f67728dcbbe1781cb3 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sat, 15 Apr 2023 05:24:29 +0200 Subject: [PATCH 3/3] listlistmodel: Add a cache Cache the last looked up item and use it for looking up the next item if it's closest. This massively speeds up iteration over the model, because each call to get_item() will be adjacent to the previous one. Improves performance of the inspector quite a bit. --- gtk/gtklistlistmodel.c | 70 +++++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 11 deletions(-) diff --git a/gtk/gtklistlistmodel.c b/gtk/gtklistlistmodel.c index 9644b896c5..a35493b03b 100644 --- a/gtk/gtklistlistmodel.c +++ b/gtk/gtklistlistmodel.c @@ -41,6 +41,9 @@ struct _GtkListListModel gpointer (* get_item) (gpointer, gpointer); gpointer data; GDestroyNotify notify; + + guint cache_pos; + gpointer cache_item; }; struct _GtkListListModelClass @@ -72,6 +75,18 @@ gtk_list_list_model_get_n_items (GListModel *list) return self->n_items; } +static gboolean +gtk_list_list_model_cache_is_valid (GtkListListModel *self) +{ + return self->cache_item != NULL; +} + +static void +gtk_list_list_model_invalidate_cache (GtkListListModel *self) +{ + self->cache_item = NULL; +} + static gpointer gtk_list_list_model_get_item (GListModel *list, guint position) @@ -79,31 +94,50 @@ gtk_list_list_model_get_item (GListModel *list, GtkListListModel *self = GTK_LIST_LIST_MODEL (list); gpointer result; guint i; + guint start, end; if (position >= self->n_items) - { - return NULL; - } - else if (self->get_last && - position >= self->n_items / 2) - { - result = self->get_last (self->data); + return NULL; - for (i = self->n_items - 1; i > position; i--) + start = 0; + end = self->n_items; + if (gtk_list_list_model_cache_is_valid (self)) + { + if (self->cache_pos <= position) + start = self->cache_pos; + else + end = self->cache_pos; + } + + if (self->get_last && + position > (start + end) / 2) + { + if (end == self->cache_pos && gtk_list_list_model_cache_is_valid (self)) + result = self->get_previous (self->cache_item, self->data); + else + result = self->get_last (self->data); + + for (i = end - 1; i > position; i--) { result = self->get_previous (result, self->data); } } else { - result = self->get_first (self->data); + if (start == self->cache_pos && gtk_list_list_model_cache_is_valid (self)) + result = self->cache_item; + else + result = self->get_first (self->data); - for (i = 0; i < position; i++) + for (i = start; i < position; i++) { result = self->get_next (result, self->data); } } + self->cache_item = result; + self->cache_pos = position; + return self->get_item (result, self->data); } @@ -275,7 +309,9 @@ gtk_list_list_model_item_added_at (GtkListListModel *self, g_return_if_fail (GTK_IS_LIST_LIST_MODEL (self)); g_return_if_fail (position <= self->n_items); - self->n_items += 1; + self->n_items++; + if (position <= self->cache_pos) + self->cache_pos++; g_list_model_items_changed (G_LIST_MODEL (self), position, 0, 1); g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_N_ITEMS]); @@ -327,6 +363,12 @@ gtk_list_list_model_item_moved (GtkListListModel *self, min = MIN (position, previous_position); max = MAX (position, previous_position) + 1; + + if (self->cache_item == item) + self->cache_pos = position; + else if (self->cache_pos >= min && self->cache_pos < max) + self->cache_pos += (self->cache_pos > position ? 1 : -1); + g_list_model_items_changed (G_LIST_MODEL (self), min, max - min, max - min); } @@ -338,6 +380,10 @@ gtk_list_list_model_item_removed_at (GtkListListModel *self, g_return_if_fail (position < self->n_items); self->n_items -= 1; + if (position == self->cache_pos) + gtk_list_list_model_invalidate_cache (self); + else if (position < self->cache_pos) + self->cache_pos--; g_list_model_items_changed (G_LIST_MODEL (self), position, 1, 0); g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_N_ITEMS]); @@ -358,6 +404,8 @@ gtk_list_list_model_clear (GtkListListModel *self) self->n_items = 0; self->notify = NULL; + gtk_list_list_model_invalidate_cache (self); + if (n_items > 0) { g_list_model_items_changed (G_LIST_MODEL (self), 0, n_items, 0);