[Enhancement]: Grid paint and scroll costs at 125k images (delegate re-scaling, unbounded pixmap cache, filmstrip re-decode) #192

New issue

Open

opened 2026-08-29 18:28:19 +00:00 by Claude · 0 comments

Claude commented

2026-08-29 18:28:19 +00:00

Member

Affected area

ui/thumbnail_delegate.py, ui/thumbnail_model.py (_pixmap_cache, patch_tag_display, clear_thumbnail_cache), ui/image_viewer.py (_thumb_for_session_index), ui/workbench_grid.py (QListView layout mode).

Background

Split out of #191 at the app owner's direction, to keep that branch focused on open/scan/hydrate sequencing and threading. These are per-paint and per-scroll costs rather than startup costs, and they are what remains between the #191 work and a genuinely smooth 125k-image grid.

Findings

1. The delegate re-scales every visible thumbnail on every repaint.
ThumbnailDelegate.paint() calls pixmap.scaled(thumb_rect.size(), Qt.KeepAspectRatio, Qt.SmoothTransformation) on every paint, with no cache of the scaled result. Display size only changes when the slider or a resize fires, but every scroll re-does a smooth resample of every visible cell. A cache keyed by (pixmap.cacheKey(), width, height) would make regenerated thumbnails miss naturally; bound it (~512 entries, LRU) and clear it in set_thumb_edge/set_cell_size.

2. Per-paint allocations that could be cached.
Each paint() builds a fresh QFont(option.font), a QFontMetrics inside _elide_filename, and a QPainterPath per tag badge; _tag_badge_color() runs hashlib.md5() per uncoloured badge per paint. Cache the derived font/metrics keyed by option.font.key(), and @lru_cache the badge colour by tag name.

3. ImageListModel._pixmap_cache is unbounded.
A plain dict[str, QPixmap] with no eviction. At 256px/32bpp a cached pixmap is roughly 256 KB, so scrolling a 125k-image workspace end to end can accumulate tens of GB. Replace with an OrderedDict plus move_to_end on hit and a cap (GRID_PIXMAP_CACHE_MAX, ~1200 ≈ 300 MB, still many viewports deep). Worth framing in core/constants.py against DR-009's memory/CPU trade-off.

4. The filmstrip re-decodes JPEGs on every repaint.
ImageViewer._thumb_for_session_index() checks the full-resolution _display_cache (usually a miss for thumbnails), then unconditionally QPixmap().loadFromData(blob), and only then falls back to the model's decoration role — which is the model's already-decoded _pixmap_cache. Reordering those two lookups removes most redundant decodes for free. A small bounded thumb cache (~256) would cover the remainder; invalidate it alongside _display_cache and on _on_thumbnails_ready for the updated paths.

5. patch_tag_display() is O(n) per tag rename/recolour.
for i, item in enumerate(self._images) scans every row regardless of how many carry the tag. At 125k that is a full scan on every rename or colour change. A tag-name → rows index would make it proportional to matches.

6. clear_thumbnail_cache() emits one dataChanged spanning the whole model.
Called after a save-resolution change. It clears _pixmap_cache first, so every visible cell then synchronously re-decodes on the next paint. Bounded by viewport size rather than row count, but still a burst worth measuring.

7. setLayoutMode(Batched) above a row threshold — needs manual verification.
QListView defaults to SinglePass with batchSize=100; neither setLayoutMode nor setBatchSize is called anywhere in the repo. With setResizeMode(Adjust) on 125k uniform items, every scheduleDelayedItemsLayout() lays out all of them in one main-thread pass. Batched fixes that, but makes scrollTo(), setCurrentIndex() to not-yet-laid-out rows, and scrollbar range approximate until layout completes — which _select_proxy_row, the viewer↔grid round trip, GridSelectionFilter and GridWasdNavFilter all depend on. Suggest adopting it only above a threshold (e.g. rowCount() > 20_000) and verifying scroll/selection manually at scale, since headless tests cannot substitute for that.

Note: setViewportUpdateMode was considered and ruled out — it is a QGraphicsView API, not available on QListView.

Acceptance criteria

Scrolling a 125k-image grid does not re-scale or re-decode already-visible thumbnails.
The grid pixmap cache has a documented bound; memory does not grow without limit while scrolling a large workspace.
Filmstrip repaints reuse decoded thumbnails instead of decoding per paint.
Tag rename/recolour cost is proportional to affected rows, not workspace size.
Any setLayoutMode(Batched) adoption is verified manually for scroll position, selection, and viewer round-trip at 100k+ rows.
Tests follow the repo's monkeypatch-and-count idiom (e.g. QPixmap.scaled called once for two paints at the same size).

