diff --git a/README.md b/README.md index ef9e74d..d603754 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ both the camera's own JPEG and this editor rendered the chart's grey row 20 to balance. Clicking one grey patch brought the row to within a couple of levels of neutral. -**Diagnose** reports what is wrong with the sensor rather than with the picture: +**Bad pixels** reports what is wrong with the sensor rather than with the picture: pixels that disagree with every one of their same-colour neighbours by more than the noise explains, the black level the frame itself implies against the one the file claims, what has already clipped, and how noisy the rest is. All diff --git a/dist/editor.css b/dist/editor.css index 4abc424..bf63402 100644 --- a/dist/editor.css +++ b/dist/editor.css @@ -8,7 +8,18 @@ * * Values are the WebUI dark theme's own, lifted rather than re-picked: * page #14161c · panel #1c1f27 · raised #232734 · border #2c313d - * body #e6e8ee · secondary #b6b9c2 · tertiary #878a94 · accent #5c70e8 + * body #e6e8ee · secondary #b6b9c2 · tertiary #a2a6b2 · accent #5c70e8 + * + * The tertiary used to be #878a94 -- 4.32:1 against the raised #232734 that + * panels are drawn in, under the 4.5:1 minimum, on the class carrying every + * piece of explanatory prose in the editor. It passed on the #1c1f27 rail at + * 4.78:1, which is presumably how it survived. + * + * #a2a6b2 replaces it: 6.12:1 on #232734, 6.77:1 on #1c1f27. Measured with the + * WCAG formula rather than by eye, checked first against known pairs -- white + * on black 21.00, #767676 on white 4.54. (The wrong colour was named here at + * first: a blanket replace of the old value rewrote the sentence describing + * it, leaving a comment that contradicted itself.) * stage #0d0f14 */ .re-root, .re-root * { box-sizing: border-box; } @@ -24,10 +35,10 @@ } .re-mono { font-family: 'PT Mono', ui-monospace, Menlo, Consolas, monospace; font-variant-numeric: tabular-nums; } -.re-cap { color: #878a94; font-size: 11px; font-weight: 600; letter-spacing: .07em; +.re-cap { color: #a2a6b2; font-size: 11px; font-weight: 600; letter-spacing: .07em; text-transform: uppercase; margin: 0; } .re-rule { flex: 1 1 auto; height: 1px; background: #2c313d; min-width: 8px; } -.re-note { color: #878a94; font-size: 11px; line-height: 1.45; } +.re-note { color: #a2a6b2; font-size: 11px; line-height: 1.45; } .re-top { flex: 0 0 52px; display: flex; align-items: center; gap: 12px; padding: 0 14px; background: #1c1f27; border-bottom: 1px solid #2c313d; } @@ -41,9 +52,13 @@ .re-top { gap: 8px; padding: 0 9px; } .re-top > .re-chip { display: none; } .re-btn > span { display: none; } - .re-btn { padding: 6px 8px; } + /* NOT less padding. This used to shrink to 6px 8px, which took the + buttons further from a finger-sized target exactly on the devices most + likely to be touched. */ + .re-btn { padding: 9px 11px; } } + .re-mid { flex: 1 1 auto; display: flex; min-height: 0; } .re-stage { flex: 1 1 auto; position: relative; display: flex; align-items: center; justify-content: center; background: #0d0f14; min-width: 0; overflow: auto; } @@ -88,10 +103,15 @@ .re-insp { flex: 0 0 328px; display: flex; flex-direction: column; gap: 9px; padding: 12px; background: #1c1f27; border-left: 1px solid #2c313d; overflow-y: auto; } .re-foot { flex: 0 0 30px; display: flex; align-items: center; gap: 16px; padding: 0 14px; - background: #1c1f27; border-top: 1px solid #2c313d; color: #878a94; font-size: 11px; } + background: #1c1f27; border-top: 1px solid #2c313d; color: #a2a6b2; font-size: 11px; } .re-panel { background: #232734; border: 1px solid #2c313d; border-radius: 12px; padding: 9px; } .re-shead { display: flex; align-items: center; gap: 9px; margin-bottom: 7px; } +/* The disclosure marker is drawn by the summary row itself, so the browser's + own triangle is taken away on both engines that draw one. */ +.re-root summary::-webkit-details-marker { display: none; } +.re-root summary { list-style: none; } +.re-root details[open] > summary .re-note { transform: rotate(180deg); } .re-seg { display: flex; gap: 2px; padding: 2px; background: #232734; border: 1px solid #2c313d; border-radius: 8px; width: fit-content; } @@ -128,13 +148,13 @@ * a bar filling from there, so how far you have moved is readable without * doing arithmetic. */ .re-row { display: flex; align-items: center; gap: 10px; height: 25px; } -.re-rname { flex: 0 0 70px; color: #878a94; font-size: 11px; font-weight: 600; +.re-rname { flex: 0 0 70px; color: #a2a6b2; font-size: 11px; font-weight: 600; letter-spacing: .07em; text-transform: uppercase; } .re-track { position: relative; flex: 1 1 auto; height: 25px; } .re-tbg, .re-fill { position: absolute; top: 10px; height: 4px; border-radius: 2px; } .re-tbg { left: 0; right: 0; background: rgba(230,232,238,.10); } .re-fill { background: #5c70e8; } -.re-tick { position: absolute; top: 6px; width: 1px; height: 12px; background: #878a94; } +.re-tick { position: absolute; top: 6px; width: 1px; height: 12px; background: #a2a6b2; } .re-track input { position: relative; z-index: 2; appearance: none; width: 100%; height: 25px; margin: 0; padding: 0; background: transparent; border: 0; cursor: ew-resize; } .re-track input::-webkit-slider-thumb { appearance: none; width: 14px; height: 14px; @@ -143,7 +163,7 @@ background: #e6e8ee; box-shadow: 0 1px 3px rgba(0,0,0,.45); } .re-rnum { flex: 0 0 46px; color: #b6b9c2; font-size: 12px; text-align: right; font-family: 'PT Mono', ui-monospace, monospace; font-variant-numeric: tabular-nums; } -.re-rst { flex: 0 0 16px; border: 0; background: none; padding: 0; color: #878a94; +.re-rst { flex: 0 0 16px; border: 0; background: none; padding: 0; color: #a2a6b2; cursor: pointer; line-height: 0; } .re-rst:hover { color: #5c70e8; } .re-rst:disabled { opacity: .18; cursor: default; } @@ -182,3 +202,41 @@ /* The deviation histogram. currentColor so the bars and rules follow the * panel's text colour rather than needing their own palette. */ .re-devplot { display: block; width: 100%; height: auto; color: #b9bec9; } + +/* + * A phone. + * + * The rail is a fixed 328px column that never shrank, so at 400px the picture + * was left about 71px of a 400px screen -- a strip, beside a panel. And the + * tab bar is nowrap inside a row whose children may not shrink, with no + * overflow anywhere, so a fifth tab and the Capture and Download buttons + * simply left the screen with no way to reach them. + * + * So: stack, and let the bar scroll. DOM order is already picture-then-rail, + * so nothing has to move. + */ +@media (max-width: 560px) { + .re-mid { flex-direction: column; } + .re-stage { + flex: 0 0 auto; + /* Something to look at, and a width for fitStep() to divide by: the + zoom step is worked out from stage.clientWidth, and a stage flexed + to nothing yields a degenerate step. */ + min-height: 42vh; + } + .re-insp { + flex: 1 1 auto; width: 100%; + border-left: 0; border-top: 1px solid #2c313d; + } + /* The bar gets somewhere to go rather than off the edge. */ + .re-top { overflow-x: auto; scrollbar-width: none; } + .re-top::-webkit-scrollbar { display: none; } + /* A finger, not a mouse: 44px is the guideline and the main tab bar was + 25. 12px text is 18px on a 1.5 line, so 13px of padding makes 44 with no + border and 12px makes 44 with one. */ + .re-seg button, .re-seg.re-tight button { padding: 13px; font-size: 12px; } + .re-btn, .re-btn.re-sm { padding: 12px 13px; font-size: 12px; } + .re-rst { flex-basis: 44px; height: 44px; } + .re-row { height: 44px; } + summary { padding: 8px 0; } +} diff --git a/dist/editor.js b/dist/editor.js index 3efdd1a..e6b3258 100644 --- a/dist/editor.js +++ b/dist/editor.js @@ -13,7 +13,8 @@ */ import { solveFromPatches, patchCentres, scoreCcm, CHART_COLS, CHART_ROWS } from './calibrate.js'; -import { parseIni, readColour, fitAwbCurve, gainsForCt, mergeCcmTables, colourFragment } from './iqprofile.js'; +import { parseIni, readColour, fitAwbCurve, gainsForCt, mergeCcmTables, colourFragment, + readDefectCorrection, enableDefectCorrection } from './iqprofile.js'; import { summarise, peakHold, normalise } from './aftune.js'; const CFA_NAMES = ['RGGB', 'GRBG', 'GBRG', 'BGGR']; @@ -50,6 +51,12 @@ const svg = (d, w = 20) => const ICON = { back: svg('', 18), reset: svg('', 13), + /* A tick, so a good answer does not arrive wearing a warning triangle. + * re-ok and re-warn were told apart only by an icon colour and a 60%-alpha + * border, and most call sites handed both variants the same triangle. */ + ok: '' + + '', warn: '' + '', @@ -249,6 +256,22 @@ export function mountEditor(root, { * sections given into the camera's profile. revert() and keep() then * answer for whichever was written last. */ calibrate, + /* + * What the camera does about its own bad pixels, and how to change it: + * { profile(), patch(ini), revert(), keep(), holdSeconds }. + * + * profile() resolves to the camera's IQ profile as text; patch() writes + * sections into it; revert() and keep() answer for the last patch, the + * same bargain calibrate strikes. Without one the defect scan still + * measures and still gives its verdict -- it simply cannot say what the + * camera is doing about what it found, and says that instead of guessing. + * + * Worth knowing before wiring one up: a coordinate list cannot be loaded + * into these parts. The static defect table is refused by the chip on a + * gk7205v300 and the profile carries none, so what is on offer is the + * camera's own automatic corrector, not this scan's findings. + */ + sensor, /* Live focus statistics: { zones(), intervalMs }. zones() resolves to the * camera's AF grid -- { rows, cols, zones: [[h1,h2,v1,v2,y,hlcnt], ...] }, * row-major. Without one there is no Focus tab at all, because a focus @@ -374,7 +397,12 @@ export function mountEditor(root, { sensorChip.hidden = true; const modeItems = [ { label: 'Develop', value: 'develop' }, - { label: 'Diagnose', value: 'diagnose' }, + /* Named for what it finds, not for what it does to find it. "Diagnose" + * is a verb that never says what is being diagnosed, and an owner + * looking for stuck pixels had no reason to guess it was in here. The + * mode key stays `diagnose` -- it is internal, and renaming it would + * be churn with nothing visible at the end of it. */ + { label: 'Bad pixels', value: 'diagnose' }, { label: 'Calibrate', value: 'calibrate' }, ]; // Not created and disabled: a tab that can never work is worse than no tab, @@ -1090,10 +1118,10 @@ export function mountEditor(root, { * and how much blur it survives before it stops reading. * * It lives on the Plates tab, under the list the plate is chosen from. It - * used to sit in Diagnose, which is about the sensor's own health and not + * used to sit in Bad pixels, which is about the sensor's own health and not * about the picture -- and every one of these questions is about one * plate, so the answer differed across a frame while the card sat on a tab - * with nothing to pick a plate on. Diagnose grew a guided run afterwards + * with nothing to pick a plate on. That tab grew a guided run afterwards * and this card landed in the middle of it, between the steps and their * result, which is what finally made the misfiling obvious. * @@ -1250,11 +1278,215 @@ export function mountEditor(root, { */ const HUNT_WANT = 5; + /* Assigned by buildDiagnose, which owns the button and the panel it writes + * into; the gate calls it from outside that closure. */ + let runScan = async () => {}; + + /* + * How many stuck pixels is too many. + * + * There is no standard answer -- EMVA 1288 declines to define a defective + * pixel at all, on the grounds that no one definition serves every + * application. So this uses the only published acceptance figure the tree + * already cites: Sony sell an IMX415 as good with up to 800 white pixels + * in the dark, and that part is 3864x2192, so the limit they are willing + * to ship is 9.4e-5 of it. Rounded to a hundredth of a percent, and + * applied as a FRACTION so it carries across sensors of other sizes. + * + * It is a weak yardstick and the copy says so rather than pretending to a + * pass mark. It is also strongly conditional: the same lab camera reports + * 882 sites at 0.5 s and 3921 at 7 s, which is dark current doing exactly + * what it should, so a verdict that ignored exposure would call one + * healthy sensor both fine and faulty within a minute. The exposure and + * the gain go in the sentence for that reason. + */ + const NORMAL_FRACTION = 1e-4; + + /* The camera's own answer, read once per run and remembered: it is a + * fetch, and the panel rebuilds on every step. */ + let dpc = null, dpcAsked = false, dpcHold = null; + + async function readDpc() { + if (dpcAsked || !sensor || !sensor.profile) return; + dpcAsked = true; + try { + /* null means the profile named no such section -- an answer, and a + * different one from "still asking". Left as null it read as a + * request in flight, and since dpcAsked blocks a retry the card + * said "Asking the camera..." for as long as the editor was open. */ + dpc = readDefectCorrection(parseIni(await sensor.profile())) + || { absent: true }; + } catch (e) { + dpc = { failed: e && e.message ? e.message : 'could not be read' }; + } + if (mode === 'diagnose') buildDiagnose(); + } + + /* + * What the camera is doing about them, under the verdict. + * + * This is the half of the owner's question the scan could never answer on + * its own -- a raw capture is taken ahead of the camera's processing, so + * the defects are in it whether or not the camera hides them downstream. + * Only the camera can say, so it is asked. + */ + function dpcCard(panel) { + if (!sensor || !sensor.profile) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: 'Cameras of this kind correct stuck pixels themselves as they ' + + 'run, and a raw capture is taken before that happens. Opened from the ' + + 'camera\u2019s own page, this would say whether yours has it switched on.', + })); + return; + } + if (!dpc) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', textContent: 'Asking the camera\u2026' })); + return; + } + if (dpc.absent) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: 'This camera\u2019s profile does not say whether it corrects ' + + 'stuck pixels, so there is nothing here to read or change.', + })); + return; + } + if (dpc.failed) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: 'The camera did not say whether it corrects these: ' + dpc.failed })); + return; + } + const box = el('div', 're-notice ' + (dpc.enabled ? 're-ok' : 're-warn'), + dpc.enabled ? ICON.ok : ICON.warn); + box.style.cssText = 'margin:0 0 9px'; + box.dataset.act = 'dpc-state'; + box.append(Object.assign(el('div'), { + /* + * What the camera REPORTS, not what was measured of the result. + * + * Proving the corrector actually removes these from the video + * needs a full-resolution snapshot taken beside the raw frame, and + * the lab camera cannot produce one: at 2592x1944 the JPEG encoder + * has no media memory left beside the H.265 stream, whichever way + * isp.blkCnt is moved -- one block short at 4, none to spare at 5 + * and above. So this says the switch is on and what that is for, + * and stops there. + */ + textContent: dpc.enabled + ? 'Your camera\u2019s own correction for these is switched on, so they should ' + + 'not reach the video it sends. This capture is read from the sensor ' + + 'ahead of that, on purpose — it is the only way to see what the sensor ' + + 'is really doing.' + : 'Your camera can hide these in the video it sends, and that is switched ' + + 'off. Turning it on costs nothing else in the picture.', + })); + panel.append(box); + if (dpc.enabled || !sensor.patch) return; + + const on = el('button', 're-btn re-pri', ''); + on.dataset.act = 'dpc-on'; + on.textContent = dpcHold ? 'Keep it on' : 'Turn it on'; + on.addEventListener('click', async () => { + on.disabled = true; + try { + if (dpcHold) { + /* Stop the clock FIRST. Setting the handle to null does not + * cancel the timer it names, so a kept change sat there + * until the hold expired and was then quietly put back -- + * while the card, rebuilt, went on saying it was on. */ + clearTimeout(dpcHold); + dpcHold = null; + await sensor.keep(); + dpc.enabled = true; + } + else { + await sensor.patch(enableDefectCorrection(dpc)); + /* Same hold-and-confirm as a calibration: written now, put + * back on its own unless someone says to keep it. */ + dpcHold = setTimeout(async () => { + dpcHold = null; + try { await sensor.revert(); } catch (e) { /* going anyway */ } + if (mode === 'diagnose') buildDiagnose(); + }, Math.max(5, sensor.holdSeconds || 30) * 1000); + } + } catch (e) { + dpc = { failed: e && e.message ? e.message : 'the write was refused' }; + } + if (mode === 'diagnose') buildDiagnose(); + }); + panel.append(on); + if (dpcHold) + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:7px 0 0', + textContent: 'Switched on. It goes back by itself in ' + + Math.max(5, sensor.holdSeconds || 30) + ' seconds unless you keep it.', + })); + } + + function sensorVerdict(pts, partial) { + const i = state.info || {}; + const total = (i.width || 0) * (i.height || 0); + if (!pts || !total) return null; + const n = pts.length; + const frac = n / total; + const share = frac >= 1e-4 ? (frac * 100).toFixed(3) + '%' + : (frac * 100).toFixed(4) + '%'; + const sp = spreadOf(pts, i.width, i.height); + const taken = (i.exposure ? 'Measured at ' + (i.exposure * 1000).toFixed(0) + ' ms' + : 'Measured') + (i.iso ? ' and ISO ' + i.iso : '') + + '. Dark current builds with time and gain, so a longer exposure or ' + + 'more of either finds more of them.'; + /* What a raw capture is: the sensor before the camera has done + * anything to the picture. True by construction -- it is the point of + * the format -- and it stops short of claiming the camera's own + * corrector actually hides these, which is a separate measurement. */ + const raw = ' This is read from the sensor ahead of the camera\u2019s own ' + + 'processing, so it is not what the video shows.'; + + if (n === 0) + return ['re-ok', 'Nothing came back in every capture. On a healthy sensor ' + + 'that is the right answer.' + ' ' + taken]; + if (sp && sp.index < 0.8) + return ['re-warn', `${n} sites kept coming back, but they are clumped rather ` + + 'than scattered — which is what detail in a picture looks like, and not ' + + 'what a sensor looks like. Cover the lens, or move the camera between ' + + 'captures, and run it again.']; + /* + * A short list cannot be called a normal one. + * + * When a capture filled the scan's store it stopped part-way down the + * frame, so the count is low by an unknown amount in a known place -- + * and a fraction computed from it is low too. "Normal" would then be + * read off the very evidence that went missing. + */ + if (partial) + return ['re-warn', `${n} stuck pixels confirmed — ${share} of this sensor, and ` + + 'scattered at random, so they are the sensor rather than the scene. How ' + + 'many there are in total is not known: at least one of these captures found ' + + 'more than the scan can hold and stopped part-way down the frame. ' + taken]; + if (frac <= NORMAL_FRACTION) + return ['re-ok', `${n} stuck pixels — ${share} of this sensor, scattered at ` + + 'random, which is what a sensor\u2019s own faults look like. That is a ' + + 'normal number: parts of this class are sold as good with up to about a ' + + 'hundredth of a percent. ' + taken + raw]; + return ['re-warn', `${n} stuck pixels — ${share} of this sensor. They are ` + + 'scattered at random, so they are the sensor rather than the scene, but it ' + + 'is more than parts of this class are usually sold with (about a hundredth ' + + 'of a percent). ' + taken + raw]; + } + function startHunt() { // A run starts from nothing kept, or the first tally would count // captures the operator has forgotten about. scanSet = []; needTouched = false; + /* Ask the camera again. Its correction can be changed from elsewhere + * between runs, and a second run reporting the first run's reading is + * worse than not reporting one. */ + if (!dpcHold) { dpc = null; dpcAsked = false; } /* And from looking everywhere, whatever the last run or the operator * left the gate on. The first capture decides whether to narrow it, * and narrowing a frame that is dark all over destroys the answer @@ -1278,10 +1510,22 @@ export function mountEditor(root, { buildDiagnose(); try { if (capture) { + /* + * Two ways a capture fails to land, and they need different + * sentences. A timeout or a throw puts its own reason on the + * stage, so pointing at it is fair. But takeFrame also returns + * in silence when the editor is already busy -- fetching the + * first frame, most often -- and that path posts nothing, so + * "the message above says why" pointed at blank space. + */ + if (state.busy) + throw new Error('The editor is still busy with the last frame. ' + + 'Give it a moment and press this again.'); const was = opened; await takeFrame(); if (opened === was) - throw new Error('That capture did not arrive. The message above says why.'); + throw new Error('That capture did not arrive — the reason is on the ' + + 'picture, above.'); } else if (!state.info) { throw new Error('Drop a frame on the picture first.'); } else if (hunt.seen === opened) { @@ -1448,14 +1692,35 @@ export function mountEditor(root, { for (const [, c] of tally) if (c === hunt.taken) every++; panel.append(Object.assign(el('p', 're-note'), { style: 'margin:0 0 7px', - textContent: 'So far: ' + tally.size + ' site' + (tally.size === 1 ? '' : 's') + + textContent: (done ? 'Across ' + hunt.taken + ' pictures: ' : 'So far: ') + + tally.size + ' site' + (tally.size === 1 ? '' : 's') + ', ' + every + ' of which ' + (every === 1 ? 'has' : 'have') + ' turned up every time.', })); panel.lastChild.dataset.act = 'hunt-progress'; - if (hunt.dark !== null) { - const box = el('div', 're-notice ' + (hunt.dark ? 're-ok' : 're-warn')); + /* + * Said once, when it is decided, and then kept as a single line. + * + * All of this -- the covered/not-covered finding and the paragraph + * about exposure below it -- used to be reprinted unchanged after + * every capture, so the panel was nine lines of advice the reader + * had already taken, three captures ago, with the one number that + * had changed buried in it. + */ + const fresh = hunt.taken === 1 || done; + if (hunt.dark !== null && !fresh) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: hunt.dark + ? 'Lens covered — leave the camera where it is.' + : 'Looking only where the picture is dark — move the camera a ' + + 'little between pictures.', + })); + } + if (hunt.dark !== null && fresh) { + const box = el('div', 're-notice ' + (hunt.dark ? 're-ok' : 're-warn'), + hunt.dark ? ICON.ok : ICON.warn); box.style.cssText = 'margin:0 0 9px'; box.dataset.act = 'hunt-verdict'; box.append(Object.assign(el('div'), { @@ -1482,7 +1747,7 @@ export function mountEditor(root, { * 3921 at 7 s. Which is also the warning: a map taken at seven * seconds is not a map of the camera that runs at a thirtieth. */ - if (state.info && state.info.exposure) + if (fresh && !done && state.info && state.info.exposure) panel.append(Object.assign(el('p', 're-note'), { style: 'margin:0 0 9px', textContent: 'Taken at ' + (state.info.exposure * 1000).toFixed(1) + @@ -1508,11 +1773,17 @@ export function mountEditor(root, { if (!done) { const go = el('button', 're-btn re-pri', ''); go.dataset.act = 'hunt-step'; + /* "Take picture N of 5" rather than "Capture": there is a Capture + * button in the chrome already, and a tester's script clicked the + * wrong one of the two. */ + const waiting = state.busy || !state.info; go.textContent = hunt.busy ? 'Working…' - : capture ? (hunt.taken === 0 ? 'Capture' : 'Capture ' + (hunt.taken + 1) + - ' of ' + HUNT_WANT) - : 'Read the frame on screen'; - go.disabled = hunt.busy; + : waiting ? 'Waiting for a frame…' + : capture ? 'Take picture ' + (hunt.taken + 1) + ' of ' + HUNT_WANT + : 'Read the frame on screen'; + /* Live before the first frame has arrived, this produced a capture + * that could not run and an error about it. */ + go.disabled = hunt.busy || waiting; go.addEventListener('click', huntStep); row.append(go); /* No capture provider: the frames have to be brought in by hand, @@ -1530,6 +1801,24 @@ export function mountEditor(root, { fin.addEventListener('click', finishHunt); row.append(fin); } else { + /* + * The answer, first and in one sentence. + * + * A run used to end on a download button and a count, which left + * the owner holding a text file and no idea whether the number in + * it was bad news. Everything under this is the working. + */ + const v = sensorVerdict(diag && diag.fromTally ? diag.defects : null, + !!hunt.clipped); + if (v) { + const box = el('div', 're-notice ' + v[0], v[0] === 're-ok' ? ICON.ok : ICON.warn); + box.style.cssText = 'margin:0 0 9px'; + box.dataset.act = 'hunt-verdict-final'; + box.append(Object.assign(el('div'), { textContent: v[1] })); + panel.append(box); + } + readDpc(); + dpcCard(panel); if (hunt.clipped) { const box = el('div', 're-notice re-warn'); box.style.cssText = 'margin:0 0 9px'; @@ -1563,16 +1852,25 @@ export function mountEditor(root, { function buildDiagnose() { insp.replaceChildren(); insp.append(buildHunt()); + /* + * One frame, by hand -- folded away under the guided run. + * + * These were side by side, which put two five-capture methods and two + * buttons labelled Capture on one tab; a tester's script pressed the + * wrong one of the two on its first attempt. The guided run is the way + * in, and this is the same machinery for someone who already knows + * what they want from it. + */ const panel = el('div', 're-panel'); - panel.append(Object.assign(el('div', 're-shead'), { - innerHTML: '

Sensor

', - })); + /* Everything the manual path needs goes in here, and the whole of it + * is folded away at the bottom of the tab. */ + const body = el('div'); const run = el('button', 're-btn re-pri', ''); run.dataset.act = 'scan'; run.textContent = 'Scan the frame'; - panel.append(run); - panel.append(Object.assign(el('p', 're-note'), { - textContent: 'Read off the mosaic, before any interpolation. A frame of ' + + body.append(run); + body.append(Object.assign(el('p', 're-note'), { + textContent: 'Reads the mosaic exactly as the sensor sent it. A frame of ' + 'something flat and out of focus gives the cleanest answer — and a ' + 'dark one gives the only reliable one.', style: 'margin:8px 0 0', @@ -1600,14 +1898,24 @@ export function mountEditor(root, { ]) { const b = el('button', pct === bgPercent ? 'on' : '', label); b.title = note; - b.addEventListener('click', () => { + /* + * Mid-run this would leave scanSet holding captures read under + * different gates, which the tally then counts as if they were + * comparable -- a site can only turn up in the captures whose gate + * admitted it. The run owns the gate while it is open. + */ + b.disabled = !!hunt && !hunt.done; + if (b.disabled) b.title = 'The run sets this while it is going.'; + b.addEventListener('click', async () => { if (bgPercent === pct) return; bgPercent = pct; - // The old reading was taken somewhere else and no longer - // describes what is being asked for. diag = null; buildDiagnose(); drawMarks(); + /* Re-read rather than leaving an empty panel where a reading + * was. Choosing where to look is a question about this frame, + * and it used to be answered by removing the answer. */ + if (state.info) await runScan(); }); gateSeg.append(b); } @@ -1635,19 +1943,23 @@ export function mountEditor(root, { 'sensor from a scene, and there is no scene here. Everywhere is the ' + 'right answer for a frame like this one.', })); - panel.append(gate); + body.append(gate); + panel.append(foldout('Advanced — one frame at a time', (b) => b.append(body))); insp.append(panel); const out = el('div', 're-panel'); out.hidden = true; insp.append(out); - // A scan that is still valid is still worth showing. Leaving Diagnose + // A scan that is still valid is still worth showing. Leaving the tab // and coming back rebuilt an empty panel over a reading that had not // gone anywhere -- and the marks stayed on the picture, so there were // rings with nothing to explain them. if (diag) renderDiagnose(out); - run.addEventListener('click', async () => { + /* One scan-and-show, so the button and the brightness gate take the + * same path. The gate used to throw the reading away and put nothing + * in its place. */ + runScan = async () => { run.disabled = true; run.textContent = 'Scanning…'; try { @@ -1663,7 +1975,8 @@ export function mountEditor(root, { run.disabled = false; run.textContent = 'Scan the frame'; } - }); + }; + run.addEventListener('click', () => runScan()); } /* @@ -1755,6 +2068,36 @@ export function mountEditor(root, { truncated: !!diag.truncated, found: diag.defectCount }); } + /* + * A section that can be folded away, for the numbers under a verdict. + * + * Built on
, which brings the keyboard and the screen-reader + * behaviour with it -- a div with a click handler brings neither, and this + * is the first disclosure in the tree, so whatever it does becomes the + * pattern. Styled off .re-shead, which is already a heading-plus-rule row + * at ~15 call sites and is exactly the shape of a summary bar. + * + * Open state is not remembered between renders on purpose: the panel is + * rebuilt from scratch on every scan, and restoring a toggle would mean + * carrying UI state through a path that otherwise carries only readings. + */ + function foldout(title, build, open = false) { + const d = el('details'); + if (open) d.open = true; + d.style.cssText = 'margin-top:9px'; + const sum = el('summary', 're-shead'); + sum.style.cssText = 'cursor:pointer;list-style:none;margin-bottom:0'; + sum.append(Object.assign(el('h3', 're-cap'), { textContent: title })); + sum.append(el('span', 're-rule')); + sum.append(Object.assign(el('span', 're-note'), { textContent: '▾' })); + d.append(sum); + const body = el('div'); + body.style.cssText = 'margin-top:7px'; + build(body); + d.append(body); + return d; + } + function statLine(name, value, note) { const row = el('div'); row.style.cssText = 'display:flex;align-items:baseline;gap:8px;margin:3px 0'; @@ -1988,7 +2331,11 @@ export function mountEditor(root, { out.append(statLine('reading', 'below ' + Math.round(diag.backgroundCut), 'only where the picture is dark')); - out.append(deviationPlot(diag)); + /* The deviation field belongs to ONE frame -- it is every pixel of the + * frame that was scanned, against its neighbours. After a tally it + * rides along on the spread of `diag` and would sit under a headline + * about five captures describing only the last of them. */ + if (!diag.fromTally) out.append(deviationPlot(diag)); /* * Two captures of different views, intersected. @@ -2022,9 +2369,47 @@ export function mountEditor(root, { * fall is itself the answer -- a population that appears once each and * never again is the scene, and one that keeps coming back is not. */ - const held = scanSet.filter((v) => v.id !== state.openId); - const all = held.concat([{ name: state.name || 'this frame', - keys: diag.defects.map((p) => p.x + ',' + p.y) }]); + /* + * The captures themselves, never the answer fed back as one of them. + * + * The open frame's reading is substituted in here so that a scan which + * has not been kept still counts -- and that is right while `diag` IS + * this frame's own scan. It stops being right the moment the tally has + * replaced `diag.defects` with its own result, because the fifth + * capture then becomes the conclusion, offered as evidence for itself. + * + * Measured on a real run of five: the progress line said 388 sites had + * turned up every time and this table said 397, because + * `in-all-5 = #{c=5} + #{c=4 and absent from capture 5}` -- nine sites + * that had missed a capture, reported as having missed none. Sites + * seen ONLY in the last capture went the other way and disappeared + * from the table altogether, which is why its rows stopped summing to + * the count above it. + * + * So: if this frame has been kept, scanSet already holds its real + * reading and is the whole truth. Only stand in for it when it has not + * been kept and `diag` is a genuine single-frame scan. + */ + const kept = scanSet.some((v) => v.id === state.openId); + /* + * A tally-derived reading carries the frame's OWN keys with it, so + * marking does not change how many captures there were. + * + * The first version of this fix sent every tally straight to scanSet, + * which is right after a guided run -- keepCurrentScan has been called, + * so the frame is in there. In the manual flow it is not: the reading + * on screen stands in for it, and pressing Mark threw that stand-in + * away. The capture count fell by one, the votes and the suggested + * threshold moved with it, and no evidence had changed at all. + */ + const self = diag.fromTally && diag.fromTally.selfKeys; + const all = kept + ? scanSet.slice() + : scanSet.filter((v) => v.id !== state.openId) + .concat(self || !diag.fromTally + ? [{ name: state.name || 'this frame', + keys: self || diag.defects.map((p) => p.x + ',' + p.y) }] + : []); const M = all.length; if (M < 2) { out.append(Object.assign(el('p', 're-note'), { @@ -2150,8 +2535,21 @@ export function mountEditor(root, { * on screen -- and if it had to be in the current frame's list * too, the rule would be an intersection again. */ + /* + * `partial` comes along. It says a capture filled the scan's + * store, so the list is short near the bottom of the frame, + * and the exported file's header is written from it. Dropping + * it here turned that header from "NO" to "yes, as far as the + * scan could see" on a click that changed no evidence -- while + * the warning on screen, which reads a different flag, went on + * saying the opposite. + */ diag = { ...diag, defects: pts, defectCount: pts.length, truncated: false, - fromTally: { need: needN, of: M } }; + fromTally: { need: needN, of: M, + partial: all.some((v) => v.truncated), + /* This frame's own reading, kept so the table can go on + * counting it once the marks are the tally's. */ + selfKeys: kept ? null : diag.defects.map((p) => p.x + ',' + p.y) } }; renderDiagnose(out); drawMarks(); }); @@ -2181,34 +2579,76 @@ export function mountEditor(root, { out.append(Object.assign(el('div', 're-shead'), { innerHTML: '

Black level

', })); - out.append(statLine('file', String(i.black), 'what the DNG says')); - out.append(statLine('frame', rgb(diag.blackFloor, (v) => v.toFixed(0).padStart(5)), - 'the darkest the frame really gets, per plane')); const floor = Math.min(...diag.blackFloor); + /* + * The judgement first, and one an owner can act on. + * + * "The file claims a higher pedestal than the frame reaches" reads as + * a fault and offers no remedy -- and for most people it is not even a + * fault they own: it is the camera's own black level, in the camera's + * own file. Say what it costs the picture and who can change it. + */ + const low = floor + 2 < i.black; out.append(Object.assign(el('p', 're-note'), { - style: 'margin:5px 0 0', - textContent: floor + 2 < i.black - ? 'The file claims a higher pedestal than the frame reaches, which clips the ' + - 'shadows to black. Worth checking against a lens-cap frame.' - : 'Consistent with the file, as far as this frame can say — a scene with ' + - 'nothing truly dark in it cannot say much.', + style: 'margin:0 0 3px;color:#e6e8ee', + textContent: low + ? 'The camera subtracts more than this frame actually reaches, so the ' + + 'deepest shadows are being flattened to pure black. Nothing here can ' + + 'change that — it is set in the camera\u2019s own tuning — but it is worth ' + + 'knowing if shadow detail is what you are missing.' + : 'Normal. The camera\u2019s black level agrees with what the frame reaches.', + })); + out.append(foldout('The numbers', (b) => { + b.append(statLine('file', String(i.black), 'what the DNG says')); + b.append(statLine('frame', rgb(diag.blackFloor, (v) => v.toFixed(0).padStart(5)), + 'the darkest the frame really gets, per plane')); + if (!low) b.append(Object.assign(el('p', 're-note'), { + style: 'margin:5px 0 0', + textContent: 'A scene with nothing truly dark in it cannot say much; a frame ' + + 'with the lens covered says the most.', + })); })); out.append(Object.assign(el('div', 're-shead'), { innerHTML: '

Clipping

', })); - out.append(statLine('R G B', rgb(diag.clipped, (v) => pct(v).padStart(7)), - 'at or above ' + i.white)); + const blown = Math.max(...diag.clipped); + out.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 3px;color:#e6e8ee', + textContent: blown >= 0.01 + ? 'Something in this frame is burnt out — ' + pct(blown).trim() + ' of one ' + + 'colour has gone as bright as the sensor goes, and no detail survives ' + + 'there. A shorter exposure keeps it.' + : 'Normal. Nothing worth speaking of has burnt out.', + })); + out.append(foldout('The numbers', (b) => { + b.append(statLine('R G B', rgb(diag.clipped, (v) => pct(v).padStart(7)), + 'at or above ' + i.white)); + })); out.append(Object.assign(el('div', 're-shead'), { innerHTML: '

Noise

', })); - out.append(statLine('R G B', rgb(diag.noise, (v) => v.toFixed(1).padStart(6)), - 'counts, one sigma')); + /* Against the frame's own signal range, which is the only thing here + * that makes a count of noise mean anything to a reader. */ + const sigma = Math.max(...diag.noise); + const range = (i.white || 0) - (i.black || 0); + const noisy = range > 0 && sigma / range > 0.01; out.append(Object.assign(el('p', 're-note'), { - style: 'margin:5px 0 0', - textContent: 'A median of local differences, so an edge in the frame does not ' + - 'read as noise' + (i.iso ? `. This frame is ISO ${i.iso}.` : '.'), + style: 'margin:0 0 3px;color:#e6e8ee', + textContent: noisy + ? 'Grainy, which is what a small sensor does when it is short of light. ' + + 'More light, or a longer exposure, is the only cure.' + : 'Normal for this exposure.', + })); + out.append(foldout('The numbers', (b) => { + b.append(statLine('R G B', rgb(diag.noise, (v) => v.toFixed(1).padStart(6)), + 'counts, one sigma')); + b.append(Object.assign(el('p', 're-note'), { + style: 'margin:5px 0 0', + textContent: 'A median of local differences, so an edge in the frame does not ' + + 'read as noise' + (i.iso ? `. This frame is ISO ${i.iso}.` : '.'), + })); })); } @@ -4598,6 +5038,14 @@ export function mountEditor(root, { // dimensions, so not even where they were measured -- and shaded // against a peak held from a lens position that no longer exists. resetFocusState(); + /* + * A frame arriving is news to the Bad pixels rail. Nothing rebuilt + * it here before, which was harmless while every control worked + * without a frame -- but the guided run's button now waits for one + * and would have stayed disabled after the frame it was waiting + * for had landed. Mode-guarded, so the other tabs are untouched. + */ + if (mode === 'diagnose') buildDiagnose(); saveBtn.disabled = false; nameEl.textContent = label; sensorChip.hidden = false; @@ -4704,6 +5152,19 @@ export function mountEditor(root, { sweepClosed = true; filterGen++; abandonAll('the editor was closed'); + /* + * The correction hold is a timer of its own, and it outlived the + * editor: close the page inside the hold and it still fired, still + * called revert(), and changed a camera whose operator had gone. + * A change nobody is watching is put back NOW rather than on a + * schedule -- the whole point of the hold is that someone is there + * to keep it. + */ + if (dpcHold) { + clearTimeout(dpcHold); + dpcHold = null; + try { sensor?.revert(); } catch (e) { /* the page is going */ } + } worker?.terminate(); worker = null; if (styles) releaseStylesheet(); diff --git a/dist/iqprofile.js b/dist/iqprofile.js index 46f786d..cb254f0 100644 --- a/dist/iqprofile.js +++ b/dist/iqprofile.js @@ -49,6 +49,42 @@ const numbers = (v) => { * rather than half-read, because a profile is built on top of it and written * to a camera: a zero curve is a real (and broken) curve, and a table set * that is short a temperature would be blended at an undefined one. */ +/* + * What the camera does about bad pixels, out of its own profile. + * + * The section is named for the profile in force -- a camera running its night + * profile calls it [ir_static_dpc] and a day one [static_dpc] -- so the name + * is found rather than assumed, and handed back so a patch can be written to + * the same place it was read from. + * + * DpcStrength is one value per gain step, which is why the answer depends on + * which step the frame was taken at; without a way to map ISO onto that index + * the honest summary is the range, not a single number. + */ +export function readDefectCorrection(ini) { + const name = Object.keys(ini || {}).find((k) => /(^|_)static_dpc$/.test(k)); + if (!name) return null; + const sec = ini[name]; + const nums = (v) => String(v || '').split(',') + .map((x) => Number(x.trim())).filter((x) => Number.isFinite(x)); + const strength = nums(sec.DpcStrength); + return { + section: name, + enabled: String(sec.DpcEnable).trim() === '1', + strength, + /* 0-255 in the vendor's units; reported as a share so the copy can + * speak of it without naming a scale nobody outside the SDK knows. */ + strongest: strength.length ? Math.max(...strength) / 255 : null, + weakest: strength.length ? Math.min(...strength) / 255 : null, + }; +} + +/* The smallest patch that turns the corrector on, written back into the + * section it was read from so a night profile is not given a day one's key. */ +export function enableDefectCorrection(state) { + return '[' + state.section + ']\nDpcEnable = "1"\n'; +} + export function readColour(ini) { const awb = ini.static_awb || {}, ccm = ini.static_ccm || {}; const staticWb = numbers(awb.AutoStaticWb); diff --git a/src/editor.css b/src/editor.css index 4abc424..bf63402 100644 --- a/src/editor.css +++ b/src/editor.css @@ -8,7 +8,18 @@ * * Values are the WebUI dark theme's own, lifted rather than re-picked: * page #14161c · panel #1c1f27 · raised #232734 · border #2c313d - * body #e6e8ee · secondary #b6b9c2 · tertiary #878a94 · accent #5c70e8 + * body #e6e8ee · secondary #b6b9c2 · tertiary #a2a6b2 · accent #5c70e8 + * + * The tertiary used to be #878a94 -- 4.32:1 against the raised #232734 that + * panels are drawn in, under the 4.5:1 minimum, on the class carrying every + * piece of explanatory prose in the editor. It passed on the #1c1f27 rail at + * 4.78:1, which is presumably how it survived. + * + * #a2a6b2 replaces it: 6.12:1 on #232734, 6.77:1 on #1c1f27. Measured with the + * WCAG formula rather than by eye, checked first against known pairs -- white + * on black 21.00, #767676 on white 4.54. (The wrong colour was named here at + * first: a blanket replace of the old value rewrote the sentence describing + * it, leaving a comment that contradicted itself.) * stage #0d0f14 */ .re-root, .re-root * { box-sizing: border-box; } @@ -24,10 +35,10 @@ } .re-mono { font-family: 'PT Mono', ui-monospace, Menlo, Consolas, monospace; font-variant-numeric: tabular-nums; } -.re-cap { color: #878a94; font-size: 11px; font-weight: 600; letter-spacing: .07em; +.re-cap { color: #a2a6b2; font-size: 11px; font-weight: 600; letter-spacing: .07em; text-transform: uppercase; margin: 0; } .re-rule { flex: 1 1 auto; height: 1px; background: #2c313d; min-width: 8px; } -.re-note { color: #878a94; font-size: 11px; line-height: 1.45; } +.re-note { color: #a2a6b2; font-size: 11px; line-height: 1.45; } .re-top { flex: 0 0 52px; display: flex; align-items: center; gap: 12px; padding: 0 14px; background: #1c1f27; border-bottom: 1px solid #2c313d; } @@ -41,9 +52,13 @@ .re-top { gap: 8px; padding: 0 9px; } .re-top > .re-chip { display: none; } .re-btn > span { display: none; } - .re-btn { padding: 6px 8px; } + /* NOT less padding. This used to shrink to 6px 8px, which took the + buttons further from a finger-sized target exactly on the devices most + likely to be touched. */ + .re-btn { padding: 9px 11px; } } + .re-mid { flex: 1 1 auto; display: flex; min-height: 0; } .re-stage { flex: 1 1 auto; position: relative; display: flex; align-items: center; justify-content: center; background: #0d0f14; min-width: 0; overflow: auto; } @@ -88,10 +103,15 @@ .re-insp { flex: 0 0 328px; display: flex; flex-direction: column; gap: 9px; padding: 12px; background: #1c1f27; border-left: 1px solid #2c313d; overflow-y: auto; } .re-foot { flex: 0 0 30px; display: flex; align-items: center; gap: 16px; padding: 0 14px; - background: #1c1f27; border-top: 1px solid #2c313d; color: #878a94; font-size: 11px; } + background: #1c1f27; border-top: 1px solid #2c313d; color: #a2a6b2; font-size: 11px; } .re-panel { background: #232734; border: 1px solid #2c313d; border-radius: 12px; padding: 9px; } .re-shead { display: flex; align-items: center; gap: 9px; margin-bottom: 7px; } +/* The disclosure marker is drawn by the summary row itself, so the browser's + own triangle is taken away on both engines that draw one. */ +.re-root summary::-webkit-details-marker { display: none; } +.re-root summary { list-style: none; } +.re-root details[open] > summary .re-note { transform: rotate(180deg); } .re-seg { display: flex; gap: 2px; padding: 2px; background: #232734; border: 1px solid #2c313d; border-radius: 8px; width: fit-content; } @@ -128,13 +148,13 @@ * a bar filling from there, so how far you have moved is readable without * doing arithmetic. */ .re-row { display: flex; align-items: center; gap: 10px; height: 25px; } -.re-rname { flex: 0 0 70px; color: #878a94; font-size: 11px; font-weight: 600; +.re-rname { flex: 0 0 70px; color: #a2a6b2; font-size: 11px; font-weight: 600; letter-spacing: .07em; text-transform: uppercase; } .re-track { position: relative; flex: 1 1 auto; height: 25px; } .re-tbg, .re-fill { position: absolute; top: 10px; height: 4px; border-radius: 2px; } .re-tbg { left: 0; right: 0; background: rgba(230,232,238,.10); } .re-fill { background: #5c70e8; } -.re-tick { position: absolute; top: 6px; width: 1px; height: 12px; background: #878a94; } +.re-tick { position: absolute; top: 6px; width: 1px; height: 12px; background: #a2a6b2; } .re-track input { position: relative; z-index: 2; appearance: none; width: 100%; height: 25px; margin: 0; padding: 0; background: transparent; border: 0; cursor: ew-resize; } .re-track input::-webkit-slider-thumb { appearance: none; width: 14px; height: 14px; @@ -143,7 +163,7 @@ background: #e6e8ee; box-shadow: 0 1px 3px rgba(0,0,0,.45); } .re-rnum { flex: 0 0 46px; color: #b6b9c2; font-size: 12px; text-align: right; font-family: 'PT Mono', ui-monospace, monospace; font-variant-numeric: tabular-nums; } -.re-rst { flex: 0 0 16px; border: 0; background: none; padding: 0; color: #878a94; +.re-rst { flex: 0 0 16px; border: 0; background: none; padding: 0; color: #a2a6b2; cursor: pointer; line-height: 0; } .re-rst:hover { color: #5c70e8; } .re-rst:disabled { opacity: .18; cursor: default; } @@ -182,3 +202,41 @@ /* The deviation histogram. currentColor so the bars and rules follow the * panel's text colour rather than needing their own palette. */ .re-devplot { display: block; width: 100%; height: auto; color: #b9bec9; } + +/* + * A phone. + * + * The rail is a fixed 328px column that never shrank, so at 400px the picture + * was left about 71px of a 400px screen -- a strip, beside a panel. And the + * tab bar is nowrap inside a row whose children may not shrink, with no + * overflow anywhere, so a fifth tab and the Capture and Download buttons + * simply left the screen with no way to reach them. + * + * So: stack, and let the bar scroll. DOM order is already picture-then-rail, + * so nothing has to move. + */ +@media (max-width: 560px) { + .re-mid { flex-direction: column; } + .re-stage { + flex: 0 0 auto; + /* Something to look at, and a width for fitStep() to divide by: the + zoom step is worked out from stage.clientWidth, and a stage flexed + to nothing yields a degenerate step. */ + min-height: 42vh; + } + .re-insp { + flex: 1 1 auto; width: 100%; + border-left: 0; border-top: 1px solid #2c313d; + } + /* The bar gets somewhere to go rather than off the edge. */ + .re-top { overflow-x: auto; scrollbar-width: none; } + .re-top::-webkit-scrollbar { display: none; } + /* A finger, not a mouse: 44px is the guideline and the main tab bar was + 25. 12px text is 18px on a 1.5 line, so 13px of padding makes 44 with no + border and 12px makes 44 with one. */ + .re-seg button, .re-seg.re-tight button { padding: 13px; font-size: 12px; } + .re-btn, .re-btn.re-sm { padding: 12px 13px; font-size: 12px; } + .re-rst { flex-basis: 44px; height: 44px; } + .re-row { height: 44px; } + summary { padding: 8px 0; } +} diff --git a/src/editor.js b/src/editor.js index 3efdd1a..e6b3258 100644 --- a/src/editor.js +++ b/src/editor.js @@ -13,7 +13,8 @@ */ import { solveFromPatches, patchCentres, scoreCcm, CHART_COLS, CHART_ROWS } from './calibrate.js'; -import { parseIni, readColour, fitAwbCurve, gainsForCt, mergeCcmTables, colourFragment } from './iqprofile.js'; +import { parseIni, readColour, fitAwbCurve, gainsForCt, mergeCcmTables, colourFragment, + readDefectCorrection, enableDefectCorrection } from './iqprofile.js'; import { summarise, peakHold, normalise } from './aftune.js'; const CFA_NAMES = ['RGGB', 'GRBG', 'GBRG', 'BGGR']; @@ -50,6 +51,12 @@ const svg = (d, w = 20) => const ICON = { back: svg('', 18), reset: svg('', 13), + /* A tick, so a good answer does not arrive wearing a warning triangle. + * re-ok and re-warn were told apart only by an icon colour and a 60%-alpha + * border, and most call sites handed both variants the same triangle. */ + ok: '' + + '', warn: '' + '', @@ -249,6 +256,22 @@ export function mountEditor(root, { * sections given into the camera's profile. revert() and keep() then * answer for whichever was written last. */ calibrate, + /* + * What the camera does about its own bad pixels, and how to change it: + * { profile(), patch(ini), revert(), keep(), holdSeconds }. + * + * profile() resolves to the camera's IQ profile as text; patch() writes + * sections into it; revert() and keep() answer for the last patch, the + * same bargain calibrate strikes. Without one the defect scan still + * measures and still gives its verdict -- it simply cannot say what the + * camera is doing about what it found, and says that instead of guessing. + * + * Worth knowing before wiring one up: a coordinate list cannot be loaded + * into these parts. The static defect table is refused by the chip on a + * gk7205v300 and the profile carries none, so what is on offer is the + * camera's own automatic corrector, not this scan's findings. + */ + sensor, /* Live focus statistics: { zones(), intervalMs }. zones() resolves to the * camera's AF grid -- { rows, cols, zones: [[h1,h2,v1,v2,y,hlcnt], ...] }, * row-major. Without one there is no Focus tab at all, because a focus @@ -374,7 +397,12 @@ export function mountEditor(root, { sensorChip.hidden = true; const modeItems = [ { label: 'Develop', value: 'develop' }, - { label: 'Diagnose', value: 'diagnose' }, + /* Named for what it finds, not for what it does to find it. "Diagnose" + * is a verb that never says what is being diagnosed, and an owner + * looking for stuck pixels had no reason to guess it was in here. The + * mode key stays `diagnose` -- it is internal, and renaming it would + * be churn with nothing visible at the end of it. */ + { label: 'Bad pixels', value: 'diagnose' }, { label: 'Calibrate', value: 'calibrate' }, ]; // Not created and disabled: a tab that can never work is worse than no tab, @@ -1090,10 +1118,10 @@ export function mountEditor(root, { * and how much blur it survives before it stops reading. * * It lives on the Plates tab, under the list the plate is chosen from. It - * used to sit in Diagnose, which is about the sensor's own health and not + * used to sit in Bad pixels, which is about the sensor's own health and not * about the picture -- and every one of these questions is about one * plate, so the answer differed across a frame while the card sat on a tab - * with nothing to pick a plate on. Diagnose grew a guided run afterwards + * with nothing to pick a plate on. That tab grew a guided run afterwards * and this card landed in the middle of it, between the steps and their * result, which is what finally made the misfiling obvious. * @@ -1250,11 +1278,215 @@ export function mountEditor(root, { */ const HUNT_WANT = 5; + /* Assigned by buildDiagnose, which owns the button and the panel it writes + * into; the gate calls it from outside that closure. */ + let runScan = async () => {}; + + /* + * How many stuck pixels is too many. + * + * There is no standard answer -- EMVA 1288 declines to define a defective + * pixel at all, on the grounds that no one definition serves every + * application. So this uses the only published acceptance figure the tree + * already cites: Sony sell an IMX415 as good with up to 800 white pixels + * in the dark, and that part is 3864x2192, so the limit they are willing + * to ship is 9.4e-5 of it. Rounded to a hundredth of a percent, and + * applied as a FRACTION so it carries across sensors of other sizes. + * + * It is a weak yardstick and the copy says so rather than pretending to a + * pass mark. It is also strongly conditional: the same lab camera reports + * 882 sites at 0.5 s and 3921 at 7 s, which is dark current doing exactly + * what it should, so a verdict that ignored exposure would call one + * healthy sensor both fine and faulty within a minute. The exposure and + * the gain go in the sentence for that reason. + */ + const NORMAL_FRACTION = 1e-4; + + /* The camera's own answer, read once per run and remembered: it is a + * fetch, and the panel rebuilds on every step. */ + let dpc = null, dpcAsked = false, dpcHold = null; + + async function readDpc() { + if (dpcAsked || !sensor || !sensor.profile) return; + dpcAsked = true; + try { + /* null means the profile named no such section -- an answer, and a + * different one from "still asking". Left as null it read as a + * request in flight, and since dpcAsked blocks a retry the card + * said "Asking the camera..." for as long as the editor was open. */ + dpc = readDefectCorrection(parseIni(await sensor.profile())) + || { absent: true }; + } catch (e) { + dpc = { failed: e && e.message ? e.message : 'could not be read' }; + } + if (mode === 'diagnose') buildDiagnose(); + } + + /* + * What the camera is doing about them, under the verdict. + * + * This is the half of the owner's question the scan could never answer on + * its own -- a raw capture is taken ahead of the camera's processing, so + * the defects are in it whether or not the camera hides them downstream. + * Only the camera can say, so it is asked. + */ + function dpcCard(panel) { + if (!sensor || !sensor.profile) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: 'Cameras of this kind correct stuck pixels themselves as they ' + + 'run, and a raw capture is taken before that happens. Opened from the ' + + 'camera\u2019s own page, this would say whether yours has it switched on.', + })); + return; + } + if (!dpc) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', textContent: 'Asking the camera\u2026' })); + return; + } + if (dpc.absent) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: 'This camera\u2019s profile does not say whether it corrects ' + + 'stuck pixels, so there is nothing here to read or change.', + })); + return; + } + if (dpc.failed) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: 'The camera did not say whether it corrects these: ' + dpc.failed })); + return; + } + const box = el('div', 're-notice ' + (dpc.enabled ? 're-ok' : 're-warn'), + dpc.enabled ? ICON.ok : ICON.warn); + box.style.cssText = 'margin:0 0 9px'; + box.dataset.act = 'dpc-state'; + box.append(Object.assign(el('div'), { + /* + * What the camera REPORTS, not what was measured of the result. + * + * Proving the corrector actually removes these from the video + * needs a full-resolution snapshot taken beside the raw frame, and + * the lab camera cannot produce one: at 2592x1944 the JPEG encoder + * has no media memory left beside the H.265 stream, whichever way + * isp.blkCnt is moved -- one block short at 4, none to spare at 5 + * and above. So this says the switch is on and what that is for, + * and stops there. + */ + textContent: dpc.enabled + ? 'Your camera\u2019s own correction for these is switched on, so they should ' + + 'not reach the video it sends. This capture is read from the sensor ' + + 'ahead of that, on purpose — it is the only way to see what the sensor ' + + 'is really doing.' + : 'Your camera can hide these in the video it sends, and that is switched ' + + 'off. Turning it on costs nothing else in the picture.', + })); + panel.append(box); + if (dpc.enabled || !sensor.patch) return; + + const on = el('button', 're-btn re-pri', ''); + on.dataset.act = 'dpc-on'; + on.textContent = dpcHold ? 'Keep it on' : 'Turn it on'; + on.addEventListener('click', async () => { + on.disabled = true; + try { + if (dpcHold) { + /* Stop the clock FIRST. Setting the handle to null does not + * cancel the timer it names, so a kept change sat there + * until the hold expired and was then quietly put back -- + * while the card, rebuilt, went on saying it was on. */ + clearTimeout(dpcHold); + dpcHold = null; + await sensor.keep(); + dpc.enabled = true; + } + else { + await sensor.patch(enableDefectCorrection(dpc)); + /* Same hold-and-confirm as a calibration: written now, put + * back on its own unless someone says to keep it. */ + dpcHold = setTimeout(async () => { + dpcHold = null; + try { await sensor.revert(); } catch (e) { /* going anyway */ } + if (mode === 'diagnose') buildDiagnose(); + }, Math.max(5, sensor.holdSeconds || 30) * 1000); + } + } catch (e) { + dpc = { failed: e && e.message ? e.message : 'the write was refused' }; + } + if (mode === 'diagnose') buildDiagnose(); + }); + panel.append(on); + if (dpcHold) + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:7px 0 0', + textContent: 'Switched on. It goes back by itself in ' + + Math.max(5, sensor.holdSeconds || 30) + ' seconds unless you keep it.', + })); + } + + function sensorVerdict(pts, partial) { + const i = state.info || {}; + const total = (i.width || 0) * (i.height || 0); + if (!pts || !total) return null; + const n = pts.length; + const frac = n / total; + const share = frac >= 1e-4 ? (frac * 100).toFixed(3) + '%' + : (frac * 100).toFixed(4) + '%'; + const sp = spreadOf(pts, i.width, i.height); + const taken = (i.exposure ? 'Measured at ' + (i.exposure * 1000).toFixed(0) + ' ms' + : 'Measured') + (i.iso ? ' and ISO ' + i.iso : '') + + '. Dark current builds with time and gain, so a longer exposure or ' + + 'more of either finds more of them.'; + /* What a raw capture is: the sensor before the camera has done + * anything to the picture. True by construction -- it is the point of + * the format -- and it stops short of claiming the camera's own + * corrector actually hides these, which is a separate measurement. */ + const raw = ' This is read from the sensor ahead of the camera\u2019s own ' + + 'processing, so it is not what the video shows.'; + + if (n === 0) + return ['re-ok', 'Nothing came back in every capture. On a healthy sensor ' + + 'that is the right answer.' + ' ' + taken]; + if (sp && sp.index < 0.8) + return ['re-warn', `${n} sites kept coming back, but they are clumped rather ` + + 'than scattered — which is what detail in a picture looks like, and not ' + + 'what a sensor looks like. Cover the lens, or move the camera between ' + + 'captures, and run it again.']; + /* + * A short list cannot be called a normal one. + * + * When a capture filled the scan's store it stopped part-way down the + * frame, so the count is low by an unknown amount in a known place -- + * and a fraction computed from it is low too. "Normal" would then be + * read off the very evidence that went missing. + */ + if (partial) + return ['re-warn', `${n} stuck pixels confirmed — ${share} of this sensor, and ` + + 'scattered at random, so they are the sensor rather than the scene. How ' + + 'many there are in total is not known: at least one of these captures found ' + + 'more than the scan can hold and stopped part-way down the frame. ' + taken]; + if (frac <= NORMAL_FRACTION) + return ['re-ok', `${n} stuck pixels — ${share} of this sensor, scattered at ` + + 'random, which is what a sensor\u2019s own faults look like. That is a ' + + 'normal number: parts of this class are sold as good with up to about a ' + + 'hundredth of a percent. ' + taken + raw]; + return ['re-warn', `${n} stuck pixels — ${share} of this sensor. They are ` + + 'scattered at random, so they are the sensor rather than the scene, but it ' + + 'is more than parts of this class are usually sold with (about a hundredth ' + + 'of a percent). ' + taken + raw]; + } + function startHunt() { // A run starts from nothing kept, or the first tally would count // captures the operator has forgotten about. scanSet = []; needTouched = false; + /* Ask the camera again. Its correction can be changed from elsewhere + * between runs, and a second run reporting the first run's reading is + * worse than not reporting one. */ + if (!dpcHold) { dpc = null; dpcAsked = false; } /* And from looking everywhere, whatever the last run or the operator * left the gate on. The first capture decides whether to narrow it, * and narrowing a frame that is dark all over destroys the answer @@ -1278,10 +1510,22 @@ export function mountEditor(root, { buildDiagnose(); try { if (capture) { + /* + * Two ways a capture fails to land, and they need different + * sentences. A timeout or a throw puts its own reason on the + * stage, so pointing at it is fair. But takeFrame also returns + * in silence when the editor is already busy -- fetching the + * first frame, most often -- and that path posts nothing, so + * "the message above says why" pointed at blank space. + */ + if (state.busy) + throw new Error('The editor is still busy with the last frame. ' + + 'Give it a moment and press this again.'); const was = opened; await takeFrame(); if (opened === was) - throw new Error('That capture did not arrive. The message above says why.'); + throw new Error('That capture did not arrive — the reason is on the ' + + 'picture, above.'); } else if (!state.info) { throw new Error('Drop a frame on the picture first.'); } else if (hunt.seen === opened) { @@ -1448,14 +1692,35 @@ export function mountEditor(root, { for (const [, c] of tally) if (c === hunt.taken) every++; panel.append(Object.assign(el('p', 're-note'), { style: 'margin:0 0 7px', - textContent: 'So far: ' + tally.size + ' site' + (tally.size === 1 ? '' : 's') + + textContent: (done ? 'Across ' + hunt.taken + ' pictures: ' : 'So far: ') + + tally.size + ' site' + (tally.size === 1 ? '' : 's') + ', ' + every + ' of which ' + (every === 1 ? 'has' : 'have') + ' turned up every time.', })); panel.lastChild.dataset.act = 'hunt-progress'; - if (hunt.dark !== null) { - const box = el('div', 're-notice ' + (hunt.dark ? 're-ok' : 're-warn')); + /* + * Said once, when it is decided, and then kept as a single line. + * + * All of this -- the covered/not-covered finding and the paragraph + * about exposure below it -- used to be reprinted unchanged after + * every capture, so the panel was nine lines of advice the reader + * had already taken, three captures ago, with the one number that + * had changed buried in it. + */ + const fresh = hunt.taken === 1 || done; + if (hunt.dark !== null && !fresh) { + panel.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 9px', + textContent: hunt.dark + ? 'Lens covered — leave the camera where it is.' + : 'Looking only where the picture is dark — move the camera a ' + + 'little between pictures.', + })); + } + if (hunt.dark !== null && fresh) { + const box = el('div', 're-notice ' + (hunt.dark ? 're-ok' : 're-warn'), + hunt.dark ? ICON.ok : ICON.warn); box.style.cssText = 'margin:0 0 9px'; box.dataset.act = 'hunt-verdict'; box.append(Object.assign(el('div'), { @@ -1482,7 +1747,7 @@ export function mountEditor(root, { * 3921 at 7 s. Which is also the warning: a map taken at seven * seconds is not a map of the camera that runs at a thirtieth. */ - if (state.info && state.info.exposure) + if (fresh && !done && state.info && state.info.exposure) panel.append(Object.assign(el('p', 're-note'), { style: 'margin:0 0 9px', textContent: 'Taken at ' + (state.info.exposure * 1000).toFixed(1) + @@ -1508,11 +1773,17 @@ export function mountEditor(root, { if (!done) { const go = el('button', 're-btn re-pri', ''); go.dataset.act = 'hunt-step'; + /* "Take picture N of 5" rather than "Capture": there is a Capture + * button in the chrome already, and a tester's script clicked the + * wrong one of the two. */ + const waiting = state.busy || !state.info; go.textContent = hunt.busy ? 'Working…' - : capture ? (hunt.taken === 0 ? 'Capture' : 'Capture ' + (hunt.taken + 1) + - ' of ' + HUNT_WANT) - : 'Read the frame on screen'; - go.disabled = hunt.busy; + : waiting ? 'Waiting for a frame…' + : capture ? 'Take picture ' + (hunt.taken + 1) + ' of ' + HUNT_WANT + : 'Read the frame on screen'; + /* Live before the first frame has arrived, this produced a capture + * that could not run and an error about it. */ + go.disabled = hunt.busy || waiting; go.addEventListener('click', huntStep); row.append(go); /* No capture provider: the frames have to be brought in by hand, @@ -1530,6 +1801,24 @@ export function mountEditor(root, { fin.addEventListener('click', finishHunt); row.append(fin); } else { + /* + * The answer, first and in one sentence. + * + * A run used to end on a download button and a count, which left + * the owner holding a text file and no idea whether the number in + * it was bad news. Everything under this is the working. + */ + const v = sensorVerdict(diag && diag.fromTally ? diag.defects : null, + !!hunt.clipped); + if (v) { + const box = el('div', 're-notice ' + v[0], v[0] === 're-ok' ? ICON.ok : ICON.warn); + box.style.cssText = 'margin:0 0 9px'; + box.dataset.act = 'hunt-verdict-final'; + box.append(Object.assign(el('div'), { textContent: v[1] })); + panel.append(box); + } + readDpc(); + dpcCard(panel); if (hunt.clipped) { const box = el('div', 're-notice re-warn'); box.style.cssText = 'margin:0 0 9px'; @@ -1563,16 +1852,25 @@ export function mountEditor(root, { function buildDiagnose() { insp.replaceChildren(); insp.append(buildHunt()); + /* + * One frame, by hand -- folded away under the guided run. + * + * These were side by side, which put two five-capture methods and two + * buttons labelled Capture on one tab; a tester's script pressed the + * wrong one of the two on its first attempt. The guided run is the way + * in, and this is the same machinery for someone who already knows + * what they want from it. + */ const panel = el('div', 're-panel'); - panel.append(Object.assign(el('div', 're-shead'), { - innerHTML: '

Sensor

', - })); + /* Everything the manual path needs goes in here, and the whole of it + * is folded away at the bottom of the tab. */ + const body = el('div'); const run = el('button', 're-btn re-pri', ''); run.dataset.act = 'scan'; run.textContent = 'Scan the frame'; - panel.append(run); - panel.append(Object.assign(el('p', 're-note'), { - textContent: 'Read off the mosaic, before any interpolation. A frame of ' + + body.append(run); + body.append(Object.assign(el('p', 're-note'), { + textContent: 'Reads the mosaic exactly as the sensor sent it. A frame of ' + 'something flat and out of focus gives the cleanest answer — and a ' + 'dark one gives the only reliable one.', style: 'margin:8px 0 0', @@ -1600,14 +1898,24 @@ export function mountEditor(root, { ]) { const b = el('button', pct === bgPercent ? 'on' : '', label); b.title = note; - b.addEventListener('click', () => { + /* + * Mid-run this would leave scanSet holding captures read under + * different gates, which the tally then counts as if they were + * comparable -- a site can only turn up in the captures whose gate + * admitted it. The run owns the gate while it is open. + */ + b.disabled = !!hunt && !hunt.done; + if (b.disabled) b.title = 'The run sets this while it is going.'; + b.addEventListener('click', async () => { if (bgPercent === pct) return; bgPercent = pct; - // The old reading was taken somewhere else and no longer - // describes what is being asked for. diag = null; buildDiagnose(); drawMarks(); + /* Re-read rather than leaving an empty panel where a reading + * was. Choosing where to look is a question about this frame, + * and it used to be answered by removing the answer. */ + if (state.info) await runScan(); }); gateSeg.append(b); } @@ -1635,19 +1943,23 @@ export function mountEditor(root, { 'sensor from a scene, and there is no scene here. Everywhere is the ' + 'right answer for a frame like this one.', })); - panel.append(gate); + body.append(gate); + panel.append(foldout('Advanced — one frame at a time', (b) => b.append(body))); insp.append(panel); const out = el('div', 're-panel'); out.hidden = true; insp.append(out); - // A scan that is still valid is still worth showing. Leaving Diagnose + // A scan that is still valid is still worth showing. Leaving the tab // and coming back rebuilt an empty panel over a reading that had not // gone anywhere -- and the marks stayed on the picture, so there were // rings with nothing to explain them. if (diag) renderDiagnose(out); - run.addEventListener('click', async () => { + /* One scan-and-show, so the button and the brightness gate take the + * same path. The gate used to throw the reading away and put nothing + * in its place. */ + runScan = async () => { run.disabled = true; run.textContent = 'Scanning…'; try { @@ -1663,7 +1975,8 @@ export function mountEditor(root, { run.disabled = false; run.textContent = 'Scan the frame'; } - }); + }; + run.addEventListener('click', () => runScan()); } /* @@ -1755,6 +2068,36 @@ export function mountEditor(root, { truncated: !!diag.truncated, found: diag.defectCount }); } + /* + * A section that can be folded away, for the numbers under a verdict. + * + * Built on
, which brings the keyboard and the screen-reader + * behaviour with it -- a div with a click handler brings neither, and this + * is the first disclosure in the tree, so whatever it does becomes the + * pattern. Styled off .re-shead, which is already a heading-plus-rule row + * at ~15 call sites and is exactly the shape of a summary bar. + * + * Open state is not remembered between renders on purpose: the panel is + * rebuilt from scratch on every scan, and restoring a toggle would mean + * carrying UI state through a path that otherwise carries only readings. + */ + function foldout(title, build, open = false) { + const d = el('details'); + if (open) d.open = true; + d.style.cssText = 'margin-top:9px'; + const sum = el('summary', 're-shead'); + sum.style.cssText = 'cursor:pointer;list-style:none;margin-bottom:0'; + sum.append(Object.assign(el('h3', 're-cap'), { textContent: title })); + sum.append(el('span', 're-rule')); + sum.append(Object.assign(el('span', 're-note'), { textContent: '▾' })); + d.append(sum); + const body = el('div'); + body.style.cssText = 'margin-top:7px'; + build(body); + d.append(body); + return d; + } + function statLine(name, value, note) { const row = el('div'); row.style.cssText = 'display:flex;align-items:baseline;gap:8px;margin:3px 0'; @@ -1988,7 +2331,11 @@ export function mountEditor(root, { out.append(statLine('reading', 'below ' + Math.round(diag.backgroundCut), 'only where the picture is dark')); - out.append(deviationPlot(diag)); + /* The deviation field belongs to ONE frame -- it is every pixel of the + * frame that was scanned, against its neighbours. After a tally it + * rides along on the spread of `diag` and would sit under a headline + * about five captures describing only the last of them. */ + if (!diag.fromTally) out.append(deviationPlot(diag)); /* * Two captures of different views, intersected. @@ -2022,9 +2369,47 @@ export function mountEditor(root, { * fall is itself the answer -- a population that appears once each and * never again is the scene, and one that keeps coming back is not. */ - const held = scanSet.filter((v) => v.id !== state.openId); - const all = held.concat([{ name: state.name || 'this frame', - keys: diag.defects.map((p) => p.x + ',' + p.y) }]); + /* + * The captures themselves, never the answer fed back as one of them. + * + * The open frame's reading is substituted in here so that a scan which + * has not been kept still counts -- and that is right while `diag` IS + * this frame's own scan. It stops being right the moment the tally has + * replaced `diag.defects` with its own result, because the fifth + * capture then becomes the conclusion, offered as evidence for itself. + * + * Measured on a real run of five: the progress line said 388 sites had + * turned up every time and this table said 397, because + * `in-all-5 = #{c=5} + #{c=4 and absent from capture 5}` -- nine sites + * that had missed a capture, reported as having missed none. Sites + * seen ONLY in the last capture went the other way and disappeared + * from the table altogether, which is why its rows stopped summing to + * the count above it. + * + * So: if this frame has been kept, scanSet already holds its real + * reading and is the whole truth. Only stand in for it when it has not + * been kept and `diag` is a genuine single-frame scan. + */ + const kept = scanSet.some((v) => v.id === state.openId); + /* + * A tally-derived reading carries the frame's OWN keys with it, so + * marking does not change how many captures there were. + * + * The first version of this fix sent every tally straight to scanSet, + * which is right after a guided run -- keepCurrentScan has been called, + * so the frame is in there. In the manual flow it is not: the reading + * on screen stands in for it, and pressing Mark threw that stand-in + * away. The capture count fell by one, the votes and the suggested + * threshold moved with it, and no evidence had changed at all. + */ + const self = diag.fromTally && diag.fromTally.selfKeys; + const all = kept + ? scanSet.slice() + : scanSet.filter((v) => v.id !== state.openId) + .concat(self || !diag.fromTally + ? [{ name: state.name || 'this frame', + keys: self || diag.defects.map((p) => p.x + ',' + p.y) }] + : []); const M = all.length; if (M < 2) { out.append(Object.assign(el('p', 're-note'), { @@ -2150,8 +2535,21 @@ export function mountEditor(root, { * on screen -- and if it had to be in the current frame's list * too, the rule would be an intersection again. */ + /* + * `partial` comes along. It says a capture filled the scan's + * store, so the list is short near the bottom of the frame, + * and the exported file's header is written from it. Dropping + * it here turned that header from "NO" to "yes, as far as the + * scan could see" on a click that changed no evidence -- while + * the warning on screen, which reads a different flag, went on + * saying the opposite. + */ diag = { ...diag, defects: pts, defectCount: pts.length, truncated: false, - fromTally: { need: needN, of: M } }; + fromTally: { need: needN, of: M, + partial: all.some((v) => v.truncated), + /* This frame's own reading, kept so the table can go on + * counting it once the marks are the tally's. */ + selfKeys: kept ? null : diag.defects.map((p) => p.x + ',' + p.y) } }; renderDiagnose(out); drawMarks(); }); @@ -2181,34 +2579,76 @@ export function mountEditor(root, { out.append(Object.assign(el('div', 're-shead'), { innerHTML: '

Black level

', })); - out.append(statLine('file', String(i.black), 'what the DNG says')); - out.append(statLine('frame', rgb(diag.blackFloor, (v) => v.toFixed(0).padStart(5)), - 'the darkest the frame really gets, per plane')); const floor = Math.min(...diag.blackFloor); + /* + * The judgement first, and one an owner can act on. + * + * "The file claims a higher pedestal than the frame reaches" reads as + * a fault and offers no remedy -- and for most people it is not even a + * fault they own: it is the camera's own black level, in the camera's + * own file. Say what it costs the picture and who can change it. + */ + const low = floor + 2 < i.black; out.append(Object.assign(el('p', 're-note'), { - style: 'margin:5px 0 0', - textContent: floor + 2 < i.black - ? 'The file claims a higher pedestal than the frame reaches, which clips the ' + - 'shadows to black. Worth checking against a lens-cap frame.' - : 'Consistent with the file, as far as this frame can say — a scene with ' + - 'nothing truly dark in it cannot say much.', + style: 'margin:0 0 3px;color:#e6e8ee', + textContent: low + ? 'The camera subtracts more than this frame actually reaches, so the ' + + 'deepest shadows are being flattened to pure black. Nothing here can ' + + 'change that — it is set in the camera\u2019s own tuning — but it is worth ' + + 'knowing if shadow detail is what you are missing.' + : 'Normal. The camera\u2019s black level agrees with what the frame reaches.', + })); + out.append(foldout('The numbers', (b) => { + b.append(statLine('file', String(i.black), 'what the DNG says')); + b.append(statLine('frame', rgb(diag.blackFloor, (v) => v.toFixed(0).padStart(5)), + 'the darkest the frame really gets, per plane')); + if (!low) b.append(Object.assign(el('p', 're-note'), { + style: 'margin:5px 0 0', + textContent: 'A scene with nothing truly dark in it cannot say much; a frame ' + + 'with the lens covered says the most.', + })); })); out.append(Object.assign(el('div', 're-shead'), { innerHTML: '

Clipping

', })); - out.append(statLine('R G B', rgb(diag.clipped, (v) => pct(v).padStart(7)), - 'at or above ' + i.white)); + const blown = Math.max(...diag.clipped); + out.append(Object.assign(el('p', 're-note'), { + style: 'margin:0 0 3px;color:#e6e8ee', + textContent: blown >= 0.01 + ? 'Something in this frame is burnt out — ' + pct(blown).trim() + ' of one ' + + 'colour has gone as bright as the sensor goes, and no detail survives ' + + 'there. A shorter exposure keeps it.' + : 'Normal. Nothing worth speaking of has burnt out.', + })); + out.append(foldout('The numbers', (b) => { + b.append(statLine('R G B', rgb(diag.clipped, (v) => pct(v).padStart(7)), + 'at or above ' + i.white)); + })); out.append(Object.assign(el('div', 're-shead'), { innerHTML: '

Noise

', })); - out.append(statLine('R G B', rgb(diag.noise, (v) => v.toFixed(1).padStart(6)), - 'counts, one sigma')); + /* Against the frame's own signal range, which is the only thing here + * that makes a count of noise mean anything to a reader. */ + const sigma = Math.max(...diag.noise); + const range = (i.white || 0) - (i.black || 0); + const noisy = range > 0 && sigma / range > 0.01; out.append(Object.assign(el('p', 're-note'), { - style: 'margin:5px 0 0', - textContent: 'A median of local differences, so an edge in the frame does not ' + - 'read as noise' + (i.iso ? `. This frame is ISO ${i.iso}.` : '.'), + style: 'margin:0 0 3px;color:#e6e8ee', + textContent: noisy + ? 'Grainy, which is what a small sensor does when it is short of light. ' + + 'More light, or a longer exposure, is the only cure.' + : 'Normal for this exposure.', + })); + out.append(foldout('The numbers', (b) => { + b.append(statLine('R G B', rgb(diag.noise, (v) => v.toFixed(1).padStart(6)), + 'counts, one sigma')); + b.append(Object.assign(el('p', 're-note'), { + style: 'margin:5px 0 0', + textContent: 'A median of local differences, so an edge in the frame does not ' + + 'read as noise' + (i.iso ? `. This frame is ISO ${i.iso}.` : '.'), + })); })); } @@ -4598,6 +5038,14 @@ export function mountEditor(root, { // dimensions, so not even where they were measured -- and shaded // against a peak held from a lens position that no longer exists. resetFocusState(); + /* + * A frame arriving is news to the Bad pixels rail. Nothing rebuilt + * it here before, which was harmless while every control worked + * without a frame -- but the guided run's button now waits for one + * and would have stayed disabled after the frame it was waiting + * for had landed. Mode-guarded, so the other tabs are untouched. + */ + if (mode === 'diagnose') buildDiagnose(); saveBtn.disabled = false; nameEl.textContent = label; sensorChip.hidden = false; @@ -4704,6 +5152,19 @@ export function mountEditor(root, { sweepClosed = true; filterGen++; abandonAll('the editor was closed'); + /* + * The correction hold is a timer of its own, and it outlived the + * editor: close the page inside the hold and it still fired, still + * called revert(), and changed a camera whose operator had gone. + * A change nobody is watching is put back NOW rather than on a + * schedule -- the whole point of the hold is that someone is there + * to keep it. + */ + if (dpcHold) { + clearTimeout(dpcHold); + dpcHold = null; + try { sensor?.revert(); } catch (e) { /* the page is going */ } + } worker?.terminate(); worker = null; if (styles) releaseStylesheet(); diff --git a/src/iqprofile.js b/src/iqprofile.js index 46f786d..cb254f0 100644 --- a/src/iqprofile.js +++ b/src/iqprofile.js @@ -49,6 +49,42 @@ const numbers = (v) => { * rather than half-read, because a profile is built on top of it and written * to a camera: a zero curve is a real (and broken) curve, and a table set * that is short a temperature would be blended at an undefined one. */ +/* + * What the camera does about bad pixels, out of its own profile. + * + * The section is named for the profile in force -- a camera running its night + * profile calls it [ir_static_dpc] and a day one [static_dpc] -- so the name + * is found rather than assumed, and handed back so a patch can be written to + * the same place it was read from. + * + * DpcStrength is one value per gain step, which is why the answer depends on + * which step the frame was taken at; without a way to map ISO onto that index + * the honest summary is the range, not a single number. + */ +export function readDefectCorrection(ini) { + const name = Object.keys(ini || {}).find((k) => /(^|_)static_dpc$/.test(k)); + if (!name) return null; + const sec = ini[name]; + const nums = (v) => String(v || '').split(',') + .map((x) => Number(x.trim())).filter((x) => Number.isFinite(x)); + const strength = nums(sec.DpcStrength); + return { + section: name, + enabled: String(sec.DpcEnable).trim() === '1', + strength, + /* 0-255 in the vendor's units; reported as a share so the copy can + * speak of it without naming a scale nobody outside the SDK knows. */ + strongest: strength.length ? Math.max(...strength) / 255 : null, + weakest: strength.length ? Math.min(...strength) / 255 : null, + }; +} + +/* The smallest patch that turns the corrector on, written back into the + * section it was read from so a night profile is not given a day one's key. */ +export function enableDefectCorrection(state) { + return '[' + state.section + ']\nDpcEnable = "1"\n'; +} + export function readColour(ini) { const awb = ini.static_awb || {}, ccm = ini.static_ccm || {}; const staticWb = numbers(awb.AutoStaticWb); diff --git a/tests/ui-check.html b/tests/ui-check.html index 6e3c999..70fd1dc 100644 --- a/tests/ui-check.html +++ b/tests/ui-check.html @@ -8,6 +8,73 @@ const results = []; const t = (name, ok, detail = '') => results.push({ name, ok, detail }); +/* + * The phone pass. + * + * Launched in its own browser at 400px because the rules under test are media + * queries and a media query asks the viewport, not an element. It runs alone + * and reports to its own endpoint, so it stays short. + */ +if (new URLSearchParams(location.search).get('narrow') === '1') { + const host = document.createElement('div'); + document.body.append(host); + const ed = mountEditor(host, { base: '../dist/', autoCapture: false, + plates: { readerSupported: false, reader: () => Promise.reject(new Error('none')) } }); + const bytes = new Uint8Array(await (await fetch('./fixture.dng')).arrayBuffer()); + await ed.open(bytes, 'phone.dng'); + await new Promise((r) => setTimeout(r, 400)); + + const stage = host.querySelector('.re-stage').getBoundingClientRect(); + const insp = host.querySelector('.re-insp').getBoundingClientRect(); + t('at 400px the picture still gets most of the width', + stage.width > window.innerWidth * 0.8, + `stage ${stage.width.toFixed(0)}px of ${window.innerWidth}px`); + t('and it is stacked above the panel, not beside it', + insp.top >= stage.bottom - 2, + `stage bottom ${stage.bottom.toFixed(0)}, panel top ${insp.top.toFixed(0)}`); + t('and the picture keeps a usable height', + stage.height > 200, `${stage.height.toFixed(0)}px tall`); + + /* + * Reachable means it can be brought into view, not that it exists. + * + * The first version of this compared each tab's right edge against + * .re-top.scrollWidth -- the width of the whole scrollable content, which + * an offscreen tab is inside by definition. It would have passed with the + * bar still overflowing off the screen, which is the fault it was written + * for. So: scroll to each tab and check it actually lands in the visible + * box. + */ + const top = host.querySelector('.re-top'); + const tabs = [...host.querySelectorAll('.re-top .re-seg button')]; + const unreachable = []; + for (const b of tabs) { + b.scrollIntoView({ block: 'nearest', inline: 'nearest' }); + await new Promise((r) => setTimeout(r, 20)); + const r = b.getBoundingClientRect(), box = top.getBoundingClientRect(); + if (r.left < box.left - 1 || r.right > box.right + 1) unreachable.push(b.textContent.trim()); + } + t('every tab can be scrolled into view', tabs.length > 0 && unreachable.length === 0, + unreachable.length ? 'cannot reach: ' + unreachable.join(', ') + : tabs.map((b) => b.textContent.trim()).join('|')); + t('and the bar is scrollable when its tabs do not fit', + top.scrollWidth <= top.clientWidth + 1 || getComputedStyle(top).overflowX === 'auto', + `scrollWidth ${top.scrollWidth}, clientWidth ${top.clientWidth}`); + + const small = [...host.querySelectorAll('.re-top button, .re-insp button')] + .filter((b) => b.offsetParent !== null) + .filter((b) => b.getBoundingClientRect().height < 40); + t('controls are close to a finger-sized target', + small.length === 0, + small.slice(0, 4).map((b) => `${b.textContent.trim().slice(0, 14) || b.dataset.act}` + + `=${b.getBoundingClientRect().height.toFixed(0)}px`).join(' ')); + + ed.destroy(); + host.remove(); + await fetch('/__results?narrow=1', { method: 'POST', body: JSON.stringify(results) }); + document.title = 'done'; +} else { + const editor = mountEditor(document.getElementById('root'), { base: '../dist/' }); try { @@ -49,8 +116,8 @@ const tabNames = (el) => [...el.querySelectorAll('.re-seg button')].map((b) => b.textContent.trim()); t('no Plates tab without a plates provider', !tabNames(bare).includes('Plates'), tabNames(bare).join('|')); - t('and Diagnose does not offer to read one either', (() => { - const d = [...bare.querySelectorAll('.re-seg button')].find((b) => b.textContent.trim() === 'Diagnose'); + t('and Bad pixels does not offer to read one either', (() => { + const d = [...bare.querySelectorAll('.re-seg button')].find((b) => b.textContent.trim() === 'Bad pixels'); if (!d) return false; d.click(); return !/Reading a plate here/i.test(bare.querySelector('.re-insp').textContent); @@ -94,9 +161,9 @@ t('and asks for one of those to be picked before it measures', /Pick one of the plates above/i.test(insp.textContent), insp.textContent.replace(/\s+/g, ' ').slice(0, 110)); - const diag = [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent.trim() === 'Diagnose'); + const diag = [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent.trim() === 'Bad pixels'); diag.click(); - t('and Diagnose is left to the sensor, with no plate card in it', + t('and Bad pixels is left to the sensor, with no plate card in it', !/Reading a plate here/i.test(host.querySelector('.re-insp').textContent), host.querySelector('.re-insp').textContent.replace(/\s+/g, ' ').slice(0, 110)); } catch (e) { @@ -711,7 +778,7 @@ // Clumped defects: the arrangement of scene detail, not of silicon. await ed.open(new Uint8Array(await (await fetch('/__defects.dng?mode=clustered&n=120')).arrayBuffer()), 'clustered.dng'); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 250)); await scan(); @@ -753,9 +820,22 @@ const wide = countOf(); [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Darkest').click(); await new Promise((r) => setTimeout(r, 250)); - t('choosing it throws away the reading taken somewhere else', - !host.querySelector('.re-devplot'), 'the old histogram is still on screen'); - await scan(); + /* + * Choosing where to look now ANSWERS the question rather than clearing + * the panel and waiting to be asked again. + * + * This check used to require the opposite -- that the old reading be + * thrown away and nothing put in its place -- which is exactly what a + * tester reported as broken: "clicking Darkest made all the results + * vanish and didn't re-scan". The old reading does still have to go; + * it was taken somewhere else. What changed is that a new one arrives + * on its own. + */ + for (let i = 0; i < 150 && !host.querySelector('.re-devplot'); i++) + await new Promise((r) => setTimeout(r, 100)); + t('choosing it re-reads the frame rather than emptying the panel', + !!host.querySelector('.re-devplot'), + host.querySelector('.re-insp').textContent.replace(/\s+/g, ' ').slice(0, 90)); t('and the new reading says where it looked', /only where the picture is dark/i.test(host.textContent)); const narrow = countOf(); @@ -1044,8 +1124,8 @@ return lit > px.length / 8; // not a black frame })()); - const diagBtn = [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose'); - t('Diagnose is a mode now, not a stub', !!diagBtn && !diagBtn.disabled); + const diagBtn = [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels'); + t('Bad pixels is a mode now, not a stub', !!diagBtn && !diagBtn.disabled); diagBtn.click(); await new Promise((r) => setTimeout(r, 300)); t('and it offers to scan rather than scanning unasked', @@ -1081,7 +1161,7 @@ input.value = String(Math.max(+input.min, +input.value - 200)); input.dispatchEvent(new Event('change', { bubbles: true })); await new Promise((r) => setTimeout(r, 500)); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 300)); t('moving the render white point keeps the scan, which is not about it', /Black level/.test(host.querySelector('.re-insp').textContent)); @@ -1093,7 +1173,7 @@ await new Promise((r) => setTimeout(r, 200)); [...host.querySelectorAll('.re-panel .re-seg button')].find((b) => b.textContent === 'BGGR').click(); await new Promise((r) => setTimeout(r, 400)); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 300)); t('changing the Bayer pattern throws the old scan away', !/Black level/.test(host.querySelector('.re-insp').textContent) && @@ -1103,7 +1183,7 @@ // Switching away must take the marks with it. [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Develop').click(); await new Promise((r) => setTimeout(r, 200)); - t('leaving Diagnose hides its marks', host.querySelector('.re-marks').hidden); + t('leaving Bad pixels hides its marks', host.querySelector('.re-marks').hidden); ed.destroy(); host.remove(); } catch (e) { @@ -1130,7 +1210,7 @@ const bytes = new Uint8Array( await (await fetch('/__defects.dng?n=1500&seed=3')).arrayBuffer()); await ed.open(bytes, 'many.dng'); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 120)); host.querySelector('[data-act="scan"]').click(); for (let i = 0; i < 120 && !host.querySelector('.re-devplot'); i++) @@ -1206,7 +1286,7 @@ const first = new Uint8Array( await (await fetch(`/__defects.dng?${query}&seed=99`)).arrayBuffer()); await ed.open(first, 'first.dng'); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 120)); const start = host.querySelector('[data-act="hunt-start"]'); @@ -1295,7 +1375,7 @@ }); await ed.open(new Uint8Array(await (await fetch( '/__defects.dng?level=20&n=40&shared=12&seed=199')).arrayBuffer()), 'frame.dng'); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 120)); host.querySelector('[data-act="hunt-start"]').click(); await new Promise((r) => setTimeout(r, 80)); @@ -1346,7 +1426,7 @@ }); await ed.open(new Uint8Array(await (await fetch( `/__defects.dng?${q}&seed=299`)).arrayBuffer()), 'big.dng'); - [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Diagnose').click(); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); await new Promise((r) => setTimeout(r, 120)); host.querySelector('[data-act="hunt-start"]').click(); await new Promise((r) => setTimeout(r, 80)); @@ -1370,6 +1450,68 @@ t('a truncated guided run', false, e.message); } +/* + * The count above the table and the table itself must be the same count. + * + * They were not. The compare table dropped the open frame's real kept scan and + * stood the tally's own result in for it, so the fifth "capture" was the + * answer offered as evidence for itself: in-all-5 came out as + * #{seen in 5} + #{seen in 4 but not in the fifth}, and a real run reported + * 388 above the table and 397 in it. + * + * The existing fixtures cannot show it. They plant the same `shared` sites in + * every capture, so nothing is ever seen in exactly four and the two numbers + * agree by construction. This one drops the shared sites from the LAST + * capture only, which is exactly the population the bug mis-sorted. + */ +try { + const host = document.createElement('div'); + document.body.append(host); + let shot = 0; + const ed = mountEditor(host, { + base: '../dist/', styles: false, autoCapture: false, + capture: async () => { + shot++; + // 12 shared sites in pictures 1-4, none in picture 5. + const shared = shot >= 5 ? 0 : 12; + const b = new Uint8Array(await (await fetch( + `/__defects.dng?level=20&n=40&shared=${shared}&seed=${400 + shot}`)).arrayBuffer()); + return { bytes: b, name: 'four-of-five-' + shot + '.dng' }; + }, + }); + await ed.open(new Uint8Array(await (await fetch( + '/__defects.dng?level=20&n=40&shared=12&seed=399')).arrayBuffer()), 'first.dng'); + [...host.querySelectorAll('.re-seg button')].find((b) => b.textContent === 'Bad pixels').click(); + await new Promise((r) => setTimeout(r, 120)); + host.querySelector('[data-act="hunt-start"]').click(); + await new Promise((r) => setTimeout(r, 80)); + for (let i = 0; i < 5; i++) { + const b = host.querySelector('[data-act="hunt-step"]'); + if (!b) break; + b.click(); + await huntSettle(host); + } + host.querySelector('[data-act="hunt-finish"]')?.click(); + await new Promise((r) => setTimeout(r, 300)); + + const txt = host.querySelector('.re-insp').textContent.replace(/\s+/g, ' '); + const above = txt.match(/(\d+) of which (?:has|have) turned up every time/); + // The table row is a label span followed by its value span. + const rowValue = (label) => { + const n = [...host.querySelectorAll('.re-rname')] + .find((e) => e.textContent.trim() === label); + return n && n.nextElementSibling ? n.nextElementSibling.textContent.trim() : null; + }; + const inAll = rowValue('in all 5'); + t('the table counts the same captures the line above it does', + !!above && inAll !== null && above[1] === inAll, + `line says ${above && above[1]}, table says ${inAll}`); + ed.destroy(); + host.remove(); +} catch (e) { + t('the compare table agrees with the progress line', false, e.message); +} + /* Confirming is only real once the host has been told. A keep() that fails * must not be reported as kept: the operator would stop watching a change the * host is still armed to undo. */ @@ -2787,4 +2929,5 @@ // which loses the race against the module's own fetches every time. await fetch('/__results', { method: 'POST', body: JSON.stringify(results) }); document.title = 'done'; +} diff --git a/tools/ui-check.mjs b/tools/ui-check.mjs index ace0107..d42acd0 100644 --- a/tools/ui-check.mjs +++ b/tools/ui-check.mjs @@ -24,13 +24,15 @@ function browser() { let reportResults; const reported = new Promise((r) => { reportResults = r; }); +let reportNarrow; +const narrowReported = new Promise((r) => { reportNarrow = r; }); const server = createServer(async (req, res) => { - if (req.method === 'POST' && req.url === '/__results') { + if (req.method === 'POST' && (req.url === '/__results' || req.url === '/__results?narrow=1')) { let body = ''; for await (const c of req) body += c; res.writeHead(204).end(); - reportResults(body); + (req.url.includes('narrow') ? reportNarrow : reportResults)(body); return; } /* @@ -106,10 +108,26 @@ const child = spawn(browser(), [ `http://localhost:${port}/tests/ui-check.html`, ], { stdio: 'ignore' }); +/* + * A second browser, at a phone's width. + * + * The layout rules that matter at 400px are media queries, and a media query + * asks the VIEWPORT -- so shrinking the host element inside the wide run + * cannot reach them. The page runs a short layout-only pass when it sees + * ?narrow=1 and reports separately. + */ +const narrowChild = spawn(browser(), [ + '--headless', '--disable-gpu', '--no-sandbox', '--hide-scrollbars', + '--window-size=400,780', + `http://localhost:${port}/tests/ui-check.html?narrow=1`, +], { stdio: 'ignore' }); + let results; try { + const both = Promise.all([reported, narrowReported]) + .then(([a, b]) => JSON.stringify(JSON.parse(a).concat(JSON.parse(b)))); results = JSON.parse(await Promise.race([ - reported, + both, new Promise((_, rej) => /* 60s once, and the suite reached 58 of them. That is not a budget * any more, it is a coin toss on a slow machine -- and the failure @@ -120,10 +138,12 @@ try { } catch (e) { console.error(e.message); child.kill('SIGKILL'); + narrowChild.kill('SIGKILL'); server.close(); process.exit(1); } finally { child.kill('SIGKILL'); + narrowChild.kill('SIGKILL'); server.close(); }