Skip to content

Fix contact sheet silently dropping frames on large rolls#600

Merged
marcinz606 merged 1 commit into
marcinz606:mainfrom
linkmodo:fix/contact-sheet-tile-memory
Jul 22, 2026
Merged

Fix contact sheet silently dropping frames on large rolls#600
marcinz606 merged 1 commit into
marcinz606:mainfrom
linkmodo:fix/contact-sheet-tile-memory

Conversation

@linkmodo

Copy link
Copy Markdown
Contributor

The bug

Reported on Windows: a contact sheet export processed 20+ frames according to the progress dialog but produced a sheet containing only a single image, with no error shown.

Root cause

render_display_array is documented as rendering "to a small sRGB uint8 RGB array for tiling" and takes target_long_px — but it never downsamples. target_long_px is only used to derive scale_factor (a pipeline hint for blur radii); the GPU path's _readback_downsampled reads the texture back at its native size despite the name, and the CPU path likewise returns full res.

Measured directly on a 24MP source with target_long_px=1200:

before:  source=6000x4000 -> tile.shape=(4000, 6000, 3)   72.0 MB/tile
after:   source=6000x4000 -> tile.shape=(800, 1200, 3)     2.9 MB/tile

run_contact_sheet accumulates every tile in a list before compositing, so peak memory scaled with frames × full resolution:

Scan size Per tile 20 frames held at once
24MP (6000×4000) 72 MB → 2.9 MB 1.44 GB → 58 MB
45MP (8000×5600) 134 MB → 2.9 MB 2.7 GB → 58 MB

The full resolution bought nothing: _paste_resized downsamples each tile to the 600px cell at composite time anyway.

When an allocation eventually failed, render_display_array's blanket except Exception caught it (including MemoryError), logged it, and returned None; the worker then discarded the tile with no error.emit. Since progress.emit(i + 1, total, name) fires before the render, the dialog faithfully counted every file regardless. Hence: "processed 20+", sheet built from however few survived. Machine-dependent, which is why it showed up on the reporter's box and not on a high-RAM dev machine.

Changes

  • Downsample the tile to target_long_px in render_display_array, via a new _downsample_to_long_edge helper. Applied after the display transform — the same point where the compositor resamples — so the tile is visually identical, just smaller.
  • Report dropped tiles through the existing error signal, which already feeds _export_failures and surfaces as "— N failed" in the HUD, matching run_batch's behaviour. A short sheet no longer looks like a clean success.

Per-tile processor.cleanup() was considered and rejected: the existing finally block documents a deliberate once-per-batch evacuation to avoid rebuilding the texture pool each frame, and I measured _tex_cache staying flat (86 textures) across a full same-size roll while the source cache holds a single entry. Nothing accumulates, so per-tile cleanup would cost time for no memory benefit.

Test plan

  • New tests/test_contact_sheet_tiles.py (10 tests): the _downsample_to_long_edge helper (shrink, aspect ratio, never upscale, no-op on non-positive target); the real render path honouring target_long_px on both the CPU and GPU paths against an on-disk source; failed tiles emitting one error each; a clean run emitting none and writing one sheet; cleanup staying once-per-batch.
  • Non-vacuous: neutering the downsample call fails all 3 size tests; reverting the worker change fails the error-reporting test.
  • 86 tests pass across the contact-sheet and export suites. ruff check + ruff format clean.

render_display_array is documented as producing a small tile and takes
target_long_px, but only used it to derive scale_factor — the returned
array was the full-resolution render (the GPU path's
_readback_downsampled reads the texture back at native size despite its
name). Measured on a 24MP source with target_long_px=1200: the tile came
back 6000x4000, 72MB, instead of 1200x800, 2.9MB.

run_contact_sheet holds every tile in memory until it composites, so
peak cost scaled with frames x full resolution — ~1.4GB for a 20-frame
24MP roll, ~2.7GB at 45MP. When an allocation failed, the blanket
except in render_display_array swallowed it and returned None, and the
worker dropped the tile with no error emitted. Progress is emitted
before the render, so the dialog still counted every file: a 20-frame
run could report 20 processed and produce a sheet with a single image.

Downsample the tile to target_long_px (after the display transform,
where the compositor also resamples, so it looks identical) and report
a dropped tile through the existing error signal so failures surface as
"N failed" instead of silently shrinking the sheet.

Per-tile processor.cleanup() was considered and rejected: the texture
pool stays flat across a same-size roll and the source cache holds one
entry, so the existing per-batch evacuation is correct and rebuilding
the pool every frame would only cost time.
@marcinz606
marcinz606 merged commit fdf43b7 into marcinz606:main Jul 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants