Colour ring-around: the RA4 filtration proof - #672
Merged
Conversation
The classic Kodak/Fuji ring-around as a canvas proof: the frame printed as a 3x3 mosaic with the centre patch at the current filtration and the ring 5cc either way on the magenta and yellow axes, so the direction of a colour cast is visible rather than guessed. Click a patch to keep its filtration. Shift+F, the target button beside the Filtration sliders, or the toolbar's overflow menu. The step is grounded rather than picked: filtration_offsets documents 1.0 slider = 20cc, so RING_CC_STEP = 0.25 is the classic RA4 5cc increment, and it keeps the centre unclamped anywhere in [-0.75, +0.75]. One proof slot, not two. _clear_test_strip() has six call sites — request_render, the toggle, the pick, and the compare / flat-peek / load_file navigate-back paths — so a parallel ring lifecycle would mean six more places to keep in sync forever, and the navigate-back path is exactly the one that gets missed. Instead the strip's slot gains a `kind`, which means every existing drop path covers the ring for free and asking for one proof while the other is up swaps rather than dismisses. The worker is generalized rather than duplicated: TestStripTask carries per-patch ExposureConfig overrides plus a grid shape, and build_strip applies them. The mosaic slicer takes the grid too. The tone strip becomes one caller, and its tests passing untouched is the regression check. Relative, not absolute — the opposite choice from the strip's ladders, for a reason: a ring-around asks "is this print neutral, and if not, which way", which an absolute filtration grid cannot ask once a correction is already dialled in. Three consequences fall out. The centre patch is the current filtration by construction, so the accent needs no nearest-rung search. The memo must NOT pin filtration — picking correctly invalidates, because the ring is meant to be iterated toward neutral and each round is genuinely new pixels. And at a slider rail the ring prints duplicate patches, which is the head running out of travel, not a bug; a test documents it rather than engineering it away. Verified end to end on samples/DSC00448.ARW from a non-neutral M=0.2 Y=-0.1 start: the centre patch is bit-identical to a plain render at those settings (max abs diff 0.0), all nine patch means are distinct, the two magenta rungs move green in opposite directions and the two yellow rungs move blue, and picking leaves cyan, density, grade, Cast Removal and both Auto toggles untouched. Per-patch cost measures 0.06s against the 36-patch strip's 0.05s, confirming the ring gets the same metering reuse.
Ring-around: - 5x5 instead of 3x3, stepping 1cc at a time out to 2cc either way on magenta and yellow. More resolution near neutral rather than a wider swing. - The ladder is now absolute and centred on neutral, like the strip's. That makes the mosaic invariant to the filtration in force, so _strip_memo_key pins M/Y the way it pins density and grade for the strip, and picking a patch no longer forces a reprint. test_strip_origin goes away with it, and so does the rail-clamping the relative ladder needed: every rung now sits well inside the sliders' travel. - The accented rung follows the filtration in force via ring_nearest_cell, the same way the strip accents the nearest density and grade. Test strip: 5x5 instead of 6x6, with both ladders centred exactly on their defaults (density 1.0, grade R115), so the settings already in force are the middle patch. Density spans 0.4-1.6 in 0.3 steps and grade R75-R155 in 20-point steps. Consequence worth knowing: with an absolute ladder the pick sets filtration outright rather than nudging it, so the ring only reaches as far as its own span. Verified on samples/DSC00448.ARW: the neutral centre patch is bit-identical to a plain neutral render (max abs diff 0.0), all 25 patch means distinct, the outer magenta rungs move green in opposite directions and the outer yellow rungs move blue, and per-patch cost matches the tone strip's at 0.05s.
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
1 participant
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.
What
The classic Kodak/Fuji ring-around, as a canvas proof: the frame printed as a 5×5 mosaic stepping 1cc at a time out to ±2cc on the magenta and yellow axes.
Shift+F, the target button beside the Filtration sliders, or the toolbar's overflow menu. Click a patch to keep its filtration.It answers the one question a colour printer can't answer by eye: not whether there's a cast, but which way it goes. Sibling to #669's test strip, which grids density × grade.
Axis labels read the rungs' absolute filtration in cc,
M -2 … +2down the left andY -2 … +2across the top, with the rung nearest the current filtration accented.This PR also retunes the test strip to 5×5 (it shares the mosaic machinery), with both ladders centred exactly on their defaults so the settings already in force are the middle patch: density 0.4-1.6 in 0.3 steps, grade R75-R155 in 20-point steps.
The finding that made it cheap
The test strip is affordable because "density and grade are absent from the analysis cache key, so the per-frame metering is reused". I checked whether that holds for filtration, since otherwise the ring would re-meter 25 times:
_analysis_cache_key(gpu_engine.py:134) keys on source hash,settings.process,settings.geometry, threeExposureConfigbooleans, andTARGETS_REVISION. No CMY field.base_key(engine.py:129) carries onlyprocess.*, geometry,distortion_k1,TARGETS_REVISION. Zero exposure fields.Measured in the E2E below, not just argued: 0.05s per patch for both proofs.
Both ladders absolute, which is what makes the cache work
The ring's rungs are fixed filtration values centred on neutral, exactly like the strip's density and grade ladders. That has three consequences, all of them simplifications:
_strip_memo_keypins M/Y the way it pins density and grade. Printing, picking a patch, and printing again is a cache hit: the patches never depended on what was dialled in.The accented rung follows the current filtration through
ring_nearest_cell, mirroring how the strip accents the nearest density and grade.One consequence worth knowing: with an absolute ladder the pick sets filtration rather than nudging it, so the ring only reaches as far as its own span.
RING_CC_STEPis a single constant if that wants widening.One proof slot, not two
The tempting shape is a second set of
AppStatefields and a parallel lifecycle. Don't — and the reason isn't code volume._clear_test_strip()has six call sites:request_render, the toggle, the pick, and the compare / flat-peek /load_filenavigate-back paths. A parallel_clear_ring_around()means six more places to keep in sync forever, and #661's own write-up shows the navigate-back path is the one that gets silently missed.So the strip's slot gains a
kind. Every existing drop path covers the ring for free, the_draw_uiguard stays one branch rather than two racing for the content rect, and asking for one proof while the other is up swaps it, which falls out of comparingkind.The worker is generalized, not duplicated:
TestStripTaskcarries per-patchExposureConfigoverride dicts plus a grid shape,build_stripapplies them, and the mosaic slicer takes the grid. The tone strip becomes one caller, and its tests passing untouched is the refactor's regression check.Step:
RING_CC_STEP = 0.05. Grounded, not guessed:filtration_offsetsdocuments1.0 slider = 20cc, so the step is 1cc and the outer rung 2cc.5×5, not a honeycomb: a hex layout needs a new slicer, new hit-testing and a new assembler to show nothing the M/Y corners already do.
Verification
make allgreen: 2706 passed, lint andtyclean. New unit tests across the ladder (absolute rungs symmetric about neutral, cc scale, accent tracking, every rung inside the slider travel), the worker's override sequence, lifecycle, swap, cache behaviour, and 5×5 hit-testing. The existing strip tests are parameterized over both grids so one test pins the shared slicer for both.Headless E2E on
samples/DSC00448.ARW:0.00e+00. If the centre isn't the print those settings produce, the whole instrument compares against something that isn't there.One honest caveat, which is in the tooltip: with Cast Removal on the patches separate less, because the neutral-axis solve is correcting toward neutral underneath. They still show the print you'd actually get.
Fourth of the darkroom-instrument set: #669 step wedge, #670 burn map, #671 grain focuser, this one. Each is independent; this is the only one that touches already-shipped code.