PNG cache ignored --resolution, and a multi-page cache hit crashed - #23
Merged
set-soft merged 3 commits intoSep 21, 2026
Merged
Conversation
added 2 commits
September 19, 2026 04:17
The branch that returns a cached set of multi-page PNGs checks isfile(destm) but then reads getmtime(dest1). When only the multi-page files exist, dest1 is absent and getmtime raises FileNotFoundError instead of using the cache.
pdf2png decided a cache hit from the file times alone, so a cache populated at one resolution was reused at another and --resolution was silently discarded. Diffing at 400 DPI against a cache built at 150 gave 150 DPI images. The PNG names now carry the resolution. The layer PDFs they come from are vector, so they stay shared: changing the resolution re-rasterizes but never re-plots, and caches for several resolutions coexist in one directory. The schematic path has the same flaw and is not touched here. svg2png rasterizes at the same resolution, but its PNG names double as sheet keys in layer_names, so tagging them is not a local change. Old untagged PNGs left in a cache are simply never read again.
Member
|
Ok, I'm taking the patch, but next time please separate them and tag WIP one if incomplete. |
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.
Two independent fixes in
pdf2png(), one commit each.1. A multi-page cache hit crashes
The branch that returns a cached set of multi-page PNGs tests
isfile(destm)but then readsgetmtime(dest1):When only the multi-page files are present,
dest1does not exist andgetmtimeraisesFileNotFoundErrorinstead of using the cache.2. The PNG cache ignores --resolution
A cache hit is decided from the file times alone, and the resolution is not part of the name,
so PNGs rasterized at one resolution are reused at another and
--resolutionis silentlydiscarded. Same command, same board, only the cache differs:
The second row is the bug: the run takes ~9 s instead of ~58 s and quietly produces 150 DPI
output. It is easy to hit, because the natural workflow is to keep one
--cache_dirand varythe resolution per invocation.
Why the PNG names, and not the options file
cur_pcb_opsalready invalidates the cache when KiCad or--zoneschanges, and addingresolutionthere would be a smaller diff. But the layer PDFs are vector and do not depend onthe resolution at all - only the rasterization does. Invalidating on resolution would therefore
throw away the expensive plot to redo the cheap part.
Tagging only the PNG names keeps one plot shared by every resolution, so switching
re-rasterizes but never re-plots, and caches for several resolutions coexist in one directory:
Verified on a real board: against the cache that previously broke it,
--resolution 400nowgives 4678 x 3308 and
--resolution 150gives 1754 x 1241, both served from the same PDFs.Not addressed
The schematic path has the same flaw.
svg2png()rasterizes at the same resolution, but itsPNG names double as sheet keys in
layer_names, so tagging them is not a local change andseemed better left to your judgement.
Untagged PNGs in an existing cache are simply never read again. They are not cleaned up.
Note on #22
This touches the same two files as #22, though a different function. The two are independent
and can be taken in either order; whichever merges second will conflict on the
[Unreleased]heading, where both bullets should be kept.