From ab2f6e562e29b6d7aaff45b88fde1dd3fda5ba87 Mon Sep 17 00:00:00 2001 From: Gordon Bolton Date: Fri, 4 Sep 2026 01:03:07 +0100 Subject: [PATCH] fix(stage5): harden gamut .gam parsing and profcheck output parsing (#179) - Make parseGamutFile support multiple BEGIN_DATA blocks, inline # comments, out-of-bounds vertex warnings, and user-visible parser warnings (returned in { vertices, faces, warnings }). - Harden profcheck report parsing: prefer the last JSON object with avg_de/max_de/rms_de keys, then fall back to a wider set of regex patterns for legacy plain-text output. - Add parser warnings to the process log when metrics cannot be found. - Fix loadGamutMesh call signature and add try/catch around gamut load. - Add manual gamut parser tests in src/js/gamut_viewer.test.js. - Update AGENTS.md, README.md, and ROADMAP.md. Generated with Devin --- AGENTS.md | 7 +++ README.md | 2 +- ROADMAP.md | 2 +- src/js/gamut_viewer.js | 78 ++++++++++++++++++++++++++------ src/js/gamut_viewer.test.js | 80 +++++++++++++++++++++++++++++++++ src/js/profcheck.js | 89 +++++++++++++++++++++++++++++++------ 6 files changed, 230 insertions(+), 28 deletions(-) create mode 100644 src/js/gamut_viewer.test.js diff --git a/AGENTS.md b/AGENTS.md index 5d83ca6..974bda0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,12 @@ # ICCery Agent Notes +## Stage 5 Verification / Profcheck + +- `profcheck` output is parsed from both JSON summaries (preferred) and legacy plain-text report formats. +- If no delta-E values can be detected, the report cards show 0.00 and a warning is appended to the process log. +- The `.gam` file for the 3D viewer is parsed using `parseGamutFile`, which supports multiple `BEGIN_DATA` blocks (some Argyll files use a separate block per surface section), inline `#` comments, and out-of-bounds vertex warnings. +- Manual parser tests live in `src/js/gamut_viewer.test.js`. + ## 3D Gamut Viewer - The viewer renders the measured/derived `.gam` volume and an optional sRGB reference wireframe in CIELAB. diff --git a/README.md b/README.md index cd917d5..205babd 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ 2. **Stage 2 — Target Creation & Raw Printing (`printtarg`)**: Format patch targets for spectrophotometers (i1Pro, i1Pro2, ColorMunki, SpyderPrint). View high-resolution downscaled TIFF previews and print directly using native OS raw unmanaged pathways (Windows GDI uncorrected / Linux CUPS `raw`). 3. **Stage 3 — Interactive Measurement (`chartread`) & Averaging (`average`)**: Instrument auto-detection (`instlist`), real-time calibration prompts, interactive strip reading state machine, live swatch grid with CIEDE2000 ($\Delta E_{00}$) quality indicators and user-configurable traffic-light thresholds, diagonally split intended-vs-measured colour swatches, white reference patch preservation, and multi-pass sheet averaging for measurement noise reduction. 4. **Stage 4 — Profile Calculation (`colprof`)**: Generate high-precision cLUT mathematical ICC/ICM profiles with configurable algorithm quality, OBA/FWA compensation, illuminant/observer selection, viewing-condition transforms, custom ambient spectrum support, descriptions, and copyright tagging. - 5. **Stage 5 — Verification & 3D Gamut (`profcheck` + `iccgamut`)**: Comprehensive mathematical validation report (Peak, Average, RMS $\Delta E$) paired with an interactive 3D CIELAB convex hull color volume viewer, per-vertex true-colour rendering, layer opacity controls, camera reset, keyboard shortcut, touch controls, and bundled sRGB reference wireframe comparison. + 5. **Stage 5 — Verification & 3D Gamut (`profcheck` + `iccgamut`)**: Comprehensive mathematical validation report (Peak, Average, RMS $\Delta E$) paired with an interactive 3D CIELAB convex hull color volume viewer, per-vertex true-colour rendering, layer opacity controls, camera reset, keyboard shortcut, touch controls, bundled sRGB reference wireframe comparison, and robust parsing of both JSON and legacy profcheck output formats. - 📋 **Profiling Presets**: One-click configuration presets (Standard RGB Photo, High-Gamut CMYK Proofing, Fast RGB Draft) with custom preset export/import and security validation. - 🐧 **glibc Compatibility**: Pre-built Linux packages compiled with Ubuntu 22.04 LTS compatibility for Debian/Ubuntu environments. - 🛡️ **Disk Artefact Gating**: Stepper navigation strictly verifies generated artefacts on disk (`.ti1`, `.ti2`, `.ti3`, `.icc`/`.icm`), preventing out-of-order execution while preserving backward navigation. diff --git a/ROADMAP.md b/ROADMAP.md index 7037a51..1254f4a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -106,7 +106,7 @@ ICCery is a native, cross-platform desktop application built with: - [x] **Configurable CIEDE2000 Thresholds (#184)**: User-configurable good/warning ΔE₀₀ upper bounds in Settings, persisted across sessions and applied to the Stage 3 swatch grid. - [x] **Stage 4 OBA/FWA Tooltips & Tests (#176)**: Contextual help for OBA/FWA, illuminant, observer, and viewing-condition controls; additional `colprof` arg builder tests. - [x] **3D Gamut Viewer Controls (#185)**: Camera reset, opacity sliders, keyboard shortcut, and full public-API JSDoc. -- [ ] **Gamut / Profcheck Hardening (#179)**: Validate `.gam` vertex/face parsing, improved `profcheck` regex fallbacks for legacy text output, and user-visible parser warnings. +- [x] **Gamut / Profcheck Hardening (#179)**: Validate `.gam` vertex/face parsing, improved `profcheck` regex fallbacks for legacy text output, and user-visible parser warnings. ### Milestone 12 — Future Workflow & Advanced Analytics (Deferred) - [ ] **Batch Verification & Drift Tracking (#95)**: Track printer drift over time by comparing periodic verification measurements against a baseline profile. diff --git a/src/js/gamut_viewer.js b/src/js/gamut_viewer.js index 4ea8756..35f318f 100644 --- a/src/js/gamut_viewer.js +++ b/src/js/gamut_viewer.js @@ -206,32 +206,78 @@ function _line(from, to, material) { // ───────────────────────────────────────────────────────────────────────────── /** * Parse an Argyll `.gam` text file into vertex and face arrays. + * + * Argyll `.gam` files contain a header followed by one or more `BEGIN_DATA` + * ... `END_DATA` blocks. The first data block is a vertex list + * (index L a b); subsequent blocks contain triangle face indices (v0 v1 v2). + * Blank lines and hash `#` comments outside data blocks are ignored. + * * @param {string} text - Raw contents of the .gam file. - * @returns {{ vertices: number[][], faces: number[][] }} + * @returns {{ vertices: number[][], faces: number[][], warnings: string[] }} */ // ───────────────────────────────────────────────────────────────────────────── export function parseGamutFile(text) { const lines = text.split('\n'); const vertices = []; const faces = []; + const warnings = []; let dataStarted = false; - let dataBlock = 0; // 1 = vertices section, 2 = faces section + let dataBlock = 0; // 1 = vertices section, 2+ = faces sections - for (const line of lines) { - const trimmed = line.trim(); - if (trimmed === 'BEGIN_DATA') { dataBlock++; dataStarted = true; continue; } - if (trimmed === 'END_DATA') { dataStarted = false; continue; } + for (let lineIdx = 0; lineIdx < lines.length; lineIdx++) { + const raw = lines[lineIdx]; + const trimmed = raw.replace(/#.*$/, '').trim(); // strip inline comments + if (trimmed === '') continue; + + if (trimmed.toUpperCase() === 'BEGIN_DATA') { + dataBlock++; + dataStarted = true; + continue; + } + if (trimmed.toUpperCase() === 'END_DATA') { + dataStarted = false; + continue; + } if (!dataStarted) continue; const parts = trimmed.split(/\s+/).map(Number); - if (dataBlock === 1 && parts.length >= 4) { - vertices.push([parts[1], parts[2], parts[3]]); // [L, a, b] - } else if (dataBlock === 2 && parts.length >= 3) { - faces.push([parts[0], parts[1], parts[2]]); + const allNumeric = parts.every(n => !Number.isNaN(n)); + if (!allNumeric) { + warnings.push(`Skipping non-numeric data at line ${lineIdx + 1}`); + continue; + } + + if (dataBlock === 1) { + if (parts.length >= 4) { + // Vertex format: index L a b (index is usually ignored) + const [_, L, a, b] = parts; + if (L < 0 || L > 100 || Math.abs(a) > 128 || Math.abs(b) > 128) { + warnings.push(`Vertex at line ${lineIdx + 1} is outside plausible CIELAB bounds: L=${L}, a=${a}, b=${b}`); + } + vertices.push([L, a, b]); + } else { + warnings.push(`Vertex data at line ${lineIdx + 1} has only ${parts.length} values`); + } + } else { + // Face data can appear in multiple blocks (some .gam files use a + // separate DATA block per surface type or per convex-hull section). + if (parts.length >= 3) { + faces.push([parts[0], parts[1], parts[2]]); + } else { + warnings.push(`Face data at line ${lineIdx + 1} has only ${parts.length} values`); + } } } - return { vertices, faces }; + if (vertices.length > 0 && faces.length === 0) { + warnings.push(`Parsed ${vertices.length} vertices but no faces; will compute convex hull on the fly.`); + } + + if (dataBlock === 0) { + warnings.push('No BEGIN_DATA blocks found; file may be empty or not a valid Argyll .gam file.'); + } + + return { vertices, faces, warnings }; } // ───────────────────────────────────────────────────────────────────────────── @@ -308,7 +354,10 @@ function _renderSrgbReference(text, previousGroup) { }); } - const { vertices, faces } = parseGamutFile(text); + const { vertices, faces, warnings } = parseGamutFile(text); + if (warnings.length > 0) { + console.warn('Gamut parser warnings:', warnings.join('\n')); + } const built = _buildGeometry(vertices, faces); if (!built) return null; @@ -352,7 +401,10 @@ function _renderProfileGamut(text, previousMesh) { if (previousMesh.material) previousMesh.material.dispose(); } - const { vertices, faces } = parseGamutFile(text); + const { vertices, faces, warnings } = parseGamutFile(text); + if (warnings.length > 0) { + console.warn('Gamut parser warnings:', warnings.join('\n')); + } const built = _buildGeometry(vertices, faces); if (!built) return null; diff --git a/src/js/gamut_viewer.test.js b/src/js/gamut_viewer.test.js new file mode 100644 index 0000000..b57a40e --- /dev/null +++ b/src/js/gamut_viewer.test.js @@ -0,0 +1,80 @@ +// Manual / browser-console tests for gamut_viewer.js and profcheck.js parsing. +// Run in a browser/devtools console after the app has loaded: +// import('./gamut_viewer.test.js').then(m => m.runAll()) + +import { parseGamutFile } from './gamut_viewer.js'; + +export function runAll() { + console.group('gamut/profcheck parser tests'); + testParseGamutBasic(); + testParseGamutDualTable(); + testParseGamutWithComments(); + console.groupEnd(); +} + +function assertEqual(actual, expected, message) { + const ok = JSON.stringify(actual) === JSON.stringify(expected); + if (ok) { + console.log('PASS:', message); + } else { + console.error('FAIL:', message, 'expected', expected, 'got', actual); + } + return ok; +} + +function testParseGamutBasic() { + const text = `GAMUT file +BEGIN_DATA +0 50 0 0 +1 100 0 0 +2 0 -128 0 +3 0 0 128 +END_DATA +BEGIN_DATA +0 1 2 +1 2 3 +END_DATA`; + const { vertices, faces, warnings } = parseGamutFile(text); + assertEqual(vertices.length, 4, 'basic gamut vertex count'); + assertEqual(faces.length, 2, 'basic gamut face count'); + assertEqual(warnings.length, 0, 'basic gamut no warnings'); +} + +function testParseGamutDualTable() { + const text = `GAMUT file +BEGIN_DATA +0 50 0 0 +1 100 0 0 +2 0 -128 0 +3 0 0 128 +END_DATA +BEGIN_DATA +0 1 2 +END_DATA +BEGIN_DATA +1 2 3 +END_DATA`; + const { vertices, faces, warnings } = parseGamutFile(text); + assertEqual(vertices.length, 4, 'dual-table gamut vertex count'); + assertEqual(faces.length, 2, 'dual-table gamut face count'); + assertEqual(warnings.length, 0, 'dual-table gamut no warnings'); +} + +function testParseGamutWithComments() { + const text = `GAMUT file +# this is a comment +BEGIN_DATA +0 50 0 0 +1 100 0 0 +# inline comment +2 0 -128 0 +3 0 0 128 +END_DATA +BEGIN_DATA +0 1 2 +1 2 3 +END_DATA`; + const { vertices, faces, warnings } = parseGamutFile(text); + assertEqual(vertices.length, 4, 'commented gamut vertex count'); + assertEqual(faces.length, 2, 'commented gamut face count'); +} diff --git a/src/js/profcheck.js b/src/js/profcheck.js index b1ad304..f88c223 100644 --- a/src/js/profcheck.js +++ b/src/js/profcheck.js @@ -98,7 +98,14 @@ export function initProfcheck() { // Ensure gamut mesh is loaded into 3D viewer const gamFilePath = cwd ? `${cwd}${sep}${basename}.gam` : `${basename}.gam`; - loadGamutMesh(gamFilePath, 0x3b82f6); + try { + const result = await loadGamutMesh(gamFilePath); + if (!result) { + logPre.textContent += `\n[WARN] Could not render 3D gamut mesh from ${gamFilePath}.\n`; + } + } catch (gamErr) { + logPre.textContent += `\n[WARN] 3D gamut render failed: ${gamErr}\n`; + } } else { logPre.textContent += `\n[ERROR] profcheck exited with code ${event.payload.code}.\n`; } @@ -113,33 +120,89 @@ export function initProfcheck() { } }); + /** + * Parse profcheck output for Average, Peak, and RMS delta-E values. + * Supports both Argyll's JSON-style summary and plain-text legacy output. + * @param {string} stdout - Full profcheck stdout. + */ function parseAndRenderReport(stdout) { reportCard.classList.remove("hidden"); let avgDe = 0.0; let maxDe = 0.0; let rmsDe = 0.0; + let parserWarnings = []; - // Check if JSON output is present - const jsonMatch = stdout.match(/\{[\s\S]*"avg_de"[\s\S]*\}/); - if (jsonMatch) { + // Argyll's JSON output can appear either as a compact object on a single + // line or embedded inside larger text. Try to find and parse the LAST valid + // JSON object in the output, which is most likely the summary. + const jsonObjects = []; + const re = /\{[\s\S]*?\}/g; + let m; + while ((m = re.exec(stdout)) !== null) { try { - const json = JSON.parse(jsonMatch[0]); - avgDe = json.avg_de || 0; - maxDe = json.max_de || json.peak_de || 0; - rmsDe = json.rms_de || 0; + const parsed = JSON.parse(m[0]); + if (typeof parsed === 'object' && parsed !== null && ('avg_de' in parsed || 'peak_de' in parsed || 'rms_de' in parsed)) { + jsonObjects.push(parsed); + } } catch (e) { - console.error("JSON parse error:", e); + // Not a valid JSON object, ignore. } + } + + if (jsonObjects.length > 0) { + const json = jsonObjects[jsonObjects.length - 1]; + avgDe = typeof json.avg_de === 'number' ? json.avg_de : 0; + maxDe = typeof json.max_de === 'number' ? json.max_de : (typeof json.peak_de === 'number' ? json.peak_de : 0); + rmsDe = typeof json.rms_de === 'number' ? json.rms_de : 0; } else { - // Regex fallbacks for standard profcheck output - const avgMatch = stdout.match(/avg\.\s*(?:dE\s*)?=\s*([\d\.]+)/i) || stdout.match(/average\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i); - const maxMatch = stdout.match(/max\.\s*(?:dE\s*)?=\s*([\d\.]+)/i) || stdout.match(/peak\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i); - const rmsMatch = stdout.match(/RMS\s*(?:dE\s*)?=\s*([\d\.]+)/i) || stdout.match(/rms\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i); + // Regex fallbacks for standard profcheck text output + const avgPatterns = [ + /avg(?:\.?|erage)\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i, + /average\s+(?:dE\s*)?([\d\.]+)/i, + /mean\s+(?:dE\s*)?([\d\.]+)/i, + /dE\s+average[^\d]*([\d\.]+)/i, + ]; + const maxPatterns = [ + /max(?:\.?|imum)\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i, + /peak\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i, + /worst\s*(?:dE\s*)?([\d\.]+)/i, + /dE\s+max[^\d]*([\d\.]+)/i, + ]; + const rmsPatterns = [ + /RMS\s*(?:dE\s*)?[:=]\s*([\d\.]+)/i, + /rms\s*(?:dE\s*)?([\d\.]+)/i, + /root\s+mean\s+sq(?:uare)?\s*(?:dE\s*)?([\d\.]+)/i, + ]; + + const find = (patterns) => { + for (const p of patterns) { + const match = stdout.match(p); + if (match) return match; + } + return null; + }; + + const avgMatch = find(avgPatterns); + const maxMatch = find(maxPatterns); + const rmsMatch = find(rmsPatterns); if (avgMatch) avgDe = parseFloat(avgMatch[1]); + else parserWarnings.push('Could not detect Average ΔE in profcheck output.'); + if (maxMatch) maxDe = parseFloat(maxMatch[1]); + else parserWarnings.push('Could not detect Peak ΔE in profcheck output.'); + if (rmsMatch) rmsDe = parseFloat(rmsMatch[1]); + else parserWarnings.push('Could not detect RMS ΔE in profcheck output.'); + + if (!avgMatch && !maxMatch && !rmsMatch) { + parserWarnings.push('No delta-E values were found in profcheck output.'); + } + } + + if (parserWarnings.length > 0) { + logPre.textContent += `\n[WARN] ${parserWarnings.join(' ')}\n`; } avgDeEl.textContent = avgDe.toFixed(2); -- 2.39.5