From 26baa0527dd3e694b52f7186af4c194881a473ae Mon Sep 17 00:00:00 2001 From: Jehan Date: Thu, 10 Jun 2021 17:44:08 +0200 Subject: [PATCH] =?UTF-8?q?app:=20check=20if=20an=20item=20was=20already?= =?UTF-8?q?=20inserted=20by=20actually=20looking=20it=20up=20in=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … the container. There was this weird case which we somehow could only reproduce on Aryeom's computer/build, not mine, with the same code and reproduction steps (reproducible at will on her build only). Basically when drag'n dropping a duplicated layer inside a collapsed layer group, the row-expanded handler would try to select the moved layer before it is actually inserted. This would end up into crash-happy code. I'm still unsure of why the order of operation is different here, but anyway what is for sure is that the `inserting_item` boolean flag was not protecting much. It's not like it's an actual mutex and anyway this is not multi-threaded code either so this flag was mostly useless (which is why we were crashing). Instead let's actually look if the item is in the container or not. --- app/widgets/gimpitemtreeview.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/app/widgets/gimpitemtreeview.c b/app/widgets/gimpitemtreeview.c index ad6f66f3d0..8be456ea3e 100644 --- a/app/widgets/gimpitemtreeview.c +++ b/app/widgets/gimpitemtreeview.c @@ -95,8 +95,6 @@ struct _GimpItemTreeViewPrivate GimpTreeHandler *color_tag_changed_handler; GimpTreeHandler *lock_content_changed_handler; GimpTreeHandler *lock_position_changed_handler; - - gboolean inserting_item; /* EEK */ }; @@ -989,13 +987,9 @@ gimp_item_tree_view_insert_item (GimpContainerView *view, GdkColor gdk_color; gboolean has_color; - item_view->priv->inserting_item = TRUE; - iter = parent_view_iface->insert_item (view, viewable, parent_insert_data, index); - item_view->priv->inserting_item = FALSE; - has_color = gimp_get_color_tag_color (gimp_item_get_merged_color_tag (item), &color, gimp_item_get_color_tag (item) == @@ -1740,13 +1734,19 @@ gimp_item_tree_view_row_expanded (GtkTreeView *tree_view, GtkTreePath *path, GimpItemTreeView *item_view) { + GimpItemTreeViewClass *item_view_class; + GimpItem *active_item; + + item_view_class = GIMP_ITEM_TREE_VIEW_GET_CLASS (item_view); + active_item = item_view_class->get_active_item (item_view->priv->image); + /* don't select the item while it is being inserted */ - if (! item_view->priv->inserting_item) + if (active_item && + gimp_container_view_lookup (GIMP_CONTAINER_VIEW (item_view), + GIMP_VIEWABLE (active_item))) { - GimpItemTreeViewClass *item_view_class; - GimpViewRenderer *renderer; - GimpItem *expanded_item; - GimpItem *active_item; + GimpViewRenderer *renderer; + GimpItem *expanded_item; gtk_tree_model_get (GIMP_CONTAINER_TREE_VIEW (item_view)->model, iter, GIMP_CONTAINER_TREE_STORE_COLUMN_RENDERER, &renderer, @@ -1754,15 +1754,10 @@ gimp_item_tree_view_row_expanded (GtkTreeView *tree_view, expanded_item = GIMP_ITEM (renderer->viewable); g_object_unref (renderer); - item_view_class = GIMP_ITEM_TREE_VIEW_GET_CLASS (item_view); - - active_item = item_view_class->get_active_item (item_view->priv->image); - /* select the active item only if it was made visible by expanding * its immediate parent. See bug #666561. */ - if (active_item && - gimp_item_get_parent (active_item) == expanded_item) + if (gimp_item_get_parent (active_item) == expanded_item) { gimp_container_view_select_item (GIMP_CONTAINER_VIEW (item_view), GIMP_VIEWABLE (active_item));