From 972669d6363c163ed6d3b737cbd6b1bd534f3d7b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 20 Aug 2012 05:48:07 -0700 Subject: [PATCH] Rework dive selection logic This completely changes how we keep track of selected dives: instead of having an array listing the selection ("selectiontracker") or trusting the gtk selection information, just save the information about whether a dive is selected in the dive itself. That makes it trivial to keep track of the state of selection across group collapse/expand events, or when changing the tree view model. It also ends up simplifying the code and logic in other ways. HOWEVER, it does currently (re-)introduce an annoying oddity with gtk: if you collapse a dive trip that has individual selections, gtk will forget those selections ("out of sight, out of mind"), and when you do *new* selections, the old hidden ones remain. So there's some games required to make gtk do sane things. We may need to either explicitly drop selections when collapsing trips, or make sure the group entry gets selected when collapsing a group that has selections in it. Or something. There may be other issues introduced by this too. Signed-off-by: Linus Torvalds --- display-gtk.h | 2 +- dive.h | 4 +- divelist.c | 306 +++++++++++++++++++++++--------------------------- info.c | 54 +++++---- profile.c | 1 - statistics.c | 21 ++-- 6 files changed, 175 insertions(+), 213 deletions(-) diff --git a/display-gtk.h b/display-gtk.h index 1f14307..dd7e2c4 100644 --- a/display-gtk.h +++ b/display-gtk.h @@ -77,7 +77,7 @@ extern GtkWidget *dive_list_create(void); unsigned int amount_selected; -extern void process_selected_dives(GList *, int *, GtkTreeModel *); +extern void process_selected_dives(void); typedef void (*data_func_t)(GtkTreeViewColumn *col, GtkCellRenderer *renderer, diff --git a/dive.h b/dive.h index 41f427a..cc27ab8 100644 --- a/dive.h +++ b/dive.h @@ -236,6 +236,7 @@ struct event { struct dive { int number; + int selected; time_t when; char *location; char *notes; @@ -284,7 +285,6 @@ struct dive_table { extern struct dive_table dive_table; -extern int *selectiontracker; extern int selected_dive; #define current_dive (get_dive(selected_dive)) @@ -355,7 +355,7 @@ extern void evn_foreach(void (*callback)(const char *, int *, void *), void *dat extern int add_new_dive(struct dive *dive); extern int edit_dive_info(struct dive *dive); -extern int edit_multi_dive_info(int nr, int *indices); +extern int edit_multi_dive_info(int idx); extern void dive_list_update_dives(void); extern void flush_divelist(struct dive *dive); diff --git a/divelist.c b/divelist.c index 0cb03f3..1c475af 100644 --- a/divelist.c +++ b/divelist.c @@ -78,85 +78,21 @@ static void dump_model(GtkListStore *store) } #endif -static GList *selected_dives; -static int st_size = 0; - -gboolean is_in_st(int idx, int *atpos) -{ - int i; - - for (i = 0; i < amount_selected; i++) - if (selectiontracker[i] == idx) { - if (atpos) - *atpos = i; - return TRUE; - } - return FALSE; -} - #if DEBUG_SELECTION_TRACKING void dump_selection(void) { int i; + struct dive *dive; - printf("currently selected are "); - for (i = 0; i < amount_selected; i++) - printf("%d ", selectiontracker[i]); + printf("currently selected are %d dives:", amount_selected); + for (i = 0; (dive = get_dive(i)) != NULL; i++) { + if (dive->selected) + printf(" %d", i); + } printf("\n"); } #endif -void track_select(int idx) -{ - if (idx < 0) - return; - -#if DEBUG_SELECTION_TRACKING - printf("add %d to selection of %d entries\n", idx, amount_selected); -#endif - if (is_in_st(idx, NULL)) - return; - if (amount_selected >= st_size) { - selectiontracker = realloc(selectiontracker, dive_table.nr * sizeof(int)); - st_size = dive_table.nr; - } - selectiontracker[amount_selected] = idx; - amount_selected++; - if (amount_selected == 1) - selected_dive = idx; -#if DEBUG_SELECTION_TRACKING - printf("increased amount_selected to %d\n", amount_selected); - dump_selection(); -#endif -} - -void track_unselect(int idx) -{ - if (idx < 0) - return; - -#if DEBUG_SELECTION_TRACKING - printf("remove %d from selection of %d entries\n", idx, amount_selected); -#endif - int atpos; - - if (! is_in_st(idx, &atpos)) - return; - memmove(selectiontracker + atpos, - selectiontracker + atpos + 1, - (amount_selected - atpos - 1) * sizeof(int)); - amount_selected--; -#if DEBUG_SELECTION_TRACKING - printf("removed %d at pos %d and decreased amount_selected to %d\n", idx, atpos, amount_selected); - dump_selection(); -#endif -} - -void clear_tracker(void) -{ - amount_selected = 0; -} - /* when subsurface starts we want to have the last dive selected. So we simply walk to the first leaf (and skip the summary entries - which have negative DIVE_INDEX) */ @@ -177,97 +113,137 @@ static void first_leaf(GtkTreeModel *model, GtkTreeIter *iter, int *diveidx) } } -/* if we click on a summary dive, we actually want to select / unselect - all the dives "below" it */ -static void select_children(GtkTreeModel *model, GtkTreeSelection * selection, - GtkTreeIter *iter, gboolean was_selected) -{ - int i, nr_children; - gboolean expanded = FALSE; - GtkTreeIter parent; - GtkTreePath *tpath; - - memcpy(&parent, iter, sizeof(parent)); - - tpath = gtk_tree_model_get_path(model, &parent); - expanded = gtk_tree_view_row_expanded(GTK_TREE_VIEW(dive_list.tree_view), tpath); - nr_children = gtk_tree_model_iter_n_children(model, &parent); - for (i = 0; i < nr_children; i++) { - gtk_tree_model_iter_nth_child(model, iter, &parent, i); - - /* if the parent is expanded, just (un)select the children and we'll - track their selection status in the callback - otherwise just change the selection status directly without - bothering gtk */ - if (expanded) { - if (was_selected) - gtk_tree_selection_unselect_iter(selection, iter); - else - gtk_tree_selection_select_iter(selection, iter); - } else { - int idx; - gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, -1); - if (was_selected) - track_unselect(idx); - else - track_select(idx); - } - } -} - /* make sure that if we expand a summary row that is selected, the children show up as selected, too */ void row_expanded_cb(GtkTreeView *tree_view, GtkTreeIter *iter, GtkTreePath *path, gpointer data) { + GtkTreeIter child; + GtkTreeModel *model = GTK_TREE_MODEL(dive_list.model); GtkTreeSelection *selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(dive_list.tree_view)); - if (gtk_tree_selection_path_is_selected(selection, path)) - select_children(GTK_TREE_MODEL(dive_list.model), selection, iter, FALSE); + if (!gtk_tree_model_iter_children(model, &child, iter)) + return; + + do { + int idx; + struct dive *dive; + + gtk_tree_model_get(model, &child, DIVE_INDEX, &idx, -1); + dive = get_dive(idx); + + if (dive->selected) + gtk_tree_selection_select_iter(selection, &child); + else + gtk_tree_selection_unselect_iter(selection, &child); + } while (gtk_tree_model_iter_next(model, &child)); } -/* this is called _before_ the selection is changed, for every single entry; - * we simply have it call down the tree to make sure that summary items toggle - * their children */ +static GList *selection_changed = NULL; + +/* + * This is called _before_ the selection is changed, for every single entry; + * + * We simply create a list of all changed entries, and make sure that the + * group entries go at the end of the list. + */ gboolean modify_selection_cb(GtkTreeSelection *selection, GtkTreeModel *model, GtkTreePath *path, gboolean was_selected, gpointer userdata) { - GtkTreeIter iter; - int dive_idx; + GtkTreeIter iter, *p; - /* if gtk thinks nothing is selected we should clear out our - tracker as well - otherwise hidden selected rows can stay - "stuck". The down side is that we now have a different bug: - If you select a dive, collapse the dive trip and ctrl-click - another dive trip, the initial dive is no longer selected. - Just don't do that, ok? */ - if (gtk_tree_selection_count_selected_rows(selection) == 0) - clear_tracker(); + if (!gtk_tree_model_get_iter(model, &iter, path)) + return TRUE; - if (gtk_tree_model_get_iter(model, &iter, path)) { - gtk_tree_model_get(model, &iter, DIVE_INDEX, &dive_idx, -1); - /* turns out we need to move the selectiontracker here */ + /* Add the group entries to the end */ + p = gtk_tree_iter_copy(&iter); + if (gtk_tree_model_iter_has_child(model, p)) + selection_changed = g_list_append(selection_changed, p); + else + selection_changed = g_list_prepend(selection_changed, p); + return TRUE; +} -#if DEBUG_SELECTION_TRACKING - printf("modify_selection_cb with idx %d (according to gtk was %sselected)\n", - dive_idx, was_selected ? "" : "un"); -#endif - if (dive_idx >= 0) { - if (was_selected) - track_unselect(dive_idx); - else - track_select(dive_idx); - } else { - select_children(model, selection, &iter, was_selected); - } +static void select_dive(struct dive *dive, int selected) +{ + if (dive->selected != selected) { + amount_selected += selected ? 1 : -1; + dive->selected = selected; } - /* allow this selection to proceed */ - return TRUE; +} + +/* + * This gets called when a dive group has changed selection. + */ +static void select_dive_group(GtkTreeModel *model, GtkTreeSelection *selection, GtkTreeIter *iter, int selected) +{ + int first = 1; + GtkTreeIter child; + + if (!gtk_tree_model_iter_children(model, &child, iter)) + return; + + do { + int idx; + struct dive *dive; + + gtk_tree_model_get(model, &child, DIVE_INDEX, &idx, -1); + if (first && selected) + selected_dive = idx; + first = 0; + dive = get_dive(idx); + if (dive->selected == selected) + break; + + select_dive(dive, selected); + if (selected) + gtk_tree_selection_select_iter(selection, &child); + else + gtk_tree_selection_unselect_iter(selection, &child); + } while (gtk_tree_model_iter_next(model, &child)); +} + +/* + * This gets called _after_ the selections have changed, for each entry that + * may have changed. Check if the gtk selection state matches our internal + * selection state to verify. + * + * The group entries are at the end, this guarantees that we have handled + * all the dives before we handle groups. + */ +static void check_selection_cb(GtkTreeIter *iter, GtkTreeSelection *selection) +{ + GtkTreeModel *model = GTK_TREE_MODEL(dive_list.model); + struct dive *dive; + int idx, gtk_selected; + + gtk_tree_model_get(model, iter, + DIVE_INDEX, &idx, + -1); + dive = get_dive(idx); + gtk_selected = gtk_tree_selection_iter_is_selected(selection, iter); + if (idx < 0) + select_dive_group(model, selection, iter, gtk_selected); + else { + select_dive(dive, gtk_selected); + if (gtk_selected) + selected_dive = idx; + } + gtk_tree_iter_free(iter); } /* this is called when gtk thinks that the selection has changed */ -static void selection_cb(GtkTreeSelection *selection, gpointer userdata) +static void selection_cb(GtkTreeSelection *selection, GtkTreeModel *model) { - process_selected_dives(selected_dives, selectiontracker, GTK_TREE_MODEL(dive_list.model)); + GList *changed = selection_changed; + + selection_changed = NULL; + g_list_foreach(changed, (GFunc) check_selection_cb, selection); + g_list_free(changed); +#if DEBUG_SELECTION_TRACKING + dump_selection(); +#endif + + process_selected_dives(); repaint_dive(); } @@ -1098,7 +1074,7 @@ void add_dive_cb(GtkWidget *menuitem, gpointer data) void edit_dive_cb(GtkWidget *menuitem, gpointer data) { - edit_multi_dive_info(amount_selected, selectiontracker); + edit_multi_dive_info(-1); } static void expand_all_cb(GtkWidget *menuitem, GtkTreeView *tree_view) @@ -1161,8 +1137,6 @@ static gboolean button_press_cb(GtkWidget *treeview, GdkEventButton *event, gpoi when gtk_tree_selection_select_path is called. We also need to keep copies of the sort order so we can restore that as well after switching models. */ -static int *oldselection; -static int old_nr_selected; static gboolean second_call = FALSE; static GtkSortType sortorder[] = { [0 ... DIVELIST_COLUMNS - 1] = GTK_SORT_DESCENDING, }; static int lastcol = DIVE_DATE; @@ -1170,20 +1144,27 @@ static int lastcol = DIVE_DATE; /* Check if this dive was selected previously and select it again in the new model; * This is used after we switch models to maintain consistent selections. * We always return FALSE to iterate through all dives */ -static gboolean select_selected(GtkTreeModel *model, GtkTreePath *path, +static gboolean set_selected(GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, gpointer data) { - int i, idx; GtkTreeSelection *selection = GTK_TREE_SELECTION(data); + int idx, selected; + struct dive *dive; - gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, -1); - for (i = 0; i < old_nr_selected; i++) - if (oldselection[i] == idx) { - gtk_tree_view_expand_to_path(GTK_TREE_VIEW(dive_list.tree_view), path); - gtk_tree_selection_select_path(selection, path); - - return FALSE; - } + gtk_tree_model_get(model, iter, + DIVE_INDEX, &idx, + -1); + if (idx < 0) { + GtkTreeIter child; + if (gtk_tree_model_iter_children(model, &child, iter)) + gtk_tree_model_get(model, &child, DIVE_INDEX, &idx, -1); + } + dive = get_dive(idx); + selected = dive && dive->selected; + if (selected) { + gtk_tree_view_expand_to_path(GTK_TREE_VIEW(dive_list.tree_view), path); + gtk_tree_selection_select_path(selection, path); + } return FALSE; } @@ -1236,20 +1217,9 @@ static void sort_column_change_cb(GtkTreeSortable *treeview, gpointer data) if (dive_list.model != currentmodel) { GtkTreeSelection *selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(dive_list.tree_view)); - /* remember what is currently selected, switch models and reselect the selected rows */ - old_nr_selected = amount_selected; - oldselection = malloc(old_nr_selected * sizeof(int)); - if (amount_selected) - memcpy(oldselection, selectiontracker, amount_selected * sizeof(int)); gtk_tree_view_set_model(GTK_TREE_VIEW(dive_list.tree_view), GTK_TREE_MODEL(dive_list.model)); - update_column_and_order(colid); - - if (old_nr_selected) { - /* we need to select all the dives that were selected */ - /* this is fundamentally an n^2 algorithm as implemented - YUCK */ - gtk_tree_model_foreach(GTK_TREE_MODEL(dive_list.model), select_selected, selection); - } + gtk_tree_model_foreach(GTK_TREE_MODEL(dive_list.model), set_selected, selection); } else { if (order != sortorder[colid]) { update_column_and_order(colid); @@ -1328,7 +1298,7 @@ GtkWidget *dive_list_create(void) g_signal_connect(dive_list.tree_view, "row-expanded", G_CALLBACK(row_expanded_cb), NULL); g_signal_connect(dive_list.tree_view, "button-press-event", G_CALLBACK(button_press_cb), NULL); g_signal_connect(dive_list.tree_view, "popup-menu", G_CALLBACK(popup_menu_cb), NULL); - g_signal_connect(selection, "changed", G_CALLBACK(selection_cb), NULL); + g_signal_connect(selection, "changed", G_CALLBACK(selection_cb), dive_list.model); g_signal_connect(dive_list.listmodel, "sort-column-changed", G_CALLBACK(sort_column_change_cb), NULL); g_signal_connect(dive_list.treemodel, "sort-column-changed", G_CALLBACK(sort_column_change_cb), NULL); diff --git a/info.c b/info.c index adc6c59..468f357 100644 --- a/info.c +++ b/info.c @@ -166,7 +166,7 @@ static int delete_dive_info(struct dive *dive) static void info_menu_edit_cb(GtkMenuItem *menuitem, gpointer user_data) { - edit_multi_dive_info(amount_selected, selectiontracker); + edit_multi_dive_info(-1); } static void info_menu_delete_cb(GtkMenuItem *menuitem, gpointer user_data) @@ -489,15 +489,14 @@ void update_equipment_data(struct dive *dive, struct dive *master) memcpy(dive->weightsystem, master->weightsystem, WS_BYTES); } -int edit_multi_dive_info(int nr, int *indices) +/* A negative index means "all selected" */ +int edit_multi_dive_info(int index) { - int success, i; + int success; GtkWidget *dialog, *vbox; struct dive_info info; struct dive *master; - if (!nr) - return 0; dialog = gtk_dialog_new_with_buttons("Dive Info", GTK_WINDOW(main_window), GTK_DIALOG_DESTROY_WITH_PARENT, @@ -506,34 +505,31 @@ int edit_multi_dive_info(int nr, int *indices) NULL); vbox = gtk_dialog_get_content_area(GTK_DIALOG(dialog)); - /* SCARY STUFF - IS THIS THE BEST WAY TO DO THIS??? - * - * current_dive is one of our selected dives - and that is - * the one that is used to pre-fill the edit widget. Its - * data is used as the starting point for all selected dives - * I think it would be better to somehow collect and combine - * info from all the selected dives */ - master = current_dive; - dive_info_widget(vbox, master, &info, (nr > 1)); + master = get_dive(index); + if (!master) + master = current_dive; + dive_info_widget(vbox, master, &info, index < 0); show_dive_equipment(master, W_IDX_SECONDARY); save_equipment_data(master); gtk_widget_show_all(dialog); success = gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT; if (success) { - /* Update the other non-current dives first */ - for (i = 0; i < nr; i++) { - int idx = indices[i]; - struct dive *dive = get_dive(idx); - - if (!dive || dive == master) - continue; - /* copy all "info" fields */ - save_dive_info_changes(dive, master, &info); - /* copy the cylinders / weightsystems */ - update_equipment_data(dive, master); - /* this is extremely inefficient... it loops through all - dives to find the right one - but we KNOW the index already */ - flush_divelist(dive); + /* Update the non-current selected dives first */ + if (index < 0) { + int i; + struct dive *dive; + + for (i = 0; (dive = get_dive(i)) != NULL; i++) { + if (dive == master || !dive->selected) + continue; + /* copy all "info" fields */ + save_dive_info_changes(dive, master, &info); + /* copy the cylinders / weightsystems */ + update_equipment_data(dive, master); + /* this is extremely inefficient... it loops through all + dives to find the right one - but we KNOW the index already */ + flush_divelist(dive); + } } /* Update the master dive last! */ @@ -553,7 +549,7 @@ int edit_dive_info(struct dive *dive) if (!dive) return 0; idx = dive->number; - return edit_multi_dive_info(1, &idx); + return edit_multi_dive_info(idx); } static GtkWidget *frame_box(GtkWidget *vbox, const char *fmt, ...) diff --git a/profile.c b/profile.c index d336243..9618c46 100644 --- a/profile.c +++ b/profile.c @@ -14,7 +14,6 @@ #include "color.h" int selected_dive = 0; -int *selectiontracker; typedef enum { STABLE, SLOW, MODERATE, FAST, CRAZY } velocity_t; diff --git a/statistics.c b/statistics.c index 0f6fbe0..0a23f90 100644 --- a/statistics.c +++ b/statistics.c @@ -143,24 +143,21 @@ static void process_all_dives(struct dive *dive, struct dive **prev_dive) } /* make sure we skip the selected summary entries */ -void process_selected_dives(GList *selected_dives, int *selectiontracker, GtkTreeModel *model) +void process_selected_dives(void) { - struct dive *dp; - unsigned int i; - int idx; + struct dive *dive; + unsigned int i, nr; memset(&stats_selection, 0, sizeof(stats_selection)); - for (i = 0; i < amount_selected; ++i) { - idx = selectiontracker[i]; - if (idx > 0) { - dp = get_dive(idx); - if (dp) { - process_dive(dp, &stats_selection); - } + nr = 0; + for (i = 0; (dive = get_dive(i)) != NULL; ++i) { + if (dive->selected) { + process_dive(dive, &stats_selection); + nr++; } } - stats_selection.selection_size = amount_selected; + stats_selection.selection_size = nr; } static void set_label(GtkWidget *w, const char *fmt, ...) -- 2.43.0