Sync iccviz engine + CTests + Copilot Review 2 fixes (in-repo, supersedes #1712)#1716
Closed
colourbill-ctrl wants to merge 3 commits into
Closed
Sync iccviz engine + CTests + Copilot Review 2 fixes (in-repo, supersedes #1712)#1716colourbill-ctrl wants to merge 3 commits into
colourbill-ctrl wants to merge 3 commits into
Conversation
) Bring the data-first visualization engine under Tools/CmdLine/IccProfilePlot/ (IccVizModel, landed in #1517) up to date with three feature-additive changes, plus the caller updates they require. No refactor of the enumerate/render surface; no change to IccProfLib or to the Mini{PDF,SVG,TIFF} writers. Engine (IccVizModel.hpp/.cpp, IccVizMath.hpp): - GamutVolume(): gamut volume in dE*ab^3 via boundary voxelisation + flood-fill through the AToB transform; scalar GamutVolumeResult with a `degenerate` flag so a collapsed sampling is distinguishable from a genuinely tiny gamut. - RoundTripDE(): B2A->A2B round-trip accuracy (mean/p90/max/std dE*ab) seeded from in-gamut device-cube samples; scalar RoundTripResult. - Kind::NeutralAxisInking: device colorant along the neutral axis from a PCS->device (BToA) table, enumerated per BToA tag. InkReversalL retired (enum value reserved). Plus GamutVolume accuracy work (IccProfLib CIELAB reuse) and CLUT-raster / new-metric hardening against malformed input. Breaking API (internal only): Enumerate(pIcc, Order) loses its Order argument. Enumerate(pIcc) now returns one canonical order (chromaticity, then per-tag A2B0/1/2, B2A0/1/2, gamut, then named/colorant). Only in-tree caller updated. iccProfileVisualize.cpp: - Drop the Order argument at the Enumerate call site. - Render Kind::NeutralAxisInking into the PDF report (was engine-enumerated but silently skipped): CreateNeutralAxesXobject (L*/%-ink axes frame) + renderNeutralAxisGraph (one polyline per colorant along L* 100->0, distinct CMYK stroke, channel-name legend) + the processLuts switch case. One page per PCS->device table. The engine sources are algorithmically identical to the upstream copy; the only deltas are the restored ICC license header on IccVizModel.cpp and an ASCII punctuation sweep (matching the #1517 strict-build convention), which changes five graph-title string literals from an em-dash to a hyphen. All translation units build clean under -Wall -Wextra. The old "byte-identical vs original iccProfileVisualize" PDF baseline is intentionally superseded by the canonical page order and the chromaticity page for device/CMYK profiles; a pinned-output regression + CTests for the two new scalar metrics follow as a separate in-repo PR per the maintainer-owned CTest policy. Refs #1711.
Adds the regression coverage requested in the #1712 review (item 1) for the iccviz engine synced in #1711. * iccviz-gamut-roundtrip-metrics: drives GamutVolume and RoundTripDE end-to-end on the stock sRGB v4 LUT profile -- positive 3-D gamut volume, not degenerate, small in-gamut round-trip dE -- and asserts a coarse-grid GamutVolume still flags degenerate through the public API. Compiles IccVizModel.cpp directly (the metrics live in tool sources, not IccProfLib). * iccviz-degenerate-detection: header-only unit test of iccvizmath::principalStdDevs on synthetic blob/plane/line clouds; calibrates the 0.02 coplanarity threshold (s3/s1 ratio 0.01 flags flat, 0.05 does not). * iccviz-pdf-axis-labels: runs iccProfileVisualizePlot and asserts the emitted LUT PDF carries both DrawAxisPDF label paths -- the default identity axis (Input/Output) and the parameterized custom-tick axis (L* (100 to 0) / % ink). A content pin (uncompressed-stream label match), not a byte-for-byte golden PDF, so a legitimate LUT curve-sampling shift will not false-fail it. Registered on both the WIN32 and POSIX test paths, gated behind their targets and labelled issue-1712. Each assertion was mutation-verified against the shipped implementation.
Engine/tool hardening (Tools/CmdLine/IccProfilePlot): - GamutVolume: reject non-AToB signatures up front. Because the transform is built with bInput=true, GetSrcSpace() reports the profile's device space even for a BToA tag, so the existing isPcsSpace() guard could not catch it; a BToA tag would run its LUT backwards and return a plausible-but-meaningless volume. - buildNeutralAxisGraph: require all three Lab components finite before formatting the colour hint (a*/b* NaN/Inf could previously reach the snprintf). - renderNeutralAxisGraph: give every one of the up-to-15 ink channels a distinct stroke colour instead of aliasing at 8 (c % 8 -> c % kInkStrokeCount). - processLuts: correct the stale Enumerate ordering comment (fixed canonical signature list, not the profile's tag-table order). Test coverage (.github/ci/regression, Build/Cmake/Testing): - gamut-roundtrip-metrics: add a BToA-rejection check and a synthetic planar-gamut AToB profile that pins GamutVolume's flat/degenerate predicate end-to-end (voxels > 27, so only the s3 < 0.02*s1 term can flag it) -- both red-green verified. - degenerate-detection: add a rotated anisotropic cloud so principalStdDevs' Smith 1961 trig branch (p1 > 0) is exercised, not just the diagonal fast-path. - RunIccVizPdfAxisTest: assert the parameterized tick operators (default 50%/100% vs reversed 100/50), not just axis titles, and add the ICC BSD header. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSLYQF8bCNVftcr9tuAAnN
colourbill-ctrl
requested review from
ChrisCoxArt,
dwtza,
maxderhak and
xsscx
as code owners
July 18, 2026 05:08
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #1712 (fork PR). This is the same iccviz sync — GamutVolume, RoundTripDE,
neutral-axis inking (#1711) — plus the CTests, plus the Copilot Review 2 fixes,
now opened in-repo so the CI-adjacent files (
.github/ci/regression/**,Build/Cmake/Testing/**) don't trip the fork-PR maintainer automation gate thatblocked #1712. #1712 will be closed once this passes CI.
Commits
Sync iccviz engine: GamutVolume, RoundTripDE, neutral-axis inking (#1711)Add CTests for iccviz gamut/roundtrip metrics, degeneracy, PDF axesAddress Copilot Review 2 on the iccviz sync (#1712)Copilot Review 2 disposition
.cpp.github/ci/regression/siblings (pawg-q4-xyz-pcs-decode.cpp,tag-layout-shared-data.cpp,embedio-read8-bounds.cpp) carry no header; following directory convention.cmakedriverRunWindows*drivers)is_flat_ratioduplicated the production predicate, not pinnedGamutVolume'ss3 < 0.02*s1flat branch end-to-end (voxels > 27, so onlyflatcan flag it)50%/100%vs reversed100/50)bInput=truemakesGetSrcSpace()report the device space so theisPcsSpace()guard could not catch a BToA tag (it would run the LUT backwards)L*checked finitea*/b*too before formatting the colour hintc % 8repeated strokes ≥ channel 9Enumeratetag-table commentFixes c, f, and e are red-green verified (revert the guard → the paired CTest fails).
All three iccviz CTests pass locally (
ctest -R iccviz).🤖 Generated with Claude Code
https://claude.ai/code/session_01PSLYQF8bCNVftcr9tuAAnN