From 78dea36b2cb81395b7fcc1751f5558db83ce955b Mon Sep 17 00:00:00 2001 From: AI Dev Date: Thu, 24 Sep 2026 19:18:46 +0000 Subject: [PATCH 1/3] studio: the scan ends with an answer, and the numbers under it agree A tester put it plainly: the owner's question is "is my sensor OK, and can you fix it?", and the tab answered neither part. It ended on a button that downloaded a list of coordinates, with nothing to say whether the number in it was bad news -- "an owner who pressed Find the bad pixels is left holding a text file". So the run ends on a verdict, and everything else is the working. Whether it is a lot of stuck pixels is judged two ways, because neither is enough alone. The arrangement decides what was found at all: a clumped set is the picture, not the sensor, and that verdict already existed. The count is then read as a FRACTION of the sensor, against the only published acceptance figure this tree has -- Sony selling an IMX415 as good with up to 800 white pixels in the dark, which on a 3864x2192 part is 9.4e-5, rounded here to a hundredth of a percent. A fraction rather than a count so it carries to 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, which matters more: the lab camera reports 882 sites at 0.5 s and 3921 at 7 s -- dark current doing exactly what it should -- so a verdict blind to exposure would call one healthy sensor both fine and faulty within a minute. The exposure and the gain are in the sentence for that reason. The other half of the question is the camera's, so the camera is asked. Its image profile carries the switch for its own defect corrector, and the editor reads it through a new `sensor` provider: already correcting, or off with an offer to turn it on behind the same hold-and-confirm a calibration uses. Without a provider the card says such a corrector exists and that this page cannot see it, which is the degradation Plates and Calibrate already do. What is NOT on offer is sending this scan's coordinates to the camera, which is what the report asked for. The static defect table is refused by the chip on these parts and the profile carries none, so there is nowhere to put a list. The corrector the camera runs is automatic and finds its own. Read from the lab camera, unauthenticated, which is where the copy comes from: GET /api/v1/isp/profile.ini [ir_static_dpc] DpcEnable = "1" DpcStrength = "50, 100, 210, 235, 240, 245, 250, 255, 220, 220, 152, ..." Note the section is named for the profile in force -- a camera on its night profile calls it ir_static_dpc -- so the name is found rather than assumed, and a patch is written back to the section it was read from. The rest of what the tester listed: - One number. "Seen in N of 5" comes from the threshold and is used by the verdict, the sentence and the button alike; the single-frame count moved under the details. - Two five-capture methods on one tab, and two buttons called Capture -- their script pressed the wrong one first. The manual scan and its own keep-and- compare flow fold away under "Advanced -- one frame at a time", and the wizard's button says "Take picture N of 5". - "Where to look" threw the reading away and did not take another. It re-reads the frame now. The check that required the old behaviour has been rewritten rather than deleted, and says why. - Step 1's seven lines of setup, the "Covered --" finding and a nine-line paragraph about dark current were reprinted after every capture. They are said once, and between captures there is a one-line reminder and the count. - Black level, Clipping and Noise lead with a plain judgement and keep their figures behind a disclosure. The black-level warning in particular read as a fault with no remedy; it now says what it costs the picture and that it is the camera's own tuning, not something this page can change. The disclosure is the first in this tree, so it is
and -- they bring the keyboard and the screen reader with them, and a div with a click handler brings neither. It is styled off .re-shead, which was already a heading-and-rule row at fifteen call sites and is the right shape for one. ---- And the numbers it rests on, which did not agree. A tester ran the guided defect hunt and found four numbers on the result screen, two of which disagreed: "388 of which have turned up every time" over a table whose IN ALL 5 row said 397. Both mean the same thing, so one of them was wrong. The compare panel drops the open frame's kept scan out of scanSet and puts a synthetic entry in its place, built from diag.defects. That is right in the manual flow, where diag.defects IS this frame's own reading. It stops being right the moment the tally has replaced diag.defects with its own result: the fifth capture then becomes the conclusion, offered back as evidence for itself. The arithmetic, exactly. With c(k) the number of real captures holding site k, the synthetic entry contributes [c(k) >= 4], so the table counted in-all-5 = #{c = 5} + #{c = 4 and absent from capture 5} and the nine sites in that second term had missed a capture while being reported as having missed none. It runs the other way too: a site seen only in the last capture came out at zero and vanished from the table altogether, which is why the rows stopped summing to the count above them -- the fourth number. The same fault fires in the manual flow the instant Mark is pressed, and asking for "5 of 5" afterwards asserted that 397 sites turned up in all five when 388 had. So the tally runs over the captures and never over its own answer: if this frame has been kept, scanSet already holds its real reading; stand in for it only when it has not been kept and diag is a genuine single-frame scan. Four more found while in here: - Mark dropped `partial` when it rebuilt fromTally, so pressing it turned the exported header's "# complete" line 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. - "That capture did not arrive. The message above says why." was shown when takeFrame returned in silence because the editor was busy, which posts no message anywhere. That case has its own sentence now, and the step button is disabled until there is a frame and the editor is idle -- it used to be live before the first frame had loaded, which is how the tester reached it. - The deviation histogram is one frame's, and after a tally it rode along on the spread of diag under a headline about five captures. - Changing "where to look" mid-run left scanSet holding captures read under different gates, which the tally counted as comparable. The run owns the gate while it is open. The regression test needed a fixture the suite could not produce: every existing one plants the same shared sites in every capture, so nothing is ever seen in exactly four and the two numbers agree whatever the code does. The new one drops the shared sites from the last capture only. Watched failing with the fix reverted. --- dist/editor.js | 469 ++++++++++++++++++++++++++++++++++++++++---- dist/iqprofile.js | 36 ++++ src/editor.js | 469 ++++++++++++++++++++++++++++++++++++++++---- src/iqprofile.js | 36 ++++ tests/ui-check.html | 81 +++++++- 5 files changed, 1002 insertions(+), 89 deletions(-) diff --git a/dist/editor.js b/dist/editor.js index 3efdd1a..de00585 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 @@ -1250,6 +1273,168 @@ 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 { dpc = readDefectCorrection(parseIni(await sensor.profile())); } + 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.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) { await sensor.keep(); dpcHold = null; 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) { + 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.']; + 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. @@ -1278,10 +1463,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 +1645,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 +1700,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 +1726,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 +1754,23 @@ 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); + 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 +1804,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 +1850,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,7 +1895,8 @@ 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'); @@ -1647,7 +1908,10 @@ export function mountEditor(root, { // 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 +1927,8 @@ export function mountEditor(root, { run.disabled = false; run.textContent = 'Scan the frame'; } - }); + }; + run.addEventListener('click', () => runScan()); } /* @@ -1755,6 +2020,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 +2283,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 +2321,33 @@ 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); + const all = kept || diag.fromTally + ? scanSet.slice() + : scanSet.filter((v) => v.id !== state.openId) + .concat([{ name: state.name || 'this frame', + keys: 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 +2473,18 @@ 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) } }; renderDiagnose(out); drawMarks(); }); @@ -2181,34 +2514,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 +4973,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 Diagnose 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; 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.js b/src/editor.js index 3efdd1a..de00585 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 @@ -1250,6 +1273,168 @@ 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 { dpc = readDefectCorrection(parseIni(await sensor.profile())); } + 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.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) { await sensor.keep(); dpcHold = null; 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) { + 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.']; + 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. @@ -1278,10 +1463,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 +1645,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 +1700,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 +1726,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 +1754,23 @@ 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); + 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 +1804,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 +1850,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,7 +1895,8 @@ 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'); @@ -1647,7 +1908,10 @@ export function mountEditor(root, { // 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 +1927,8 @@ export function mountEditor(root, { run.disabled = false; run.textContent = 'Scan the frame'; } - }); + }; + run.addEventListener('click', () => runScan()); } /* @@ -1755,6 +2020,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 +2283,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 +2321,33 @@ 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); + const all = kept || diag.fromTally + ? scanSet.slice() + : scanSet.filter((v) => v.id !== state.openId) + .concat([{ name: state.name || 'this frame', + keys: 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 +2473,18 @@ 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) } }; renderDiagnose(out); drawMarks(); }); @@ -2181,34 +2514,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 +4973,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 Diagnose 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; 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..95532bd 100644 --- a/tests/ui-check.html +++ b/tests/ui-check.html @@ -753,9 +753,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(); @@ -1370,6 +1383,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 === 'Diagnose').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. */ From e1917997994505fe82615262abf722d116ce6193 Mon Sep 17 00:00:00 2001 From: AI Dev Date: Thu, 24 Sep 2026 19:18:50 +0000 Subject: [PATCH 2/3] studio: the editor fits a phone, and its text can be read Three presentation faults, all of them editor-wide rather than anything to do with the tab they were found on. The inspector rail is flex: 0 0 328px with no min-width, inside a row that never wraps. At 400px that leaves the picture 71px of a 400px screen -- 43px of it once the stage's own padding is taken -- so a phone showed a strip beside a panel. Under 560px the two stack instead: the rail goes full width below the picture, the stage keeps a floor of 42vh, and the rail's left border becomes a top one. The DOM is already picture-then-rail, so nothing moves. The tab bar is white-space: nowrap inside .re-top > * { flex: 0 0 auto }, with no overflow declared anywhere. A fifth tab and the Capture and Download buttons after it simply left the screen, with nothing to scroll to reach them. The bar scrolls now. And the tertiary text colour, #878a94, is 4.32:1 on the raised #232734 that panels are drawn in -- under the 4.5:1 minimum, on the class that carries every piece of explanatory prose in the editor. It passes on the rail, at 4.78:1, which is presumably how it survived. #a2a6b2 is 6.12:1 on the panel and 6.77:1 on the rail. Measured with the WCAG formula, checked against known pairs first (white on black 21.00, #767676 on white 4.54); two other figures were in circulation for this and both were wrong. Touch targets: nothing in the editor reached the 44px guideline -- 32 for a button, 28 for a segment, 25 for the main tab bar, 16 for a reset. Worse, the 720px query REDUCED button padding and stripped their labels, making them smaller on exactly the devices most likely to be touched. Under 560px the interactive minimum is 44px, and that query no longer shrinks anything. The check for this needed a second browser. The rules are media queries and a media query asks the viewport, so shrinking a host element inside the existing 1280px run cannot reach them; the harness now launches a 400px pass that runs a short layout-only page and reports separately. It earned itself immediately: the first version of the touch-target rules sat ABOVE .re-seg.re-tight button in the file and lost to it on source order at equal specificity, so the tab bar stayed at 25px and the new check said so. Also, re-ok and re-warn were told apart by an icon colour and a 1px border at 60% alpha -- and most call sites handed both variants the same warning triangle, so a good answer arrived wearing one. There is a tick now. --- dist/editor.css | 74 ++++++++++++++++++++++++++++++++++++++++----- src/editor.css | 74 ++++++++++++++++++++++++++++++++++++++++----- tests/ui-check.html | 53 ++++++++++++++++++++++++++++++++ tools/ui-check.mjs | 26 ++++++++++++++-- 4 files changed, 208 insertions(+), 19 deletions(-) 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/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/tests/ui-check.html b/tests/ui-check.html index 95532bd..3d3a630 100644 --- a/tests/ui-check.html +++ b/tests/ui-check.html @@ -8,6 +8,58 @@ 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`); + + const tabs = [...host.querySelectorAll('.re-top .re-seg button')]; + t('every tab is reachable rather than off the edge', + tabs.length > 0 && tabs.every((b) => { + const r = b.getBoundingClientRect(); + return r.left >= -1 && r.right <= host.querySelector('.re-top').scrollWidth + 1; + }), tabs.map((b) => b.textContent).join('|')); + const top = host.querySelector('.re-top'); + t('and the bar can be scrolled to them when it overflows', + 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 { @@ -2862,4 +2914,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(); } From 01326aadf6cf3366f86fae36a211c5377c4af838 Mon Sep 17 00:00:00 2001 From: AI Dev Date: Thu, 24 Sep 2026 19:35:07 +0000 Subject: [PATCH 3/3] studio: the tab says what it finds, not what it does to find it "Diagnose" is a verb with no object. It never said what was being diagnosed, so an owner wondering whether their sensor has stuck pixels had no reason to guess they were behind it -- and the tab's whole job, after the guided run went in, is to answer exactly that question. It is "Bad pixels" now. The other labels on that bar are Develop, Calibrate, Plates and Focus: three of them name the thing you are working on. This one does too. The mode key stays `diagnose`. It is internal, it appears at around forty sites, and renaming it would be churn with nothing visible at the end of it. The checks move to the new label, which is also the first real exercise of the scrollable tab bar from the commit below -- "Bad pixels" is two characters wider than what it replaces. ---- Review of the three commits below found eight things, and the two that matter most were in the part that writes to the camera. Pressing "Keep it on" assigned null to the hold's timer handle without cancelling the timer it named. The clock ran on, and when it expired it put back a change the owner had just confirmed -- while the card, reading dpc.enabled, went on saying the correction was on. It is cleared first now, before keep() is called at all. And that hold outlived the editor. Closing the page inside it left a timer that still fired and still called revert(), changing a camera whose operator had gone. destroy() cancels it and puts the change back at once instead: the hold exists because somebody is there to keep it, so once nobody is, there is nothing left to wait for. Two in the tally, one of them mine from two commits down. "Use scanSet when the frame is kept or the reading is a tally" is right after a guided run, where keepCurrentScan has already put the frame in. In the manual flow it has not, and the reading on screen stands in for it -- so Mark threw that stand-in away, dropped the capture count by one, and moved the votes and the suggested threshold with it, on a click that changed no evidence. A tally carries the frame's own keys now. And the verdict did not know about truncation: a run where a capture filled the store has a short list, by an unknown amount in a known place, so the fraction taken from it could be read as "normal" off the very evidence that went missing. It says what it does not know instead. Three about the camera card. A profile with no correction section read as a request still in flight, and since dpcAsked blocks the retry the card said "Asking the camera..." for as long as the editor stayed open; that is an answer now, with its own sentence. The reading was never asked for twice, so a second run reported the first one's state though the setting can change between them. And the palette comment named the wrong colour -- a blanket replace of the old value rewrote the sentence describing it, leaving a comment claiming #a2a6b2 was both 4.32:1 and 6.12:1 against the same background. The eighth was a check, and it was worthless in the way its own comment now admits: it compared each tab's right edge against the bar's scrollWidth, which is the width of the whole scrollable content -- an offscreen tab is inside that by definition, so it would have passed with the bar still overflowing off the screen, which is the fault it was written for. It scrolls to each tab and checks it lands in the visible box. --- README.md | 2 +- dist/editor.js | 106 ++++++++++++++++++++++++++++++++++++++------ src/editor.js | 106 ++++++++++++++++++++++++++++++++++++++------ tests/ui-check.html | 59 +++++++++++++++--------- 4 files changed, 222 insertions(+), 51 deletions(-) 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.js b/dist/editor.js index de00585..e6b3258 100644 --- a/dist/editor.js +++ b/dist/editor.js @@ -397,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, @@ -1113,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. * @@ -1304,8 +1309,16 @@ export function mountEditor(root, { async function readDpc() { if (dpcAsked || !sensor || !sensor.profile) return; dpcAsked = true; - try { dpc = readDefectCorrection(parseIni(await sensor.profile())); } - catch (e) { dpc = { failed: e && e.message ? e.message : 'could not be read' }; } + 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(); } @@ -1332,6 +1345,14 @@ export function mountEditor(root, { 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', @@ -1371,7 +1392,16 @@ export function mountEditor(root, { on.addEventListener('click', async () => { on.disabled = true; try { - if (dpcHold) { await sensor.keep(); dpcHold = null; dpc.enabled = true; } + 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 @@ -1396,7 +1426,7 @@ export function mountEditor(root, { })); } - function sensorVerdict(pts) { + function sensorVerdict(pts, partial) { const i = state.info || {}; const total = (i.width || 0) * (i.height || 0); if (!pts || !total) return null; @@ -1424,6 +1454,19 @@ export function mountEditor(root, { '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 ' + @@ -1440,6 +1483,10 @@ export function mountEditor(root, { // 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 @@ -1761,7 +1808,8 @@ export function mountEditor(root, { * 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); + 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'; @@ -1902,7 +1950,7 @@ export function mountEditor(root, { 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. @@ -2343,11 +2391,25 @@ export function mountEditor(root, { * been kept and `diag` is a genuine single-frame scan. */ const kept = scanSet.some((v) => v.id === state.openId); - const all = kept || diag.fromTally + /* + * 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([{ name: state.name || 'this frame', - keys: diag.defects.map((p) => p.x + ',' + p.y) }]); + .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'), { @@ -2484,7 +2546,10 @@ export function mountEditor(root, { */ diag = { ...diag, defects: pts, defectCount: pts.length, truncated: false, fromTally: { need: needN, of: M, - partial: all.some((v) => v.truncated) } }; + 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(); }); @@ -4974,7 +5039,7 @@ export function mountEditor(root, { // against a peak held from a lens position that no longer exists. resetFocusState(); /* - * A frame arriving is news to the Diagnose rail. Nothing rebuilt + * 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 @@ -5087,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/editor.js b/src/editor.js index de00585..e6b3258 100644 --- a/src/editor.js +++ b/src/editor.js @@ -397,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, @@ -1113,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. * @@ -1304,8 +1309,16 @@ export function mountEditor(root, { async function readDpc() { if (dpcAsked || !sensor || !sensor.profile) return; dpcAsked = true; - try { dpc = readDefectCorrection(parseIni(await sensor.profile())); } - catch (e) { dpc = { failed: e && e.message ? e.message : 'could not be read' }; } + 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(); } @@ -1332,6 +1345,14 @@ export function mountEditor(root, { 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', @@ -1371,7 +1392,16 @@ export function mountEditor(root, { on.addEventListener('click', async () => { on.disabled = true; try { - if (dpcHold) { await sensor.keep(); dpcHold = null; dpc.enabled = true; } + 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 @@ -1396,7 +1426,7 @@ export function mountEditor(root, { })); } - function sensorVerdict(pts) { + function sensorVerdict(pts, partial) { const i = state.info || {}; const total = (i.width || 0) * (i.height || 0); if (!pts || !total) return null; @@ -1424,6 +1454,19 @@ export function mountEditor(root, { '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 ' + @@ -1440,6 +1483,10 @@ export function mountEditor(root, { // 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 @@ -1761,7 +1808,8 @@ export function mountEditor(root, { * 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); + 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'; @@ -1902,7 +1950,7 @@ export function mountEditor(root, { 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. @@ -2343,11 +2391,25 @@ export function mountEditor(root, { * been kept and `diag` is a genuine single-frame scan. */ const kept = scanSet.some((v) => v.id === state.openId); - const all = kept || diag.fromTally + /* + * 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([{ name: state.name || 'this frame', - keys: diag.defects.map((p) => p.x + ',' + p.y) }]); + .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'), { @@ -2484,7 +2546,10 @@ export function mountEditor(root, { */ diag = { ...diag, defects: pts, defectCount: pts.length, truncated: false, fromTally: { need: needN, of: M, - partial: all.some((v) => v.truncated) } }; + 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(); }); @@ -4974,7 +5039,7 @@ export function mountEditor(root, { // against a peak held from a lens position that no longer exists. resetFocusState(); /* - * A frame arriving is news to the Diagnose rail. Nothing rebuilt + * 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 @@ -5087,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/tests/ui-check.html b/tests/ui-check.html index 3d3a630..70fd1dc 100644 --- a/tests/ui-check.html +++ b/tests/ui-check.html @@ -35,14 +35,29 @@ t('and the picture keeps a usable height', stage.height > 200, `${stage.height.toFixed(0)}px tall`); - const tabs = [...host.querySelectorAll('.re-top .re-seg button')]; - t('every tab is reachable rather than off the edge', - tabs.length > 0 && tabs.every((b) => { - const r = b.getBoundingClientRect(); - return r.left >= -1 && r.right <= host.querySelector('.re-top').scrollWidth + 1; - }), tabs.map((b) => b.textContent).join('|')); + /* + * 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'); - t('and the bar can be scrolled to them when it overflows', + 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}`); @@ -101,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); @@ -146,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) { @@ -763,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(); @@ -1109,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', @@ -1146,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)); @@ -1158,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) && @@ -1168,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) { @@ -1195,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++) @@ -1271,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"]'); @@ -1360,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)); @@ -1411,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)); @@ -1466,7 +1481,7 @@ }); 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 === '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));