From e994acb6cf8411eeba89f626761b968a97fbd01b Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 15 Dec 2024 21:42:10 +0300 Subject: [PATCH 1/6] clamp: Stop overriding get_request_mode Let the default implementation in GtkLayoutManager take over. Specifically, it will propagate child's preferred request mode. Generally, it's best to avoid measuring a widget against its preferred size request mode, for two reasons: 1. It may be extremely slow, since it will commonly have to do (multiple layers of) binary searching. 2. Many widgets have a not-quite-strictly-correct measure implementation for the opposite size request mode. Indeed, oftentimes the only way to implement correct measurement is to do a binary search. As a relevant example of the latter point, AdwWrapLayout itself has an incorrect implementation of measuring in the opposite orientation, which will be fixed over the next few commits. This is why it is generally a very good idea to propagate the child's request mode, unless there are good reasons why a container widget strongly prefers to be measured in a specific orientation. GtkListBox is a good example of a case where it's reasonable to prefer height-for- width (which is likely to match the children's preferred mode anyway), while the regular GtkBox (intended to be used with a small number of child widgets) should propagate its children's preferred request mode, even though it's generally easier for a box itself to be measured along its own orientation, not across it. AdwWrapLayout can be quickly and nicely measured in either orientation, so don't force a request mode and propagate what the child asks for. As a bonus point, we will report constant-size when the child is, which enables further optimizations elsewhere in GTK. Signed-off-by: Sergey Bugaev --- src/adw-clamp-layout.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/adw-clamp-layout.c b/src/adw-clamp-layout.c index 2b36c69fd..4938099f1 100644 --- a/src/adw-clamp-layout.c +++ b/src/adw-clamp-layout.c @@ -221,17 +221,6 @@ child_size_from_clamp (AdwClampLayout *self, return ceil (adw_lerp (lower, max, adw_easing_ease (ADW_EASE_OUT_CUBIC, progress))); } -static GtkSizeRequestMode -adw_clamp_layout_get_request_mode (GtkLayoutManager *manager, - GtkWidget *widget) -{ - AdwClampLayout *self = ADW_CLAMP_LAYOUT (manager); - - return self->orientation == GTK_ORIENTATION_HORIZONTAL ? - GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH : - GTK_SIZE_REQUEST_WIDTH_FOR_HEIGHT; -} - static void adw_clamp_layout_measure (GtkLayoutManager *manager, GtkWidget *widget, @@ -361,7 +350,6 @@ adw_clamp_layout_class_init (AdwClampLayoutClass *klass) object_class->get_property = adw_clamp_layout_get_property; object_class->set_property = adw_clamp_layout_set_property; - layout_manager_class->get_request_mode = adw_clamp_layout_get_request_mode; layout_manager_class->measure = adw_clamp_layout_measure; layout_manager_class->allocate = adw_clamp_layout_allocate; -- GitLab From 3d4fe3e2a2e3b03f2e3c171c95b342faee10714c Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 15 Dec 2024 22:00:29 +0300 Subject: [PATCH 2/6] clamp: Fix multi-child support Signed-off-by: Sergey Bugaev --- src/adw-clamp-layout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adw-clamp-layout.c b/src/adw-clamp-layout.c index 4938099f1..e9e1494cd 100644 --- a/src/adw-clamp-layout.c +++ b/src/adw-clamp-layout.c @@ -293,7 +293,7 @@ adw_clamp_layout_allocate (GtkLayoutManager *manager, gtk_widget_remove_css_class (child, "medium"); gtk_widget_remove_css_class (child, "large"); - return; + continue; } if (self->orientation == GTK_ORIENTATION_HORIZONTAL) { -- GitLab From ef3a04ee196482323b30c5e242499d14e4918495 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 15 Dec 2024 22:03:56 +0300 Subject: [PATCH 3/6] clamp: Round child size down, not up If both child_size_from_clamp () and clamp_size_from_child () round up, the measurements are inconsistent between the two directions. It can be shown mathematically that the proper arrangement is to round down when converting from a container size to its child's size, and round up when converting from the child's size to the container size. So make child_size_from_clamp () use floor () in stead of ceil (). Signed-off-by: Sergey Bugaev --- src/adw-clamp-layout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adw-clamp-layout.c b/src/adw-clamp-layout.c index e9e1494cd..c60d463ff 100644 --- a/src/adw-clamp-layout.c +++ b/src/adw-clamp-layout.c @@ -218,7 +218,7 @@ child_size_from_clamp (AdwClampLayout *self, progress = inverse_lerp (lower, upper, for_size); - return ceil (adw_lerp (lower, max, adw_easing_ease (ADW_EASE_OUT_CUBIC, progress))); + return floor (adw_lerp (lower, max, adw_easing_ease (ADW_EASE_OUT_CUBIC, progress))); } static void -- GitLab From 5d4c1a2e6c967be8a5d1031a81d8072873766cae Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 15 Dec 2024 23:17:32 +0300 Subject: [PATCH 4/6] clamp: Separate child vs clamp measurements This is in a separate patch to make things more reviewable. No functionality changes. Signed-off-by: Sergey Bugaev --- src/adw-clamp-layout.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/adw-clamp-layout.c b/src/adw-clamp-layout.c index c60d463ff..f93872cc7 100644 --- a/src/adw-clamp-layout.c +++ b/src/adw-clamp-layout.c @@ -238,10 +238,12 @@ adw_clamp_layout_measure (GtkLayoutManager *manager, for (child = gtk_widget_get_first_child (widget); child != NULL; child = gtk_widget_get_next_sibling (child)) { + int clamp_min = 0; + int clamp_nat = 0; int child_min = 0; int child_nat = 0; - int child_min_baseline = -1; - int child_nat_baseline = -1; + int min_baseline = -1; + int nat_baseline = -1; if (!gtk_widget_should_layout (child)) continue; @@ -249,24 +251,27 @@ adw_clamp_layout_measure (GtkLayoutManager *manager, if (self->orientation == orientation) { gtk_widget_measure (child, orientation, for_size, &child_min, &child_nat, - &child_min_baseline, &child_nat_baseline); + &min_baseline, &nat_baseline); - child_nat = clamp_size_from_child (self, settings, child_min, child_nat); + clamp_min = child_min; + clamp_nat = clamp_size_from_child (self, settings, child_min, child_nat); } else { int child_size = child_size_from_clamp (self, settings, child, for_size, NULL, NULL); gtk_widget_measure (child, orientation, child_size, &child_min, &child_nat, - &child_min_baseline, &child_nat_baseline); + &min_baseline, &nat_baseline); + clamp_min = child_min; + clamp_nat = child_nat; } - *minimum = MAX (*minimum, child_min); - *natural = MAX (*natural, child_nat); + *minimum = MAX (*minimum, clamp_min); + *natural = MAX (*natural, clamp_nat); - if (child_min_baseline > -1) - *minimum_baseline = MAX (*minimum_baseline, child_min_baseline); - if (child_nat_baseline > -1) - *natural_baseline = MAX (*natural_baseline, child_nat_baseline); + if (min_baseline > -1) + *minimum_baseline = MAX (*minimum_baseline, min_baseline); + if (nat_baseline > -1) + *natural_baseline = MAX (*natural_baseline, nat_baseline); } } -- GitLab From 3585a623ca009ae40d51584a5ed5360924ae2aef Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 15 Dec 2024 23:21:59 +0300 Subject: [PATCH 5/6] clamp: Fix measuring along the clamped orientation The existing code attempted to base the clamping calculations of the minimum and natural sizes of the child for the given size in the opposite orientation. While it's generally a good idea to pass the appropriate for_size value instead of -1 to gtk_widget_measure () calls whenever possible, in this case this only leads to inconsistencies with measurements along clamp's own orientation (i.e. height-for-width for a horizontal clamp) and with allocate () behavior. So stop passing minimum and natural sizes to clamp_size_from_child (), and only pass a single size to be mapped. In clamp_size_from_child (), query the child for its overall minimum and natural sizes, and base our calculations on those. This is consistent with child_size_from_clamp (). Moreover, the existing code only mapped the child's natural size, passing through the minimum size as is. This is the correct thing to do for overall minimum size, but not when measuring for a specific size in the other orientation: when the minimum size for the specific size in the opposite orientation is larger than the overall minimum size and larger than our tightening threshold, we would give the child less space if the clamp got allocated this size, and that would contradict it being the minimum acceptable size for the child for the given size in the other orientation. So we must map the minimum size reported by the child as well. Also, fix baseline handling for vertical clamps. Signed-off-by: Sergey Bugaev --- src/adw-clamp-layout.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/adw-clamp-layout.c b/src/adw-clamp-layout.c index f93872cc7..9f80af822 100644 --- a/src/adw-clamp-layout.c +++ b/src/adw-clamp-layout.c @@ -149,10 +149,11 @@ inverse_lerp (double a, static int clamp_size_from_child (AdwClampLayout *self, GtkSettings *settings, - int min, - int nat) + GtkWidget *child, + int child_size) { double max, lower, upper, progress, maximum_size, tightening_threshold; + int min, nat; maximum_size = adw_length_unit_to_px (self->unit, self->maximum_size, @@ -161,16 +162,20 @@ clamp_size_from_child (AdwClampLayout *self, self->tightening_threshold, settings); + gtk_widget_measure (child, self->orientation, -1, &min, &nat, NULL, NULL); + lower = MAX (MIN (tightening_threshold, maximum_size), min); max = MAX (lower, maximum_size); upper = lower + ADW_EASE_OUT_TAN_CUBIC * (max - lower); - if (nat <= lower) - progress = 0; - else if (nat >= max) - progress = 1; + g_assert (child_size != -1); + + if (child_size <= lower) + return child_size; + else if (child_size >= max) + return upper; else { - double ease = inverse_lerp (lower, max, nat); + double ease = inverse_lerp (lower, max, child_size); progress = 1 + cbrt (ease - 1); // inverse ease out cubic } @@ -253,8 +258,12 @@ adw_clamp_layout_measure (GtkLayoutManager *manager, &child_min, &child_nat, &min_baseline, &nat_baseline); - clamp_min = child_min; - clamp_nat = clamp_size_from_child (self, settings, child_min, child_nat); + clamp_min = clamp_size_from_child (self, settings, child, child_min); + clamp_nat = clamp_size_from_child (self, settings, child, child_nat); + if (min_baseline > -1) + min_baseline += (clamp_min - child_min) / 2; + if (nat_baseline > -1) + nat_baseline += (clamp_nat - child_nat) / 2; } else { int child_size = child_size_from_clamp (self, settings, child, for_size, NULL, NULL); -- GitLab From 08845694ff412a1695b403fef8cac108ce89d973 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Sun, 15 Dec 2024 23:38:12 +0300 Subject: [PATCH 6/6] clamp: Fix measuring opposite minimum size When passed for_size = -1, the existing code measured the child for MIN (nat, ceil (max)), where nat is the natural size of the child in the opposite orientation, and ceil (max) is the maximum size that the clamp will possibly allocate to the child. This generally makes sense. However, it's not always true that the overall minimum size of a widget equals it minimum size for its natural size in the opposite orientation. A specific example of this is GtkLabel with max-width-chars set. The overall minimum height for such a label will be very small (perhaps the height of a single line of text), while the minimum height for its natural width will be considerably larger. Of course, the same applies to complex widget trees that contain such labels. What we really need to report here is the smallest possible size that the child will ever have. We cannot just propagate the child's overall minimum size, since that might be only achievable at a size in the opposite orientation that's above the maximum size we're going to give it. So instead use the minimum size for that maximum size in the other orientation; this is going to be at least as small as minimum size for any other size, since measurements are assumed to be monotonic. Signed-off-by: Sergey Bugaev --- src/adw-clamp-layout.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/adw-clamp-layout.c b/src/adw-clamp-layout.c index 9f80af822..816419ee3 100644 --- a/src/adw-clamp-layout.c +++ b/src/adw-clamp-layout.c @@ -254,6 +254,9 @@ adw_clamp_layout_measure (GtkLayoutManager *manager, continue; if (self->orientation == orientation) { + /* We pass all of for_size to the child, but we have to post-process + * its measurements. + */ gtk_widget_measure (child, orientation, for_size, &child_min, &child_nat, &min_baseline, &nat_baseline); @@ -265,11 +268,31 @@ adw_clamp_layout_measure (GtkLayoutManager *manager, if (nat_baseline > -1) nat_baseline += (clamp_nat - child_nat) / 2; } else { - int child_size = child_size_from_clamp (self, settings, child, for_size, NULL, NULL); + /* We clamp for_size, but after that, child's minimum and natural + * sizes in this orientation are what we report. + */ + int child_maximum = 0; + int child_size = child_size_from_clamp (self, settings, child, + for_size, &child_maximum, + NULL); gtk_widget_measure (child, orientation, child_size, &child_min, &child_nat, &min_baseline, &nat_baseline); + + if (for_size == -1 && child_size < child_maximum) { + /* For any specific for_size, we have a definite child_size that + * we measure the child for. When passed for_size = -1 however, + * we're supposed to report overall minimum size, for any + * potential value of for_size. For that, we have to measure the + * child at the maximum size we could give it. Note that we + * should not use this measurement for the natural size. + */ + gtk_widget_measure (child, orientation, child_maximum, + &child_min, NULL, + &min_baseline, NULL); + } + clamp_min = child_min; clamp_nat = child_nat; } -- GitLab