From fc4044bbceccd577e410e5a6c3fa1fe8315583e5 Mon Sep 17 00:00:00 2001 From: Ivan Molodetskikh Date: Tue, 7 Mar 2023 09:51:32 -0800 Subject: [PATCH 1/6] combobox: Avoid extra queue_resize() width-request already ensures it's above the minimum width, so avoid an extra queue_resize() when setting size request to (-1, -1). This is the same way as GtkDropDown works. This also unbreaks GtkComboBox after the recent allocation fix in 75a417e33708dab2bdb2f784a8952e085a12bf03. Incidentally, this also makes GtkComboBox actually resize its popup as intended (that was broken before). I don't think this is ultimately the final fix, sometimes I still get allocation warnings. But the proper fix will probably involve changing some more allocation machinery around popovers. This is good enough for now. --- gtk/gtkcombobox.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/gtk/gtkcombobox.c b/gtk/gtkcombobox.c index 09a36bfa1e..652dc60aeb 100644 --- a/gtk/gtkcombobox.c +++ b/gtk/gtkcombobox.c @@ -362,7 +362,6 @@ gtk_combo_box_size_allocate (GtkWidget *widget, { GtkComboBox *combo_box = GTK_COMBO_BOX (widget); GtkComboBoxPrivate *priv = gtk_combo_box_get_instance_private (combo_box); - int menu_width; gtk_widget_size_allocate (priv->box, &(GtkAllocation) { @@ -370,17 +369,8 @@ gtk_combo_box_size_allocate (GtkWidget *widget, width, height }, baseline); - gtk_widget_set_size_request (priv->popup_widget, -1, -1); - - if (priv->popup_fixed_width) - gtk_widget_measure (priv->popup_widget, GTK_ORIENTATION_HORIZONTAL, -1, - &menu_width, NULL, NULL, NULL); - else - gtk_widget_measure (priv->popup_widget, GTK_ORIENTATION_HORIZONTAL, -1, - NULL, &menu_width, NULL, NULL); - - gtk_widget_set_size_request (priv->popup_widget, - MAX (width, menu_width), -1); + gtk_widget_set_size_request (priv->popup_widget, width, -1); + gtk_widget_queue_resize (priv->popup_widget); gtk_popover_present (GTK_POPOVER (priv->popup_widget)); } From d671fdab7c832ccb0ca48cc5fd0c6823a69b2349 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Mar 2023 13:06:53 -0500 Subject: [PATCH 2/6] widget: Simplify size allocation Don't call ensure_allocate if we've just done size_allocate. This makes criticals from reshowing popovers go away. --- gtk/gtkwidget.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 00e1f63af3..d8b2b7667f 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -4020,7 +4020,10 @@ gtk_widget_allocate (GtkWidget *widget, size_changed = (priv->width != adjusted.width) || (priv->height != adjusted.height); if (!alloc_needed && !size_changed && !baseline_changed) - goto skip_allocate; + { + gtk_widget_ensure_allocate (widget); + goto skip_allocate; + } priv->width = adjusted.width; priv->height = adjusted.height; @@ -4068,9 +4071,6 @@ skip_allocate: gtk_widget_queue_draw (priv->parent); out: - if (priv->alloc_needed_on_child) - gtk_widget_ensure_allocate (widget); - gtk_widget_pop_verify_invariants (widget); } From 76c7602b92b4a777f72addefae0e45917e116667 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 8 Mar 2023 18:23:50 +0000 Subject: [PATCH 3/6] widget: Split out gtk_widget_ensure_allocate_on_children() allocate() should not be calling into ensure_allocate(), they do a similar job. In the end, the code does the same work, but it should be easier to follow now. --- gtk/gtkwidget.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index d8b2b7667f..2e810f10d9 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -3854,6 +3854,28 @@ gtk_widget_adjust_size_allocation (GtkWidget *widget, } } +static void +gtk_widget_ensure_allocate_on_children (GtkWidget *widget) +{ + GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget); + GtkWidget *child; + + g_assert (!priv->resize_needed); + g_assert (!priv->alloc_needed); + + if (!priv->alloc_needed_on_child) + return; + + priv->alloc_needed_on_child = FALSE; + + for (child = _gtk_widget_get_first_child (widget); + child != NULL; + child = _gtk_widget_get_next_sibling (child)) + { + gtk_widget_ensure_allocate (child); + } +} + /** * gtk_widget_allocate: * @widget: A `GtkWidget` @@ -4021,7 +4043,7 @@ gtk_widget_allocate (GtkWidget *widget, if (!alloc_needed && !size_changed && !baseline_changed) { - gtk_widget_ensure_allocate (widget); + gtk_widget_ensure_allocate_on_children (widget); goto skip_allocate; } @@ -10547,18 +10569,9 @@ gtk_widget_ensure_allocate (GtkWidget *widget) priv->allocated_size_baseline, gsk_transform_ref (priv->allocated_transform)); } - else if (priv->alloc_needed_on_child) + else { - GtkWidget *child; - - priv->alloc_needed_on_child = FALSE; - - for (child = _gtk_widget_get_first_child (widget); - child != NULL; - child = _gtk_widget_get_next_sibling (child)) - { - gtk_widget_ensure_allocate (child); - } + gtk_widget_ensure_allocate_on_children (widget); } } From 7b0c6b6c9cf16925ee2baa6c9676bf0b95dad26c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 8 Mar 2023 18:27:46 +0000 Subject: [PATCH 4/6] widget: Remove goto usage in widget_allocate() --- gtk/gtkwidget.c | 75 +++++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 2e810f10d9..1819036323 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -4044,52 +4044,53 @@ gtk_widget_allocate (GtkWidget *widget, if (!alloc_needed && !size_changed && !baseline_changed) { gtk_widget_ensure_allocate_on_children (widget); - goto skip_allocate; } - - priv->width = adjusted.width; - priv->height = adjusted.height; - priv->baseline = baseline; - - priv->alloc_needed_on_child = FALSE; - - if (priv->layout_manager != NULL) + else { - gtk_layout_manager_allocate (priv->layout_manager, widget, - priv->width, - priv->height, - baseline); - } - else - { - GTK_WIDGET_GET_CLASS (widget)->size_allocate (widget, - priv->width, - priv->height, - baseline); - } + priv->width = adjusted.width; + priv->height = adjusted.height; + priv->baseline = baseline; - /* Size allocation is god... after consulting god, no further requests or allocations are needed */ + priv->alloc_needed_on_child = FALSE; + + if (priv->layout_manager != NULL) + { + gtk_layout_manager_allocate (priv->layout_manager, widget, + priv->width, + priv->height, + baseline); + } + else + { + GTK_WIDGET_GET_CLASS (widget)->size_allocate (widget, + priv->width, + priv->height, + baseline); + } + + /* Size allocation is god... after consulting god, no further requests or allocations are needed */ #ifdef G_ENABLE_DEBUG - if (GTK_DISPLAY_DEBUG_CHECK (_gtk_widget_get_display (widget), GEOMETRY) && - gtk_widget_get_resize_needed (widget)) - { - g_warning ("%s %p or a child called gtk_widget_queue_resize() during size_allocate().", - gtk_widget_get_name (widget), widget); - } + if (GTK_DISPLAY_DEBUG_CHECK (_gtk_widget_get_display (widget), GEOMETRY) && + gtk_widget_get_resize_needed (widget)) + { + g_warning ("%s %p or a child called gtk_widget_queue_resize() during size_allocate().", + gtk_widget_get_name (widget), widget); + } #endif - gtk_widget_ensure_resize (widget); - priv->alloc_needed = FALSE; + gtk_widget_ensure_resize (widget); + priv->alloc_needed = FALSE; - gtk_widget_update_paintables (widget); + gtk_widget_update_paintables (widget); - if (size_changed) - gtk_accessible_bounds_changed (GTK_ACCESSIBLE (widget)); + if (size_changed) + gtk_accessible_bounds_changed (GTK_ACCESSIBLE (widget)); -skip_allocate: - if (size_changed || baseline_changed) - gtk_widget_queue_draw (widget); - else if (transform_changed && priv->parent) + if (size_changed || baseline_changed) + gtk_widget_queue_draw (widget); + } + + if (transform_changed && priv->parent) gtk_widget_queue_draw (priv->parent); out: From 4e0633a6ef348604c9cfe81c1ce5e3167e539c68 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Mar 2023 13:13:15 -0500 Subject: [PATCH 5/6] widget: Stop propagating alloc_needed beyond popovers This should not be necessary, since popovers get their new size from present_popup via the compositor. --- gtk/gtkwidget.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 1819036323..678adccf25 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -10522,7 +10522,10 @@ gtk_widget_set_alloc_needed (GtkWidget *widget) break; if (GTK_IS_NATIVE (widget)) - gtk_native_queue_relayout (GTK_NATIVE (widget)); + { + gtk_native_queue_relayout (GTK_NATIVE (widget)); + return; + } widget = priv->parent; if (widget == NULL) From 6af8a3189373473da71687b55712bfdf14278906 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Mar 2023 13:36:42 -0500 Subject: [PATCH 6/6] widget: Skip popovers in allocation Native widgets get allocated via their surface, so can skip them here. This avoids criticals when re-mapping a popover for the second time, as can be seen e.g. in the 'Selections' demo in gtk4-demo. --- gtk/gtkwidget.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 678adccf25..1672a24705 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -3872,7 +3872,8 @@ gtk_widget_ensure_allocate_on_children (GtkWidget *widget) child != NULL; child = _gtk_widget_get_next_sibling (child)) { - gtk_widget_ensure_allocate (child); + if (gtk_widget_should_layout (child)) + gtk_widget_ensure_allocate (child); } }