From 5d9dc25115e111c71d27ea765cf1a31fa0f256ef Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 8 Jun 2020 18:06:01 +0200 Subject: [PATCH 1/3] listitemmanager: Update selections properly Replace a previous fix with a more correct one: Update the selected state from the model instead of reusing the old state, the model might have updated the selected state. --- gtk/gtklistitemmanager.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/gtk/gtklistitemmanager.c b/gtk/gtklistitemmanager.c index e08592bca8..34c52e48bd 100644 --- a/gtk/gtklistitemmanager.c +++ b/gtk/gtklistitemmanager.c @@ -395,7 +395,6 @@ gtk_list_item_manager_add_items (GtkListItemManager *self, if (item == NULL || item->widget) item = gtk_rb_tree_insert_before (self->items, item); - item->n_items += n_items; gtk_rb_tree_node_mark_dirty (item); @@ -570,12 +569,6 @@ gtk_list_item_manager_ensure_items (GtkListItemManager *self, gtk_list_item_manager_release_list_item (self, NULL, widget); } -static void -gtk_list_item_manager_model_selection_changed_cb (GListModel *model, - guint position, - guint n_items, - GtkListItemManager *self); - static void gtk_list_item_manager_model_items_changed_cb (GListModel *model, guint position, @@ -742,8 +735,6 @@ gtk_list_item_manager_model_items_changed_cb (GListModel *model, g_hash_table_unref (change); - gtk_list_item_manager_model_selection_changed_cb (model, position, added, self); - gtk_widget_queue_resize (self->widget); } @@ -988,7 +979,7 @@ gtk_list_item_manager_try_reacquire_list_item (GtkListItemManager *self, gtk_list_item_widget_update (list_item, position, gtk_list_item_widget_get_item (list_item), - gtk_list_item_widget_get_selected (list_item)); + gtk_selection_model_is_selected (self->model, position)); gtk_widget_insert_after (result, self->widget, prev_sibling); /* XXX: Should we let the listview do this? */ gtk_widget_queue_resize (result); From d294b01cee804c95024f1623289686d77c549d62 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 8 Jun 2020 18:26:48 +0200 Subject: [PATCH 2/3] selectionmodel: Rename "exclusive" to "unselect_rest" The name is better at explaining what this boolean is meant to do. --- gtk/gtkselectionmodel.c | 16 ++++++++-------- gtk/gtkselectionmodel.h | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/gtk/gtkselectionmodel.c b/gtk/gtkselectionmodel.c index 9deb1e32de..c2edaa2a51 100644 --- a/gtk/gtkselectionmodel.c +++ b/gtk/gtkselectionmodel.c @@ -85,7 +85,7 @@ gtk_selection_model_default_is_selected (GtkSelectionModel *model, static gboolean gtk_selection_model_default_select_item (GtkSelectionModel *model, guint position, - gboolean exclusive) + gboolean unselect_rest) { return FALSE; } @@ -100,7 +100,7 @@ static gboolean gtk_selection_model_default_select_range (GtkSelectionModel *model, guint position, guint n_items, - gboolean exclusive) + gboolean unselect_rest) { return FALSE; } @@ -228,21 +228,21 @@ gtk_selection_model_is_selected (GtkSelectionModel *model, * gtk_selection_model_select_item: * @model: a #GtkSelectionModel * @position: the position of the item to select - * @exclusive: whether previously selected items should be unselected + * @unselect_rest: whether previously selected items should be unselected * * Requests to select an item in the model. */ gboolean gtk_selection_model_select_item (GtkSelectionModel *model, guint position, - gboolean exclusive) + gboolean unselect_rest) { GtkSelectionModelInterface *iface; g_return_val_if_fail (GTK_IS_SELECTION_MODEL (model), 0); iface = GTK_SELECTION_MODEL_GET_IFACE (model); - return iface->select_item (model, position, exclusive); + return iface->select_item (model, position, unselect_rest); } /** @@ -269,7 +269,7 @@ gtk_selection_model_unselect_item (GtkSelectionModel *model, * @model: a #GtkSelectionModel * @position: the first item to select * @n_items: the number of items to select - * @exclusive: whether previously selected items should be unselected + * @unselect_rest: whether previously selected items should be unselected * * Requests to select a range of items in the model. */ @@ -277,14 +277,14 @@ gboolean gtk_selection_model_select_range (GtkSelectionModel *model, guint position, guint n_items, - gboolean exclusive) + gboolean unselect_rest) { GtkSelectionModelInterface *iface; g_return_val_if_fail (GTK_IS_SELECTION_MODEL (model), 0); iface = GTK_SELECTION_MODEL_GET_IFACE (model); - return iface->select_range (model, position, n_items, exclusive); + return iface->select_range (model, position, n_items, unselect_rest); } /** diff --git a/gtk/gtkselectionmodel.h b/gtk/gtkselectionmodel.h index 86657fba15..9a93a6a399 100644 --- a/gtk/gtkselectionmodel.h +++ b/gtk/gtkselectionmodel.h @@ -97,13 +97,13 @@ struct _GtkSelectionModelInterface gboolean (* select_item) (GtkSelectionModel *model, guint position, - gboolean exclusive); + gboolean unselect_rest); gboolean (* unselect_item) (GtkSelectionModel *model, guint position); gboolean (* select_range) (GtkSelectionModel *model, guint position, guint n_items, - gboolean exclusive); + gboolean unselect_rest); gboolean (* unselect_range) (GtkSelectionModel *model, guint position, guint n_items); @@ -129,7 +129,7 @@ gboolean gtk_selection_model_is_selected (GtkSelectionMod GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_select_item (GtkSelectionModel *model, guint position, - gboolean exclusive); + gboolean unselect_rest); GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_unselect_item (GtkSelectionModel *model, guint position); @@ -137,7 +137,7 @@ GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_select_range (GtkSelectionModel *model, guint position, guint n_items, - gboolean exclusive); + gboolean unselect_rest); GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_unselect_range (GtkSelectionModel *model, guint position, From 541aaa23920d1e6ac27b186334aadde78a3ecf43 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 8 Jun 2020 18:47:44 +0200 Subject: [PATCH 3/3] selectionmodel: Add unselect_rest argument to select_callback This is not just about consistency with other functions. It is about avoiding reentrancy problems. GtkListBase first doing an unselect_all() will then force the SelectionModel to consider a state where all items are unselected (and potentially deciding to autoselect one) and then cause a "selection-changed" emission that unselects all items and potentially updates all the list item widgets in the GtkListBase to the unselected state. After this, GtkListBase selects new items, but to the SelectionModel and the list item widgets this looks like an enitrely new operation and there is no way to associate it with the previous state, so the SelectionModel cannot undo any previous actions it took when unselecting. And all listitem widgets will now think they were just selected and start running animations about selecting. --- gtk/gtklistbase.c | 7 +++---- gtk/gtkmultiselection.c | 13 +++++++++++-- gtk/gtkpropertyselection.c | 20 ++++++++++++++++---- gtk/gtkselectionmodel.c | 5 ++++- gtk/gtkselectionmodel.h | 2 ++ testsuite/gtk/multiselection.c | 2 +- 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/gtk/gtklistbase.c b/gtk/gtklistbase.c index 27e5075ec3..507e3c81fd 100644 --- a/gtk/gtklistbase.c +++ b/gtk/gtklistbase.c @@ -1389,10 +1389,9 @@ gtk_list_base_stop_rubberband (GtkListBase *self) } else { - if (!priv->rubberband->extend) - gtk_selection_model_unselect_all (model); - - gtk_selection_model_select_callback (model, range_cb, priv->rubberband->active); + gtk_selection_model_select_callback (model, + !priv->rubberband->extend, + range_cb, priv->rubberband->active); } g_clear_pointer (&priv->rubberband, rubberband_data_free); diff --git a/gtk/gtkmultiselection.c b/gtk/gtkmultiselection.c index a852972bee..70045b525c 100644 --- a/gtk/gtkmultiselection.c +++ b/gtk/gtkmultiselection.c @@ -175,6 +175,7 @@ gtk_multi_selection_unselect_all (GtkSelectionModel *model) static gboolean gtk_multi_selection_add_or_remove (GtkSelectionModel *model, + gboolean unselect_rest, gboolean add, GtkSelectionCallback callback, gpointer data) @@ -190,6 +191,13 @@ gtk_multi_selection_add_or_remove (GtkSelectionModel *model, min = G_MAXUINT; max = 0; + if (unselect_rest) + { + min = gtk_set_get_min (self->selected); + max = gtk_set_get_max (self->selected); + gtk_set_remove_all (self->selected); + } + for (pos = 0; pos < n; pos = start + n_items) { callback (pos, &start, &n_items, &in, data); @@ -223,10 +231,11 @@ gtk_multi_selection_add_or_remove (GtkSelectionModel *model, static gboolean gtk_multi_selection_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { - return gtk_multi_selection_add_or_remove (model, TRUE, callback, data); + return gtk_multi_selection_add_or_remove (model, unselect_rest, TRUE, callback, data); } static gboolean @@ -234,7 +243,7 @@ gtk_multi_selection_unselect_callback (GtkSelectionModel *model, GtkSelectionCallback callback, gpointer data) { - return gtk_multi_selection_add_or_remove (model, FALSE, callback, data); + return gtk_multi_selection_add_or_remove (model, FALSE, FALSE, callback, data); } static void diff --git a/gtk/gtkpropertyselection.c b/gtk/gtkpropertyselection.c index 948e75ae2d..3f4c192ba1 100644 --- a/gtk/gtkpropertyselection.c +++ b/gtk/gtkpropertyselection.c @@ -210,16 +210,24 @@ gtk_property_selection_unselect_all (GtkSelectionModel *model) static gboolean gtk_property_selection_add_or_remove (GtkSelectionModel *model, + gboolean unselect_rest, gboolean add, GtkSelectionCallback callback, gpointer data) { GtkPropertySelection *self = GTK_PROPERTY_SELECTION (model); - guint pos, start, n; + guint pos, start, n, n_items; gboolean in; guint min, max; guint i; + n_items = g_list_model_get_n_items (G_LIST_MODEL (self)); + if (unselect_rest) + { + for (i = 0; i < n_items; i++) + set_selected (self, i, FALSE); + } + min = G_MAXUINT; max = 0; @@ -241,7 +249,10 @@ gtk_property_selection_add_or_remove (GtkSelectionModel *model, } while (n > 0); - if (min <= max) + /* FIXME: do better here */ + if (unselect_rest) + gtk_selection_model_selection_changed (model, 0, n_items); + else if (min <= max) gtk_selection_model_selection_changed (model, min, max - min + 1); return TRUE; @@ -249,10 +260,11 @@ gtk_property_selection_add_or_remove (GtkSelectionModel *model, static gboolean gtk_property_selection_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { - return gtk_property_selection_add_or_remove (model, TRUE, callback, data); + return gtk_property_selection_add_or_remove (model, unselect_rest, TRUE, callback, data); } static gboolean @@ -260,7 +272,7 @@ gtk_property_selection_unselect_callback (GtkSelectionModel *model, GtkSelectionCallback callback, gpointer data) { - return gtk_property_selection_add_or_remove (model, FALSE, callback, data); + return gtk_property_selection_add_or_remove (model, FALSE, FALSE, callback, data); } static void diff --git a/gtk/gtkselectionmodel.c b/gtk/gtkselectionmodel.c index c2edaa2a51..1bd62e810d 100644 --- a/gtk/gtkselectionmodel.c +++ b/gtk/gtkselectionmodel.c @@ -115,6 +115,7 @@ gtk_selection_model_default_unselect_range (GtkSelectionModel *model, static gboolean gtk_selection_model_default_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { @@ -345,6 +346,7 @@ gtk_selection_model_unselect_all (GtkSelectionModel *model) /** * gtk_selection_model_select_callback: * @model: a #GtkSelectionModel + * @unselect_rest: whether previously selected items should be unselected * @callback: (scope call): a #GtkSelectionCallback to determine items to select * @data: data to pass to @callback * @@ -353,12 +355,13 @@ gtk_selection_model_unselect_all (GtkSelectionModel *model) */ gboolean gtk_selection_model_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { g_return_val_if_fail (GTK_IS_SELECTION_MODEL (model), FALSE); - return GTK_SELECTION_MODEL_GET_IFACE (model)->select_callback (model, callback, data); + return GTK_SELECTION_MODEL_GET_IFACE (model)->select_callback (model, unselect_rest, callback, data); } /** diff --git a/gtk/gtkselectionmodel.h b/gtk/gtkselectionmodel.h index 9a93a6a399..55a012e3d6 100644 --- a/gtk/gtkselectionmodel.h +++ b/gtk/gtkselectionmodel.h @@ -110,6 +110,7 @@ struct _GtkSelectionModelInterface gboolean (* select_all) (GtkSelectionModel *model); gboolean (* unselect_all) (GtkSelectionModel *model); gboolean (* select_callback) (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data); gboolean (* unselect_callback) (GtkSelectionModel *model, @@ -149,6 +150,7 @@ gboolean gtk_selection_model_unselect_all (GtkSelectionMod GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data); GDK_AVAILABLE_IN_ALL diff --git a/testsuite/gtk/multiselection.c b/testsuite/gtk/multiselection.c index f56387a878..3c59d307f3 100644 --- a/testsuite/gtk/multiselection.c +++ b/testsuite/gtk/multiselection.c @@ -496,7 +496,7 @@ test_callback (void) assert_selection (selection, ""); assert_selection_changes (selection, ""); - ret = gtk_selection_model_select_callback (selection, select_some, data); + ret = gtk_selection_model_select_callback (selection, FALSE, select_some, data); g_assert_true (ret); assert_selection (selection, "3 4 5 7 8 9"); assert_selection_changes (selection, "2:7");