## Affected area `ui/thumbnail_delegate.py`, `ui/thumbnail_model.py` (`_pixmap_cache`, `patch_tag_display`, `clear_thumbnail_cache`), `ui/image_viewer.py` (`_thumb_for_session_index`), `ui/workbench_grid.py` (`QListView` layout mode). ## Background Split out of #191 at the app owner's direction, to keep that branch focused on open/scan/hydrate sequencing and threading. These are per-paint and per-scroll costs rather than startup costs, and they are what remains between the #191 work and a genuinely smooth 125k-image grid. ## Findings **1. The delegate re-scales every visible thumbnail on every repaint.** `ThumbnailDelegate.paint()` calls `pixmap.scaled(thumb_rect.size(), Qt.KeepAspectRatio, Qt.SmoothTransformation)` on every paint, with no cache of the scaled result. Display size only changes when the slider or a resize fires, but every scroll re-does a smooth resample of every visible cell. A cache keyed by `(pixmap.cacheKey(), width, height)` would make regenerated thumbnails miss naturally; bound it (~512 entries, LRU) and clear it in `set_thumb_edge`/`set_cell_size`. **2. Per-paint allocations that could be cached.** Each `paint()` builds a fresh `QFont(option.font)`, a `QFontMetrics` inside `_elide_filename`, and a `QPainterPath` per tag badge; `_tag_badge_color()` runs `hashlib.md5()` per uncoloured badge per paint. Cache the derived font/metrics keyed by `option.font.key()`, and `@lru_cache` the badge colour by tag name. **3. `ImageListModel._pixmap_cache` is unbounded.** A plain `dict[str, QPixmap]` with no eviction. At 256px/32bpp a cached pixmap is roughly 256 KB, so scrolling a 125k-image workspace end to end can accumulate tens of GB. Replace with an `OrderedDict` plus `move_to_end` on hit and a cap (`GRID_PIXMAP_CACHE_MAX`, ~1200 ≈ 300 MB, still many viewports deep). Worth framing in `core/constants.py` against DR-009's memory/CPU trade-off. **4. The filmstrip re-decodes JPEGs on every repaint.** `ImageViewer._thumb_for_session_index()` checks the full-resolution `_display_cache` (usually a miss for thumbnails), then unconditionally `QPixmap().loadFromData(blob)`, and only *then* falls back to the model's decoration role — which is the model's already-decoded `_pixmap_cache`. Reordering those two lookups removes most redundant decodes for free. A small bounded thumb cache (~256) would cover the remainder; invalidate it alongside `_display_cache` and on `_on_thumbnails_ready` for the updated paths. **5. `patch_tag_display()` is O(n) per tag rename/recolour.** `for i, item in enumerate(self._images)` scans every row regardless of how many carry the tag. At 125k that is a full scan on every rename or colour change. A tag-name → rows index would make it proportional to matches. **6. `clear_thumbnail_cache()` emits one `dataChanged` spanning the whole model.** Called after a save-resolution change. It clears `_pixmap_cache` first, so every visible cell then synchronously re-decodes on the next paint. Bounded by viewport size rather than row count, but still a burst worth measuring. **7. `setLayoutMode(Batched)` above a row threshold — needs manual verification.** `QListView` defaults to `SinglePass` with `batchSize=100`; neither `setLayoutMode` nor `setBatchSize` is called anywhere in the repo. With `setResizeMode(Adjust)` on 125k uniform items, every `scheduleDelayedItemsLayout()` lays out all of them in one main-thread pass. `Batched` fixes that, but makes `scrollTo()`, `setCurrentIndex()` to not-yet-laid-out rows, and scrollbar range approximate until layout completes — which `_select_proxy_row`, the viewer↔grid round trip, `GridSelectionFilter` and `GridWasdNavFilter` all depend on. Suggest adopting it only above a threshold (e.g. `rowCount() > 20_000`) and verifying scroll/selection manually at scale, since headless tests cannot substitute for that. **Note:** `setViewportUpdateMode` was considered and ruled out — it is a `QGraphicsView` API, not available on `QListView`. ## Acceptance criteria - [ ] Scrolling a 125k-image grid does not re-scale or re-decode already-visible thumbnails. - [ ] The grid pixmap cache has a documented bound; memory does not grow without limit while scrolling a large workspace. - [ ] Filmstrip repaints reuse decoded thumbnails instead of decoding per paint. - [ ] Tag rename/recolour cost is proportional to affected rows, not workspace size. - [ ] Any `setLayoutMode(Batched)` adoption is verified manually for scroll position, selection, and viewer round-trip at 100k+ rows. - [ ] Tests follow the repo's monkeypatch-and-count idiom (e.g. `QPixmap.scaled` called once for two paints at the same size).

Claude added the

Kind/Enhancement

Priority/Medium

Reviewed/Confirmed

labels

2026-08-29 18:28:24 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

ai-collab/bulk-image-organizer#192

No description provided.

Rows
Columns