From b5ab90143a2df7ffcbae00a36b4db1367c07918f Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Mon, 2 May 2022 14:37:11 +0100 Subject: [PATCH] Improve the align_next_to mechanism. Various alignments I want to do in the host CA box have shown up deficiencies in this system, so I've reworked it a bit. Firstly, you can now specify more than two controls to be tied together with an align_next_to (e.g. multiple checkboxes alongside something else). Secondly, as well as forcing the controls to be the same height as each other, the layout algorithm will also move the later controls further _downward_, so that their top y positions also line up. Until now that hasn't been necessary, because they lined up already. In the GTK implementation of this via the Columns class, I've renamed 'columns_force_same_height' to 'columns_align_next_to', and similarly for some of the internal fields, since the latter change makes the previous names a misnomer. In the Windows implementation, I found it most convenient to set this up by following a linked list of align_next_to fields backwards. But it won't always be convenient to initialise them that way, so I've also written a crude normaliser that will rewrite those links into a canonical form. But I only call that on Windows; it's unnecessary in GTK, where the Columns class provides plenty of per-widget extra storage so I just keep each alignment class as a circular list. --- dialog.h | 24 ++++++++++++---- unix/columns.c | 58 ++++++++++++++++++++++++++------------ unix/columns.h | 10 +++++-- unix/dialog.c | 19 +++---------- utils/CMakeLists.txt | 1 + utils/ctrlset_normalise.c | 59 +++++++++++++++++++++++++++++++++++++++ windows/controls.c | 47 +++++++++++++++++++++---------- 7 files changed, 162 insertions(+), 56 deletions(-) create mode 100644 utils/ctrlset_normalise.c diff --git a/dialog.h b/dialog.h index b85bdcd5..45b4cde7 100644 --- a/dialog.h +++ b/dialog.h @@ -163,14 +163,20 @@ struct dlgcontrol { */ intorptr helpctx; /* - * Setting this to non-NULL coerces two controls to have their - * y-coordinates adjusted so that they can sit alongside each - * other and look nicely aligned, even if they're different + * Setting this to non-NULL coerces two or more controls to have + * their y-coordinates adjusted so that they can sit alongside + * each other and look nicely aligned, even if they're different * heights. * - * Set this field on the _second_ control of the pair (in terms of - * order in the data structure), so that when it's instantiated, - * the first one is already there to be referred to. + * Set this field on later controls (in terms of order in the data + * structure), pointing back to earlier ones, so that when each + * control is instantiated, the referred-to one is already there + * to be referred to. + * + * Don't expect this to change the position of the _first_ + * control. Currently, the layout is done one control at a time, + * so that once the first control has been placed, the second one + * can't cause the first one to be retrospectively moved. */ dlgcontrol *align_next_to; @@ -667,3 +673,9 @@ int ctrl_path_elements(const char *path); /* Return the number of matching path elements at the starts of p1 and p2, * or INT_MAX if the paths are identical. */ int ctrl_path_compare(const char *p1, const char *p2); + +/* + * Normalise the align_next_to fields in a controlset so that they + * form a backwards linked list. + */ +void ctrlset_normalise_aligns(struct controlset *s); diff --git a/unix/columns.c b/unix/columns.c index a1049b47..917b39e6 100644 --- a/unix/columns.c +++ b/unix/columns.c @@ -332,7 +332,6 @@ static void columns_remove(GtkContainer *container, GtkWidget *widget) ColumnsChild *child; GtkWidget *childw; GList *children; - bool was_visible; g_return_if_fail(container != NULL); g_return_if_fail(IS_COLUMNS(container)); @@ -346,23 +345,28 @@ static void columns_remove(GtkContainer *container, GtkWidget *widget) if (child->widget != widget) continue; - was_visible = gtk_widget_get_visible(widget); + bool need_layout = false; + if (gtk_widget_get_visible(widget)) + need_layout = true; gtk_widget_unparent(widget); cols->children = g_list_remove_link(cols->children, children); g_list_free(children); - if (child->same_height_as) { - g_return_if_fail(child->same_height_as->same_height_as == child); - child->same_height_as->same_height_as = NULL; - if (gtk_widget_get_visible(child->same_height_as->widget)) - gtk_widget_queue_resize(GTK_WIDGET(container)); - } + /* Unlink this widget from its valign list, and if anything + * else on the list is still visible, ensure we recompute our + * layout */ + for (ColumnsChild *ch = child->valign_next; ch != child; + ch = ch->valign_next) + if (gtk_widget_get_visible(ch->widget)) + need_layout = true; + child->valign_next->valign_prev = child->valign_prev; + child->valign_prev->valign_next = child->valign_next; if (cols->vexpand == child) cols->vexpand = NULL; g_free(child); - if (was_visible) + if (need_layout) gtk_widget_queue_resize(GTK_WIDGET(container)); break; } @@ -465,7 +469,8 @@ void columns_add(Columns *cols, GtkWidget *child, childdata->colstart = colstart; childdata->colspan = colspan; childdata->force_left = false; - childdata->same_height_as = NULL; + childdata->valign_next = childdata; + childdata->valign_prev = childdata; childdata->percentages = NULL; cols->children = g_list_append(cols->children, childdata); @@ -516,7 +521,7 @@ void columns_force_left_align(Columns *cols, GtkWidget *widget) gtk_widget_queue_resize(GTK_WIDGET(cols)); } -void columns_force_same_height(Columns *cols, GtkWidget *cw1, GtkWidget *cw2) +void columns_align_next_to(Columns *cols, GtkWidget *cw1, GtkWidget *cw2) { ColumnsChild *child1, *child2; @@ -530,8 +535,13 @@ void columns_force_same_height(Columns *cols, GtkWidget *cw1, GtkWidget *cw2) child2 = columns_find_child(cols, cw2); g_return_if_fail(child2 != NULL); - child1->same_height_as = child2; - child2->same_height_as = child1; + ColumnsChild *child1prev = child1->valign_prev; + ColumnsChild *child2prev = child2->valign_prev; + child1prev->valign_next = child2; + child2->valign_prev = child1prev; + child2prev->valign_next = child1; + child1->valign_prev = child2prev; + if (gtk_widget_get_visible(cw1) || gtk_widget_get_visible(cw2)) gtk_widget_queue_resize(GTK_WIDGET(cols)); } @@ -843,8 +853,9 @@ static gint columns_compute_height(Columns *cols, widget_dim_fn_t get_height) continue; childheight = get_height(child); - if (child->same_height_as) { - gint childheight2 = get_height(child->same_height_as); + for (ColumnsChild *ch = child->valign_next; ch != child; + ch = ch->valign_next) { + gint childheight2 = get_height(ch); if (childheight < childheight2) childheight = childheight2; } @@ -902,6 +913,11 @@ static void columns_alloc_vert(Columns *cols, gint ourheight, colypos = g_new(gint, 1); colypos[0] = 0; + for (children = cols->children; + children && (child = children->data); + children = children->next) + child->visited = false; + for (children = cols->children; children && (child = children->data); children = children->next) { @@ -922,14 +938,19 @@ static void columns_alloc_vert(Columns *cols, gint ourheight, if (!gtk_widget_get_visible(child->widget)) continue; + int ymin = 0; + realheight = get_height(child); if (child == cols->vexpand) realheight += vexpand_extra; fakeheight = realheight; - if (child->same_height_as) { - gint childheight2 = get_height(child->same_height_as); + for (ColumnsChild *ch = child->valign_next; ch != child; + ch = ch->valign_next) { + gint childheight2 = get_height(ch); if (fakeheight < childheight2) fakeheight = childheight2; + if (ch->visited && ymin < ch->y) + ymin = ch->y; } colspan = child->colspan ? child->colspan : ncols-child->colstart; @@ -943,13 +964,14 @@ static void columns_alloc_vert(Columns *cols, gint ourheight, { int topy, boty; - topy = 0; + topy = ymin; for (i = 0; i < colspan; i++) { if (topy < colypos[child->colstart+i]) topy = colypos[child->colstart+i]; } child->y = topy + fakeheight/2 - realheight/2; child->h = realheight; + child->visited = true; boty = topy + fakeheight + cols->spacing; for (i = 0; i < colspan; i++) { colypos[child->colstart+i] = boty; diff --git a/unix/columns.h b/unix/columns.h index d2d58c4a..bca306b4 100644 --- a/unix/columns.h +++ b/unix/columns.h @@ -43,11 +43,17 @@ struct ColumnsChild_tag { GtkWidget *widget; gint colstart, colspan; bool force_left; /* for recalcitrant GtkLabels */ - ColumnsChild *same_height_as; /* Otherwise, this entry represents a change in the column setup. */ gint ncols; gint *percentages; gint x, y, w, h; /* used during an individual size computation */ + + /* Circularly linked list of children that are vertically aligned + * with each other. */ + ColumnsChild *valign_next, *valign_prev; + + /* Temporary space used within some methods */ + bool visited; }; GType columns_get_type(void); @@ -57,7 +63,7 @@ void columns_add(Columns *cols, GtkWidget *child, gint colstart, gint colspan); void columns_taborder_last(Columns *cols, GtkWidget *child); void columns_force_left_align(Columns *cols, GtkWidget *child); -void columns_force_same_height(Columns *cols, GtkWidget *ch1, GtkWidget *ch2); +void columns_align_next_to(Columns *cols, GtkWidget *ch1, GtkWidget *ch2); void columns_vexpand(Columns *cols, GtkWidget *child); #ifdef __cplusplus diff --git a/unix/dialog.c b/unix/dialog.c index bd5d535f..5b205201 100644 --- a/unix/dialog.c +++ b/unix/dialog.c @@ -2109,8 +2109,7 @@ GtkWidget *layout_ctrls( columns_add(COLUMNS(container), label, 0, 1); columns_force_left_align(COLUMNS(container), label); columns_add(COLUMNS(container), w, 1, 1); - columns_force_same_height(COLUMNS(container), - label, w); + columns_align_next_to(COLUMNS(container), label, w); } gtk_widget_show(label); gtk_widget_show(w); @@ -2163,7 +2162,7 @@ GtkWidget *layout_ctrls( columns_add(COLUMNS(w), ww, 1, 1); gtk_widget_show(ww); - columns_force_same_height(COLUMNS(w), uc->entry, uc->button); + columns_align_next_to(COLUMNS(w), uc->entry, uc->button); g_signal_connect(G_OBJECT(uc->entry), "key_press_event", G_CALLBACK(editbox_key), dp); @@ -2456,8 +2455,7 @@ GtkWidget *layout_ctrls( columns_add(COLUMNS(container), label, 0, 1); columns_force_left_align(COLUMNS(container), label); columns_add(COLUMNS(container), w, 1, 1); - columns_force_same_height(COLUMNS(container), - label, w); + columns_align_next_to(COLUMNS(container), label, w); } gtk_widget_show(label); gtk_widget_show(w); @@ -2520,19 +2518,10 @@ GtkWidget *layout_ctrls( if (left) columns_force_left_align(cols, w); if (ctrl->align_next_to) { - /* - * Implement align_next_to by simply forcing the two - * controls to have the same height of size allocation. At - * least for the controls we're currently doing this with, - * the GTK layout system will automatically vertically - * centre each control within its allocation, which will - * get the two controls aligned alongside each other - * reasonably well. - */ struct uctrl *uc2 = dlg_find_byctrl( dp, ctrl->align_next_to); assert(uc2); - columns_force_same_height(cols, w, uc2->toplevel); + columns_align_next_to(cols, w, uc2->toplevel); #if GTK_CHECK_VERSION(3, 10, 0) /* Slightly nicer to align baselines than just vertically diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index dfd28efd..2c99217a 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -14,6 +14,7 @@ add_sources_from_current_dir(utils conf_dest.c conf_launchable.c ctrlparse.c + ctrlset_normalise.c debug.c decode_utf8.c decode_utf8_to_wchar.c diff --git a/utils/ctrlset_normalise.c b/utils/ctrlset_normalise.c new file mode 100644 index 00000000..46f5f19f --- /dev/null +++ b/utils/ctrlset_normalise.c @@ -0,0 +1,59 @@ +/* + * Helper function from the dialog.h mechanism. + */ + +#include "misc.h" +#include "dialog.h" + +void ctrlset_normalise_aligns(struct controlset *s) +{ + /* + * The algorithm in here is quadratic time. Never on very much data, but + * even so, let's avoid bothering to use it where possible. In most + * controlsets, there's no use of align_next_to in any case, so we have + * nothing to do. + */ + for (size_t j = 0; j < s->ncontrols; j++) + if (s->ctrls[j]->align_next_to) + goto must_do_something; + /* If we fell out of this loop, there's nothing to do here */ + return; + must_do_something:; + + size_t *idx = snewn(s->ncontrols, size_t); + + /* + * Follow align_next_to links to identify, for each control, the least + * index within this controlset of things it's linked to. That way, + * controls with the same idx[j] will be in the same alignment class. + */ + for (size_t j = 0; j < s->ncontrols; j++) { + dlgcontrol *c = s->ctrls[j]; + idx[j] = j; + if (c->align_next_to) { + for (size_t k = 0; k < j; k++) { + if (s->ctrls[k] == c->align_next_to) { + idx[j] = idx[k]; + break; + } + } + } + } + + /* + * Having done that, re-link each control to the most recent one in its + * class, so that the links form a backward linked list. + */ + for (size_t j = 0; j < s->ncontrols; j++) { + dlgcontrol *c = s->ctrls[j]; + c->align_next_to = NULL; + for (size_t k = j; k-- > 0 ;) { + if (idx[k] == idx[j]) { + c->align_next_to = s->ctrls[k]; + break; + } + } + } + + sfree(idx); +} diff --git a/windows/controls.c b/windows/controls.c index e82d215d..d0de4674 100644 --- a/windows/controls.c +++ b/windows/controls.c @@ -1367,6 +1367,8 @@ void winctrl_layout(struct dlgparam *dp, struct winctrls *wc, base_id = *id; + ctrlset_normalise_aligns(s); + /* Start a containing box, if we have a boxname. */ if (s->boxname && *s->boxname) { struct winctrl *c = snew(struct winctrl); @@ -1718,21 +1720,36 @@ void winctrl_layout(struct dlgparam *dp, struct winctrls *wc, * moving one or the other downwards so that they're * centred on a common horizontal line. */ - struct winctrl *c2 = winctrl_findbyctrl( - wc, ctrl->align_next_to); - HWND win1 = GetDlgItem(pos.hwnd, c->align_id); - HWND win2 = GetDlgItem(pos.hwnd, c2->align_id); - RECT rect1, rect2; - if (win1 && win2 && - GetWindowRect(win1, &rect1) && - GetWindowRect(win2, &rect2)) { - LONG top = (rect1.top < rect2.top ? rect1.top : rect2.top); - LONG bottom = (rect1.bottom > rect2.bottom ? - rect1.bottom : rect2.bottom); - move_windows(pos.hwnd, c->base_id, c->num_ids, - (top + bottom - rect1.top - rect1.bottom)/2); - move_windows(pos.hwnd, c2->base_id, c2->num_ids, - (top + bottom - rect2.top - rect2.bottom)/2); + LONG mid2 = 0; + for (dlgcontrol *thisctrl = ctrl; thisctrl; + thisctrl = thisctrl->align_next_to) { + struct winctrl *thisc = winctrl_findbyctrl(wc, thisctrl); + assert(thisc); + + HWND win = GetDlgItem(pos.hwnd, thisc->align_id); + assert(win); + + RECT rect; + if (!GetWindowRect(win, &rect)) + continue; + if (mid2 < rect.top + rect.bottom) + mid2 = rect.top + rect.bottom; + } + + for (dlgcontrol *thisctrl = ctrl; thisctrl; + thisctrl = thisctrl->align_next_to) { + struct winctrl *thisc = winctrl_findbyctrl(wc, thisctrl); + assert(thisc); + + HWND win = GetDlgItem(pos.hwnd, thisc->align_id); + assert(win); + + RECT rect; + if (!GetWindowRect(win, &rect)) + continue; + + LONG dy = (mid2 - (rect.top + rect.bottom)) / 2; + move_windows(pos.hwnd, c->base_id, c->num_ids, dy); } } } else {