From 77a53a42c9ceafb3b1b838bfb2dbcb3ce4c02b35 Mon Sep 17 00:00:00 2001 From: pax Date: Wed, 8 Apr 2026 21:29:55 -0500 Subject: [PATCH] grid: standardize cell width in FlowLayout (fix column collapse) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous FlowLayout._do_layout walked each thumb summing `widget.width() + THUMB_SPACING` and wrapped on `x + item_w > self.width()`. This was vulnerable to two issues that conspired to produce the "grid collapses by a column when switching to a post" bug: 1. **Per-widget width drift**: ThumbnailWidget calls `setFixedSize(THUMB_SIZE, THUMB_SIZE)` in __init__, capturing the constant at construction time. If `THUMB_SIZE` is later mutated via `_apply_settings` (main_window.py:2953 writes `grid_mod.THUMB_SIZE = new_size`), existing thumbs keep their old fixed size while new ones (e.g. from infinite-scroll backfill via `append_posts`) get the new value. Mixed widths break the width-summing wrap loop. 2. **Off-by-one in the columns property**: `w // (THUMB_SIZE + THUMB_SPACING)` overcounted by 1 at column boundaries because it omitted the leading THUMB_SPACING margin. A row that fits N thumbs needs `THUMB_SPACING + N * step` pixels, not `N * step`. For width=1135 with step=188, the formula returned 6 columns while `_do_layout` only fit 5 — the two diverged whenever the grid sat in the boundary range. Both are fixed by using a deterministic position formula: cols = max(1, (width - THUMB_SPACING) // step) for each thumb i: col = i % cols row = i // cols x = THUMB_SPACING + col * step y = THUMB_SPACING + row * step The layout is now a function of `self.width()` and the constants only — no per-widget reads, no width-summing accumulator. The columns property uses the EXACT same formula so callers (e.g. main_window's keyboard Up/Down nav step) always get the value the visual layout actually used. Standardizing on the constant means existing thumbs that were created with an old `THUMB_SIZE` value still position correctly (they sit in the cells positioned by the new step), and any future mutation of THUMB_SIZE only affects newly-created thumbs without breaking the layout of the surviving ones. Affects all three tabs (Browse / Bookmarks / Library) since they all use ThumbnailGrid from grid.py. Verification: - Phase A test suite (16 tests) still passes - Popout state machine tests (65 tests) still pass - Total: 81 / 81 automated tests green - Imports clean - Manual: open the popout to a column boundary (resize window width such that the grid is exactly N columns wide), switch between posts — column count should NOT flip to N-1 anymore. Also verify keyboard Up/Down nav steps by exactly the column count visible on screen (was off-by-one before at boundaries). --- booru_viewer/gui/grid.py | 63 +++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/booru_viewer/gui/grid.py b/booru_viewer/gui/grid.py index dec992d..bc95075 100644 --- a/booru_viewer/gui/grid.py +++ b/booru_viewer/gui/grid.py @@ -313,28 +313,64 @@ class FlowLayout(QWidget): self._do_layout() def _do_layout(self) -> None: + """Position children in a deterministic grid. + + Uses the THUMB_SIZE / THUMB_SPACING constants instead of each + widget's actual `width()` so the layout is independent of per- + widget size variance. This matters because: + + 1. ThumbnailWidget calls `setFixedSize(THUMB_SIZE, THUMB_SIZE)` + in `__init__`, capturing the constant at construction time. + If `THUMB_SIZE` is later mutated (`_apply_settings` writes + `grid_mod.THUMB_SIZE = new_size` in main_window.py:2953), + existing thumbs keep their old fixed size while new ones + (e.g. from infinite-scroll backfill via `append_posts`) get + the new one. Mixed widths break a width-summing wrap loop. + + 2. The previous wrap loop walked each thumb summing + `widget.width() + THUMB_SPACING` and wrapped on + `x + item_w > self.width()`. At column boundaries + (window width within a few pixels of `N * step + margin`) + the boundary depends on every per-widget width, and any + sub-pixel or mid-mutation drift could collapse the column + count by 1. + + Now: compute the column count once from the container width + and the constant step, then position thumbs by `(col, row)` + index. The layout is a function of `self.width()` and the + constants only — no per-widget reads. + """ if not self._items: return - x, y = THUMB_SPACING, THUMB_SPACING - row_height = 0 width = self.width() or 800 + step = THUMB_SIZE + THUMB_SPACING + # Account for the leading THUMB_SPACING margin: a row that fits + # N thumbs needs `THUMB_SPACING + N * step` pixels minimum, not + # `N * step`. The previous formula `w // step` overcounted by 1 + # at the boundary (e.g. width=1135 returned 6 columns where the + # actual fit is 5). + cols = max(1, (width - THUMB_SPACING) // step) - for widget in self._items: - item_w = widget.width() + THUMB_SPACING - item_h = widget.height() + THUMB_SPACING - if x + item_w > width and x > THUMB_SPACING: - x = THUMB_SPACING - y += row_height - row_height = 0 + for i, widget in enumerate(self._items): + col = i % cols + row = i // cols + x = THUMB_SPACING + col * step + y = THUMB_SPACING + row * step widget.move(x, y) widget.show() - x += item_w - row_height = max(row_height, item_h) - self.setMinimumHeight(y + row_height + THUMB_SPACING) + rows = (len(self._items) + cols - 1) // cols + self.setMinimumHeight(THUMB_SPACING + rows * step) @property def columns(self) -> int: + """Same formula as `_do_layout`'s column count. + + Both must agree exactly so callers (e.g. main_window's + keyboard Up/Down nav step) get the value the visual layout + actually used. The previous version was off-by-one because it + omitted the leading THUMB_SPACING from the calculation. + """ if not self._items: return 1 # Use parent viewport width if inside a QScrollArea @@ -343,7 +379,8 @@ class FlowLayout(QWidget): w = parent.viewport().width() else: w = self.width() or 800 - return max(1, w // (THUMB_SIZE + THUMB_SPACING)) + step = THUMB_SIZE + THUMB_SPACING + return max(1, (w - THUMB_SPACING) // step) class ThumbnailGrid(QScrollArea):