focus: a grid you can read, and the zones that focus somewhere else - #37
Merged
Merged
Conversation
PR Summary by QodoAdd readable focus grids and sweep distance outlier detection
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Member
Author
|
All five were real and are fixed in 6702653. 1 — Exact-zone view hides anomaly rings. Correct, and the most embarrassing of the five: "All zones" is the view someone switches to precisely to see which zones were flagged, and it dropped every ring while the verdict went on saying they were on the picture. Both renderers now share one 2 — Saturated zones trigger false warnings. Correct, and the subtle one. A clamped zone plateaus at the ceiling, so the first reading to reach it wins the argmax — and where the counter filled up is not where the lens was sharpest. The zone would be ringed as "focuses somewhere else" on the strength of an artefact. You are right that 3 — Old warnings remain after changes. Correct. They were cleared when a sweep started and nowhere else, so opening a frame, driving the lens by hand, or applying/reverting a filter each left rings describing a camera that no longer existed. Cleared in all four paths. 4 — Changed grid shapes misplace warnings. Correct. Every index is read back as a row and column through the current width. Frames carry 5 — Zero settle still waits 700 ms. Correct — On the testsTwo of these guards came back vacuous on the first mutation pass and needed real tests written before they meant anything:
Seven more checks. All five mutations now discriminate:
Also asserts that the rest of the frame still reaches consensus when a zone is dropped for clamping — without that, the fix for #2 could quietly disable the whole feature and every other test would still pass.
|
First feedback from someone calibrating autofocus with this: "пока не понятно, нужны цифры -- меньше зон, зато в зоне рисовать цифру". Fair. 255 coloured cells show that there is a bright patch somewhere and say nothing about what any of it reads. The readout is now 3x3 by default with the value drawn in each block, digits grouped so five figures can be read at a glance, the sharpest block outlined and labelled. 3x3 / 4x4 / All zones switches it. The fine grid is still what gets measured -- this only changes what is drawn over the picture, so the readout cannot move where the peak actually is. Block value is the MEAN over measured zones, not the sum: 17 columns do not divide by three, and a sum ranks the wider block higher for being wider. Not the maximum either, which is the reduction clamping already defeats. A block with nothing believable in it is null, not zero -- zero is a value, and it sorts below every real block as though it had been measured. The same conversation named the real failure of cheap autofocus: after a year it focuses anywhere but the subject, and dirt on the glass the owner never notices is a large part of why. That turns out to be measurable with what the sweep already collects. Dirt sits millimetres from the lens, so across a defocus sweep it peaks at a lens position nowhere near the rest of the frame. The sweep reads every zone at every position and was throwing all but the maximum away. Each zone's own peak position is now kept, the consensus taken as the median -- one smeared corner peaking at the far end drags a mean toward itself and then judges everything else against a consensus it invented -- and zones far from it are ringed on the picture and named in the verdict. Two restraints. A zone that does not move across the sweep gets no opinion: a blank wall has an argmax and it is noise. And it is reported as focusing at a different distance, not as dirt -- a genuinely near object gives the same signature and is not a fault. Naming the cause is the operator's job; knowing where to look is what they could not get from a live image, where a smear reads as nothing but a soft patch. Also makes the sweep settle a host seam (sweepSettleMs) beside intervalMs and moveRepeatMs. A fixture has nothing to settle, and at 700ms the sweep checks alone outran the page's whole time budget. Every guard mutation-tested: breaking it turns exactly the checks written for it red, and no others.
Five from review, all real. "All zones" dropped every ring -- the one view someone switches to precisely to see WHICH zones were flagged, while the verdict went on saying they were on the picture. Both renderers now share the ring drawing. A clamped zone was still being judged. Saturation plateaus at the ceiling, so the FIRST reading to reach it wins the argmax, and where the counter filled up is not where the lens was sharpest: the zone got accused of focusing somewhere else on the strength of an artefact. `state` cannot carry this -- summarise keeps saturation separate on purpose, because a pinned zone is still perfectly well exposed -- so the frames now carry satZone and a zone clamped anywhere along the sweep gets no opinion at all. Findings outlived their conditions. They were cleared when a sweep started and nowhere else, so opening a frame, driving the lens by hand, or applying or reverting a filter each left rings describing a camera that no longer existed. Shape, not just zone count. Every index is read back as a row and a column through the CURRENT width, so two grids of equal size and different shape put the same index somewhere else in the picture. Frames carry rows and cols, a change mid-sweep is refused, and the finding carries its shape so drawing will not place it on a grid it does not match. sweepSettleMs: 0 waited the full 700ms. `0 || 700` -- zero is a real answer, a host with nothing to settle, and the fallback turned it into the production wait twice per step in exactly the case that asked for none. Seven more checks, including that the rest of the frame still reaches consensus when a zone is dropped for clamping: without it the fix could quietly disable the whole feature and every test would still pass. The fixture had the same truthiness bug as the code under test, which is why the settle guard first came back vacuous.
widgetii
force-pushed
the
focus-zones
branch
from
September 25, 2026 05:19
6702653 to
eacbcb9
Compare
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.
First feedback from someone actually calibrating autofocus with this:
Fair. 255 coloured cells show that there is a bright patch somewhere and say nothing about what any of it reads.
A readable grid
The readout is now 3×3 by default with the value drawn in each block, digits grouped (
50 625) so five figures read at a glance, the sharpest block outlined and labelled.3×3 / 4×4 / All zonesswitches it.The fine grid is still what gets measured — this only changes what is drawn over the picture, so the readout cannot move where the peak actually is.
Three choices worth stating:
null, not0. Zero is a value, and it sorts below every real block as though it had been measured.The zones that focus somewhere else
The same conversation named the real failure of cheap autofocus — after a year it focuses anywhere but the subject, and dirt on the glass the owner never notices is a large part of why:
That turns out to be measurable with what the sweep already collects. Dirt sits millimetres from the lens, so across a defocus sweep it peaks at a lens position nowhere near the rest of the frame. The sweep reads every zone at every position and was throwing all but the maximum away.
Each zone's own peak position is now kept, the consensus taken as the median — one smeared corner peaking at the far end drags a mean toward itself and then judges everything else against a consensus it invented — and zones far from it are ringed on the picture and named in the verdict.
Two deliberate restraints:
Also
sweepSettleMsbecomes a host seam besideintervalMsandmoveRepeatMs. A fixture has nothing to settle, and at 700 ms the sweep checks alone outran the page's whole 180 s budget.Tests
Six new assertions in
tools/smoke.mjs, thirteen intests/ui-check.html. Every guard mutation-tested — breaking it turns exactly the checks written for it red:Also asserts the negative direction: a scene that all sits at one distance is not accused, because a warning that fires on every sweep is a warning nobody reads.
node tools/smoke.mjsandnode tools/ui-check.mjsboth pass. Running on an 85H50AI as a development build viaMJ_RAW_BASE.