Contact sheet: render tiles at proof scale (fixes the remaining memory blowup) - #666
Merged
marcinz606 merged 1 commit intoJul 29, 2026
Merged
Conversation
Follow-up to the earlier tile fix, which shrank the tiles the worker accumulates but left the dominant cost untouched: the render itself still ran the whole pipeline on the full-resolution source and threw away all but a 1200px tile. target_long_px only fed scale_factor; run_pipeline never downsamples, and the GPU path's _readback_downsampled reads the texture back at native size despite its name. Measured per 24MP frame (peak working set / peak commit): CPU 3.53GB / 4.28GB -> 1.23GB / 1.99GB GPU 1.54GB / 8.12GB -> 1.23GB / 2.70GB Peak is now flat across a roll (24 frames, no growth) instead of a multi-GB spike per frame that swapped and then failed allocation, which is what dropped tiles silently. - Downsample the source (and IR) to target_long_px before the pipeline. - Bound the paper layout to the tile as well: a Print/Target-px export setting sizes paper from print_size x DPI and would re-inflate the render to print resolution immediately after the downsample. Uses the same virtual-DPI convention as PrintService.preview_paper_layout so borders stay proportional. ORIGINAL mode is left alone (it can't upscale) to avoid changing framing for the default case. - Drop the full-res source cache entry after downsampling: contact-sheet frames are decoded once each, so it only pinned ~300MB across the next frame's decode. Tiles are unchanged visually (mean |delta| ~0.9/255 on a synthetic hard-edged target, from resampling at proof scale rather than full-res-then-shrink). render_display_array has no caller outside the contact sheet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Still reported after #600: "When I export a contact sheet, the used memory goes through the roof, swap also, and I get only one view on the contact sheet out of my 24 selected views."
#600 was a real fix but only addressed the accumulation side — the tiles the worker holds in a list. It left the dominant cost untouched.
Root cause
render_display_arraytakestarget_long_px, but that value only fedscale_factor. Nothing downsampled the input:run_pipelinerenders whatever buffer it's given, and the GPU path's_readback_downsampledreads the texture back at native size despite the name. So every contact-sheet tile ran the entire pipeline on the full-resolution source and then discarded all but a 1200px tile.Measured per 24MP frame (peak working set / peak commit,
K32GetProcessMemoryInfo):Peak is hit on the first tile, so this is per-frame transient cost, not accumulation — which is exactly what "memory through the roof + swap" describes. Once an allocation fails,
render_display_array's blanketexceptreturnsNoneand the tile is dropped, leaving a sheet with a handful (or one) of 24 frames. Machine-dependent, so it doesn't reproduce on a high-RAM box — verified 24/24 tiles survive here both before and after; the fix is validated by the memory numbers, not by a repro of the drop.Changes
target_long_pxbefore the pipeline runs. This is the actual fix.scale_factorshrinks proportionally with the buffer, so effect radii land in the same relative place.print_size × DPI, which would re-inflate the render to full print resolution right after the downsample (a 60cm @ 600dpi setting → ~14000px paper). Bounded with the same virtual-DPI conventionPrintService.preview_paper_layoutalready uses for UI previews, so borders stay proportional.ORIGINALmode is deliberately left alone — it can't upscale, and forcing PRINT there would change framing for the default case.run_batchstill benefits from the cache for multi-format presets.)Notes
render_display_arrayhas no caller outsiderun_contact_sheet, so the blast radius is the contact sheet only.scale_factorconventions here (CPU derives it frompreview_render_sizeinsiderun_pipeline, the GPU call passes it relative totarget_long_px). Both shrink proportionally so the fix is neutral to it, but they don't match each other — worth a separate look.Test plan
tests/test_contact_sheet_tiles.py: the pipeline must receive a buffer already bounded totarget_long_px(spying onrun_pipeline), and a 60cm @ 600dpi Print export must not inflate either the pipeline input or the returned tile. Both verified to fail when the source-downsample and layout-bound are neutered.test_effective_input_icc,test_output_dir_subfolder_of_source,test_sidecar_path), unrelated.make format/ruff checkclean.