From 43a22bb3507ca9069ca3c3accd912ae39c744384 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 20 May 2023 07:39:21 -0400 Subject: [PATCH 1/7] Cosmetics Use the proper g_assert variant. --- testsuite/gtk/listitemmanager.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testsuite/gtk/listitemmanager.c b/testsuite/gtk/listitemmanager.c index ba5fbd54ba..d572f41404 100644 --- a/testsuite/gtk/listitemmanager.c +++ b/testsuite/gtk/listitemmanager.c @@ -123,7 +123,7 @@ check_list_item_manager (GtkListItemManager *items, case GTK_LIST_TILE_UNMATCHED_HEADER: g_assert_cmpint (section_state, ==, NO_SECTION); g_assert_cmpint (tile->n_items, ==, 0); - g_assert_false (tile->widget); + g_assert_null (tile->widget); section_state = UNMATCHED_SECTION; break; @@ -131,14 +131,14 @@ check_list_item_manager (GtkListItemManager *items, g_assert_cmpint (section_state, ==, MATCHED_SECTION); g_assert_cmpint (tile->n_items, ==, 0); g_assert_true (has_sections); - g_assert_false (tile->widget); + g_assert_null (tile->widget); section_state = NO_SECTION; break; case GTK_LIST_TILE_UNMATCHED_FOOTER: g_assert_cmpint (section_state, ==, UNMATCHED_SECTION); g_assert_cmpint (tile->n_items, ==, 0); - g_assert_false (tile->widget); + g_assert_null (tile->widget); section_state = NO_SECTION; break; @@ -167,7 +167,7 @@ check_list_item_manager (GtkListItemManager *items, case GTK_LIST_TILE_REMOVED: g_assert_cmpint (tile->n_items, ==, 0); - g_assert_false (tile->widget); + g_assert_null (tile->widget); break; default: From 28905572366477dad7f4211a31059df1feeb3da2 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 17 May 2023 19:26:12 -0400 Subject: [PATCH 2/7] Revert "gridview: GC tiles first" This reverts commit e121a5ca6f761fc4433187bf1ad13a093ede205d. The tile that was causing the critical in #5836 (what that commit was about) was a FILLER, and we are getting rid of FILLER tiles here. Which will avoid the issue in a more elegant way. --- gtk/gtkgridview.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/gtk/gtkgridview.c b/gtk/gtkgridview.c index ea2701d86c..94307af524 100644 --- a/gtk/gtkgridview.c +++ b/gtk/gtkgridview.c @@ -756,14 +756,8 @@ gtk_grid_view_size_allocate (GtkWidget *widget, min_row_height = ceil ((double) height / GTK_GRID_VIEW_MAX_VISIBLE_ROWS); gtk_list_base_get_border_spacing (GTK_LIST_BASE (self), &xspacing, &yspacing); - /* before we start: gc tiles */ - for (tile = gtk_list_tile_gc (self->item_manager, gtk_list_item_manager_get_first (self->item_manager)); - tile != NULL && tile->type == GTK_LIST_TILE_FILLER; - tile = gtk_list_tile_gc (self->item_manager, tile)) - {}; - /* step 0: exit early if list is empty */ - tile = gtk_list_item_manager_get_first (self->item_manager); + tile = gtk_list_tile_gc (self->item_manager, gtk_list_item_manager_get_first (self->item_manager)); if (tile == NULL) { gtk_list_base_allocate (GTK_LIST_BASE (self)); From e9731fc99b26dc28c5ddeeebeb67a9c1f4a18a94 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 17 May 2023 20:10:08 -0400 Subject: [PATCH 3/7] Add some tile helpers --- gtk/gtklistitemmanagerprivate.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gtk/gtklistitemmanagerprivate.h b/gtk/gtklistitemmanagerprivate.h index cc92ef74a9..fb8991fb69 100644 --- a/gtk/gtklistitemmanagerprivate.h +++ b/gtk/gtklistitemmanagerprivate.h @@ -96,6 +96,17 @@ GtkListTile * gtk_list_item_manager_get_nearest_tile (GtkListItemMana int x, int y); +static inline gboolean +gtk_list_tile_is_header (GtkListTile *tile) +{ + return tile->type == GTK_LIST_TILE_HEADER || tile->type == GTK_LIST_TILE_UNMATCHED_HEADER; +} + +static inline gboolean +gtk_list_tile_is_footer (GtkListTile *tile) +{ + return tile->type == GTK_LIST_TILE_FOOTER || tile->type == GTK_LIST_TILE_UNMATCHED_FOOTER; +} guint gtk_list_tile_get_position (GtkListItemManager *self, GtkListTile *tile); From 42a0dcc7e4e528268ae43b6cc6d20b5425d9dce9 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 17 May 2023 19:01:08 -0400 Subject: [PATCH 4/7] gridview: Add an assertion The want to use the footer tile at the end to fill leftover space at the bottome right. So lets assert that we actually dealing with a footer tile, just in case something changes in the future that might have us end up with some other kind of tile. --- gtk/gtkgridview.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gtk/gtkgridview.c b/gtk/gtkgridview.c index 94307af524..9618f8e5c2 100644 --- a/gtk/gtkgridview.c +++ b/gtk/gtkgridview.c @@ -885,6 +885,7 @@ gtk_grid_view_size_allocate (GtkWidget *widget, { GtkListTile *filler; tile = gtk_list_item_manager_get_last (self->item_manager); + g_assert (gtk_list_tile_is_footer (tile)); filler = gtk_list_tile_append_filler (self->item_manager, tile); gtk_list_tile_set_area_position (self->item_manager, filler, From 8cb3e01eef7605126d291348e1d4f2b6d5ed84d3 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 17 May 2023 19:09:43 -0400 Subject: [PATCH 5/7] gridview: Stop using a filler tile We can just use the footer to fill that space. --- gtk/gtkgridview.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/gtk/gtkgridview.c b/gtk/gtkgridview.c index 9618f8e5c2..63199d6bea 100644 --- a/gtk/gtkgridview.c +++ b/gtk/gtkgridview.c @@ -883,17 +883,16 @@ gtk_grid_view_size_allocate (GtkWidget *widget, /* Add a filler tile for empty space in the bottom right */ if (i > 0) { - GtkListTile *filler; - tile = gtk_list_item_manager_get_last (self->item_manager); - g_assert (gtk_list_tile_is_footer (tile)); - filler = gtk_list_tile_append_filler (self->item_manager, tile); + GtkListTile *footer = gtk_list_item_manager_get_last (self->item_manager); + g_assert (gtk_list_tile_is_footer (footer)); + tile = gtk_rb_tree_node_get_previous (footer); gtk_list_tile_set_area_position (self->item_manager, - filler, + footer, column_start (self, xspacing, i), y); gtk_list_tile_set_area_size (self->item_manager, - filler, - column_end (self, xspacing, self->n_columns - 1) - filler->area.x, + footer, + column_end (self, xspacing, self->n_columns - 1) - footer->area.x, tile->area.height); } From 1d4f383ba5945c69e3c5aa9b9f8b06f026a610c4 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 17 May 2023 19:13:12 -0400 Subject: [PATCH 6/7] Drop the FILLER tile type It is not used anymore. --- gtk/gtklistitemmanager.c | 51 +++------------------------------ gtk/gtklistitemmanagerprivate.h | 3 -- gtk/gtklistview.c | 1 - testsuite/gtk/listitemmanager.c | 9 ------ 4 files changed, 4 insertions(+), 60 deletions(-) diff --git a/gtk/gtklistitemmanager.c b/gtk/gtklistitemmanager.c index 8fc0476370..a8b12cef7f 100644 --- a/gtk/gtklistitemmanager.c +++ b/gtk/gtklistitemmanager.c @@ -198,7 +198,6 @@ gtk_list_item_manager_augment_node (GtkRbTree *tree, aug->has_footer = TRUE; break; case GTK_LIST_TILE_ITEM: - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: aug->has_header = FALSE; aug->has_footer = FALSE; @@ -633,7 +632,7 @@ static GtkListTile * gtk_list_tile_get_next_skip (GtkListTile *tile) { for (tile = gtk_rb_tree_node_get_next (tile); - tile && (tile->type == GTK_LIST_TILE_FILLER || tile->type == GTK_LIST_TILE_REMOVED); + tile && tile->type == GTK_LIST_TILE_REMOVED; tile = gtk_rb_tree_node_get_next (tile)) { } @@ -644,7 +643,7 @@ static GtkListTile * gtk_list_tile_get_previous_skip (GtkListTile *tile) { for (tile = gtk_rb_tree_node_get_previous (tile); - tile && (tile->type == GTK_LIST_TILE_FILLER || tile->type == GTK_LIST_TILE_REMOVED); + tile && tile->type == GTK_LIST_TILE_REMOVED; tile = gtk_rb_tree_node_get_previous (tile)) { } @@ -885,7 +884,6 @@ gtk_list_item_manager_remove_items (GtkListItemManager *self, gtk_list_tile_set_type (tile, GTK_LIST_TILE_REMOVED); break; - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: default: g_assert_not_reached (); @@ -928,7 +926,7 @@ gtk_list_item_manager_add_items (GtkListItemManager *self, { /* at end of list, pick the footer */ for (tile = gtk_rb_tree_get_last (self->items); - tile && (tile->type == GTK_LIST_TILE_REMOVED || tile->type == GTK_LIST_TILE_FILLER); + tile && tile->type == GTK_LIST_TILE_REMOVED; tile = gtk_rb_tree_node_get_previous (tile)) { } @@ -1029,34 +1027,6 @@ gtk_list_tile_split (GtkListItemManager *self, return result; } -/* - * gtk_list_tile_append_filler: - * @self: the listitemmanager - * @previous: tile to append to - * - * Appends a filler tile. - * - * Filler tiles don't refer to any items or header and exist - * just to take up space, so that finding items by position gets - * easier. - * - * They ave a special garbage-collection behavior, see - * gtk_list_tile_gc(). - * - * Returns: The new filler tile - **/ -GtkListTile * -gtk_list_tile_append_filler (GtkListItemManager *self, - GtkListTile *previous) -{ - GtkListTile *result; - - result = gtk_rb_tree_insert_after (self->items, previous); - result->type = GTK_LIST_TILE_FILLER; - - return result; -} - /* * gtk_list_tile_gc: * @self: the listitemmanager @@ -1083,13 +1053,6 @@ gtk_list_tile_gc (GtkListItemManager *self, if (tile == NULL) return NULL; - if (tile->type == GTK_LIST_TILE_FILLER) - { - next = gtk_rb_tree_node_get_next (tile); - gtk_rb_tree_remove (self->items, tile); - tile = next; - } - while (tile) { next = gtk_rb_tree_node_get_next (tile); @@ -1113,7 +1076,6 @@ gtk_list_tile_gc (GtkListItemManager *self, case GTK_LIST_TILE_FOOTER: case GTK_LIST_TILE_UNMATCHED_HEADER: case GTK_LIST_TILE_UNMATCHED_FOOTER: - case GTK_LIST_TILE_FILLER: break; case GTK_LIST_TILE_REMOVED: @@ -1180,7 +1142,6 @@ gtk_list_item_manager_release_items (GtkListItemManager *self, deleted_section = TRUE; break; - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: default: g_assert_not_reached (); @@ -1414,7 +1375,6 @@ gtk_list_item_manager_ensure_items (GtkListItemManager *self, break; case GTK_LIST_TILE_UNMATCHED_FOOTER: - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: default: g_assert_not_reached (); @@ -1660,9 +1620,7 @@ gtk_list_item_manager_clear_model (GtkListItemManager *self) for (tile = gtk_list_tile_gc (self, gtk_list_item_manager_get_first (self)); tile; tile = gtk_list_tile_gc (self, tile)) - { - g_assert (tile->type == GTK_LIST_TILE_FILLER); - } + { } g_assert (gtk_rb_tree_get_root (self->items) == NULL); } @@ -1775,7 +1733,6 @@ gtk_list_item_manager_set_has_sections (GtkListItemManager *self, footer = tile; break; case GTK_LIST_TILE_ITEM: - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: break; default: diff --git a/gtk/gtklistitemmanagerprivate.h b/gtk/gtklistitemmanagerprivate.h index fb8991fb69..ea855f4cc5 100644 --- a/gtk/gtklistitemmanagerprivate.h +++ b/gtk/gtklistitemmanagerprivate.h @@ -51,7 +51,6 @@ typedef enum GTK_LIST_TILE_FOOTER, GTK_LIST_TILE_UNMATCHED_HEADER, GTK_LIST_TILE_UNMATCHED_FOOTER, - GTK_LIST_TILE_FILLER, GTK_LIST_TILE_REMOVED, } GtkListTileType; @@ -127,8 +126,6 @@ void gtk_list_tile_set_area_size (GtkListItemMana GtkListTile * gtk_list_tile_split (GtkListItemManager *self, GtkListTile *tile, guint n_items); -GtkListTile * gtk_list_tile_append_filler (GtkListItemManager *self, - GtkListTile *previous); GtkListTile * gtk_list_tile_gc (GtkListItemManager *self, GtkListTile *tile); diff --git a/gtk/gtklistview.c b/gtk/gtklistview.c index c3ac2f681f..28c4706814 100644 --- a/gtk/gtklistview.c +++ b/gtk/gtklistview.c @@ -235,7 +235,6 @@ gtk_list_view_update_factories_with (GtkListView *self, case GTK_LIST_TILE_UNMATCHED_HEADER: case GTK_LIST_TILE_FOOTER: case GTK_LIST_TILE_UNMATCHED_FOOTER: - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: g_assert (tile->widget == NULL); break; diff --git a/testsuite/gtk/listitemmanager.c b/testsuite/gtk/listitemmanager.c index d572f41404..7f14a0f6d8 100644 --- a/testsuite/gtk/listitemmanager.c +++ b/testsuite/gtk/listitemmanager.c @@ -71,9 +71,6 @@ print_list_item_manager_tiles (GtkListItemManager *items) g_string_append_c (string, ')'); break; - case GTK_LIST_TILE_FILLER: - g_string_append_c (string, '_'); - break; case GTK_LIST_TILE_REMOVED: g_string_append_c (string, '.'); break; @@ -160,11 +157,6 @@ check_list_item_manager (GtkListItemManager *items, n_items += tile->n_items; break; - case GTK_LIST_TILE_FILLER: - /* We don't add fillers */ - g_assert_not_reached (); - break; - case GTK_LIST_TILE_REMOVED: g_assert_cmpint (tile->n_items, ==, 0); g_assert_null (tile->widget); @@ -245,7 +237,6 @@ check_list_item_manager (GtkListItemManager *items, n_items += tile->n_items; break; - case GTK_LIST_TILE_FILLER: case GTK_LIST_TILE_REMOVED: default: g_assert_not_reached (); From 48e49b4c507e886c92773053808a30b9f7aa4793 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sat, 20 May 2023 11:35:31 -0400 Subject: [PATCH 7/7] gridview: Update factories in set_factory Call update_factories() so the children get their factories properly updated. This matches what GtkListView does. --- gtk/gtkgridview.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gtk/gtkgridview.c b/gtk/gtkgridview.c index 63199d6bea..9408e16123 100644 --- a/gtk/gtkgridview.c +++ b/gtk/gtkgridview.c @@ -1337,6 +1337,8 @@ gtk_grid_view_set_factory (GtkGridView *self, if (!g_set_object (&self->factory, factory)) return; + gtk_grid_view_update_factories (self); + g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_FACTORY]); }