refactor(cmm): perform PCS adjustments in CIccPcsXform, not CIccXform::Apply() by maxderhak · Pull Request #2332 · InternationalColorConsortium/iccDEV

maxderhak · 2026-08-27T16:07:10Z

PR Summary

Moves PCS adjustments out of CIccXform::Apply() and into CIccPcsXform, so that
adjustments which cancel are folded away by Optimize() instead of leaving a float
residual in the output.

Relates to #2330 and #2331 — both are pre-existing defects this work surfaced and
documented. Neither is fixed here; see "Behaviour changes" below for what this branch
does to them.

Why

A PCS adjustment — absolute-colorimetric media-white scaling, the v2-perceptual
black-point shift, or an IIccAdjustPCSXform hint such as BPC — used to be applied in
two different places depending on where a transform sat in the chain. On an interior
PCS connection, CheckPCSConnections() cleared suppression flags and Connect() pushed
the adjustment as CIccPcsSteps that Optimize() could fold. At a chain edge,
nothing cleared those flags, so the device xform did the work itself inside Apply() —
and the code in ConnectFirst()/ConnectLast() written to handle it was unreachable
dead code, guarded on predicates that nothing ever satisfied.

Applying defaultcmyk.icc forward and back at perceptual intent showed the symptom: the
two adjustments should cancel exactly, and did on the interior path, but not at the edges.

What changed

CheckPCSConnections() hands the adjustment to CIccPcsXform at both chain edges,
not just at interior connections. The leading-edge condition gained the
NeedAdjustPCS() term the trailing-edge one already had.
Per-side virtual predicates NeedsSrcPcsAdjust() / NeedsDstPcsAdjust() answer at
Begin() time what used to be decided inside Apply(), with overrides in
CIccXformMpe and CIccXformNamedColor reproducing their own Apply() guards.
Spectral PCS ports convert against the spectral white point
(relative = absolute / white, absolute = relative × white) instead of receiving the
XYZ affine. Applies to reflectance, transmission and radiant PCS; bidirectional
reflectance and sparse-matrix are excluded because their sample vectors are not
spectra. See docs/superpowers/plans/2026-08-26-spectral-pcs-white-point-conversion.md.
CheckSrcAbs(), CheckDstAbs(), AdjustPCS() and the m_AbsLab scratch buffer are
deleted, along with the m_bSrcPcsConversion / m_bDstPcsConversion flags and the
NeedAdjustSrcPCS() / NeedAdjustDstPCS() predicates.
Design record: docs/pcs-adjustment-placement.md.

Two latent bugs fixed, both in code that had never executed

ConnectLast() halved XYZ values. Its adjustment branch assumed an actual-XYZ
pixel where an XYZ-PCS xform emits internal XYZ, so the unconditional
pushXyzToXyzIn() rescaled by 32768/65535 twice. Wrong since it was written; invisible
because the branch was unreachable. Confirmed fixed arithmetically — the XYZ-PCS
absolute/relative ratio is now exactly mediaX/illumX.
The spectral interior connection dropped its from-side adjustment. Connect()'s
six spectral branches pushed only the to-side, while the interior loop cleared both
flags. Now both sides convert.

Behaviour changes

Every item here is intentional and documented; none is a silent side effect.

Change	Effect
Colorimetric PCS-edged chains	results move ~1e-7 relative from re-associating the same affine math (~4e-6 absolute in Lab, measured on `defaultcmyk.icc`)
Spectral chain edge (#2331)	was an XYZ affine over samples 0-2; now nothing. Not yet the conversion it should get — see the issue
Spectral interior connection	from-side was dropped; now converts
MCS port via a BPC hint (#2330)	was an XYZ affine over channels 0-2; now nothing
XYZ-PCS `NegClip`	the pushed chain is pure affine and does not clamp. Reachable only through an `IIccAdjustPCSXform` hint with `Scale > 1`
`bUsePCSConversions == true`	previously selected the in-`Apply()` path at interior connections; that path no longer exists. No in-tree caller passes `true`
`CheckSrcAbs()` / `CheckDstAbs()` / `AdjustPCS()` deleted	an out-of-tree `CIccXform` subclass calling them now fails to compile. Deliberate: retaining them would have let such a subclass double-apply the adjustment silently, since `m_bAdjustPCS` reads true and the suppressing flags are gone

Verification

.github/ci/regression/pcs-adjust-placement.cpp is new: 176 assertions covering
interior connections, both chain edges, the per-side predicates, the spectral conversion
in all four intent/tag combinations, the excluded PCS types, and both issue reproductions.

Numeric assertions use a 1e-5 relative band throughout, never bit-equality. The
adjustments under test are ~3.5e-3 or larger; re-associating the same affine math moves
results ~1e-7. That gap is the oracle, and it separates "applied exactly once" from
"dropped, doubled, or inverted" while tolerating legitimate float reassociation.

Before deleting the in-Apply() path, its reachability was measured rather than argued:
the two guarded branches were instrumented and the whole suite run for 0 firings, with
a positive control — commenting out the four handover calls produced 64 firings, split
59 destination / 5 source — to prove the instrument fires when the path is live.

BPC black-point deltas were measured against a master baseline binary and are
byte-identical across both BPC intents, with a runtime probe confirming the changed math
actually executes in those chains rather than the null being an artefact. Recorded in
docs/superpowers/plans/2026-08-26-pcs-adjust-bpc-deltas.md, which also carries the
base...HEAD contract matrix.

Checklist

Notes for review

Three checklist items need a maintainer's eye:

Commits are not signed, by decision. All 33 are unsigned: the machine this was
developed on has no signing key configured, so nothing was skipped — there was
nothing to sign with. Signing them retroactively means rewriting the branch, which
would invalidate roughly fifteen commit SHAs cited across the resumption record, the
contract matrix, the design note and this description — a large part of what makes
this work auditable. Nothing in CI verifies signatures. Weighed and left as-is; ask
if maintainers want the rewrite anyway.
Sanitizer coverage: ASan run and clean; the rest is unavailable on Windows.

AddressSanitizer, MSVC 19.44.35227 with /fsanitize=address, Release, over the whole
CMM-focused test set: 76/76 runnable tests pass with no ASan diagnostic, and the new
iccPcsAdjustPlacementTest reports 176/176 assertions clean. The three Not Run entries
are the same missing binaries as in the non-sanitizer tree — tools, IccXML and IccJSON
are off in this configuration — not sanitizer failures. This is the coverage that matters
most here: the branch deletes code on the per-pixel apply path, removes the m_AbsLab
member from CIccApplyXform, and adds heap-allocated spectral white-point vectors.

What could not be covered locally, so CI must carry it:
- UBSan and IntSan do not exist on MSVC. Build/Cmake/CMakeLists.txt refuses them
  by design — message(WARNING "UndefinedBehaviorSanitizer not available on MSVC").
- LeakSanitizer is unsupported by MSVC ASan, so cmmsearch-rebegin-leak,
  cmmsearch-namedcolor-ownership and xform-create-tag-ownership are registered in no
  Windows configuration. Two of those are xform-ownership tests, adjacent to this
  branch's surface, so they are worth watching on the Linux sanitizer job specifically.
- The ClangCL sanitizer path documented in .github/instructions/build-system.instructions.md
  builds clean but will not run on this machine. Configured with -T ClangCL and
  clang 19.1.5, the library and all ~70 test binaries compile under /fsanitize=address
  with exit 0, but every binary dies before main() with
  interception_win: unhandled instruction at 0x..., twice, and
  windows_hook_rtl_allocators=0 does not help. That is ASan's function hooking failing
  against this Windows build, unrelated to this branch — but it means that documented
  path is currently unusable on Windows for any change, which maintainers may want to
  know independently of this PR.
Doxygen ran clean for the changed documents, but with HAVE_DOT=NO because Graphviz is
absent, so CI's exact warning set was not reproduced.
The new regression helper carries the full ICC Software License block, matching
IccProfLib/IccApplyBPC.cpp. Most existing files under .github/ci/regression/ carry
no licence header at all, so this is stricter than local convention rather than looser —
flagging it in case the convention is deliberate.

Also worth a maintainer's opinion: five planning documents under
docs/superpowers/plans/ are included. docs/pcs-adjustment-placement.md is genuine
project documentation and belongs in the tree, and the BPC delta document carries the
contract matrix the checklist requires. The remaining specs and plans are development
artefacts; happy to drop them from the branch if they do not belong upstream.

xsscx

2026-08-27 16:07:46 UTC

Records the measured placement of PCS adjustments today, the unreachable ConnectFirst/ConnectLast branches that cause it, and an eight-task plan to make CIccPcsXform the sole owner so Optimize() can cancel what is redundant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Ruling 1: Task 5 deletes four SetSrc/DstPCSConversion calls, not three pairs. Ruling 2: the XYZ-PCS NegClip is reachable only via an IIccAdjustPCSXform hint with Scale > 1 (BPC); the built-in adjustments cannot produce a negative offset. Task 6 no longer claims a delta it cannot demonstrate. Ruling 3: Testing/iccApplyNamedCmm.exe is a valid master baseline for Task 7 -- every commit from 96507a4 to master is CI-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ization trailingEdgeAdjustmentApplies() called with icRelativeColorimetric compared a relative-intent apply against a relative-intent reference built the same way, so the assertion was unconditionally true regardless of library behavior. Replace it with relativeIntentTakesNoAdjustment(), a structural check that Begin() on a relative-intent xform leaves NeedAdjustPCS() false -- falsifiable the moment relative intent starts requesting a PCS adjustment for any reason. Drop the now-unused bExpectAdjust parameter.

…time Adds CIccXform::NeedsSrcPcsAdjust()/NeedsDstPcsAdjust(), mirroring CheckSrcAbs()/CheckDstAbs() exactly, plus the CIccXformMpe override that reproduces the B2D3/D2B3 absolute-intent guard from CIccXformMpe::Apply(). CIccXformNamedColor overrides are intentionally NOT included in this commit: the brief's specified predicate bodies reference an unqualified IsSpaceSpectralPCS() that does not resolve from IccCmm.h (it collides with an unrelated same-named function in IccSignatureUtils.h, and the real one used by CIccXformNamedColor::Apply() is static to IccCmm.cpp), and the destination-side body as specified also omits the !IsSpaceSpectralPCS(m_nDestSpace) guard that the source-side body carries and that CIccXformNamedColor::Apply() applies symmetrically. See task-3-report.md for detail; flagged for a decision rather than silently resolved. Nothing consumes the new predicates yet, so no behavior changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Declares CIccXformNamedColor::NeedsSrcPcsAdjust()/NeedsDstPcsAdjust() in IccCmm.h but defines them in IccCmm.cpp, beside IsSrcPCS()/IsDestPCS(), where the correct IsSpaceSpectralPCS() (the file-local one testing the five real spectral PCS signatures) is in scope. IccSignatureUtils.h declares a same-named function that tests only a single 'spc ' signature; its parameter type is an exact match for icColorSpaceSignature, so including that header from IccCmm.h to resolve the identifier would have silently selected the wrong predicate instead of failing to compile. Also fixes the destination-side body to carry the same !IsSpaceSpectralPCS(m_nDestSpace) guard the source-side body already had, matching the symmetric spectral-tint branch in CIccXformNamedColor::Apply() (IccCmm.cpp:7800-7827) that the previous commit's dst-side predicate omitted. Adds namedColorAdjustPredicatesRequireAnActualPcsSide(), which builds a real CIccXformNamedColor through CIccXform::Create(..., icXformLutNamedColor) so m_bAdjustPCS is set by the real Begin() path, then sets each xform's untested side to a non-PCS space and asserts the override still says no where the base m_bAdjustPCS/m_bInput formula alone would say yes. Verified by temporarily reverting the source-side override to CIccXform::NeedsSrcPcsAdjust() and confirming the new assertion fails, then restoring it. Nothing consumes the new predicates yet, so no behavior changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Finding 3 (partially addressed): namedColorAdjustPredicatesRequireAnActualPcsSide() used CMYK for the tested side, which makes IsSrcPCS()/IsDestPCS() false on its own -- the !IsSpaceSpectralPCS(...) term added in the previous commit was never exercised, since deleting it from either override still passed every existing assertion. Adds namedColorAdjustPredicatesExcludeSpectralPcs(), which repoints each fixture's CIccTagNamedColor2 PCS at icSigReflectanceSpectralPcsData after CIccXform::Create() (SetColorSpaces() takes the value unvalidated) and sets the tested side to that same signature. IsSrcPCS()/IsDestPCS() -- asserted true directly, so the case is paired the way the PCS term alone would pass -- and only the spectral term can still force NeedsSrcPcsAdjust()/ NeedsDstPcsAdjust() to false. Verified by temporarily deleting "&& !IsSpaceSpectralPCS(...)" from both CIccXformNamedColor overrides in IccCmm.cpp, rebuilding, and confirming exactly the two new assertions fail while the CMYK case and everything else still passes -- pinning that only the new case catches this regression. Restored afterward; IccCmm.cpp/IccCmm.h are unchanged in this commit. Nothing consumes the predicates yet, so no behavior changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Round 2's namedColorAdjustPredicatesExcludeSpectralPcs() forced a CIccTagNamedColor2's PCS to a spectral signature via SetColorSpaces(), directly bypassing CIccProfile's rejection of a spectral m_Header.pcs for any non-DeviceLink profile. CIccXform::Create() always derives a tag's PCS from m_Header.pcs, so that state can never arise through the library's own construction paths -- the test passed, but only inside an object the library cannot build, which is the same "looks like protection and isn't" problem as an unreachable-state test. Spectral PCS reaches named colour only through the m_pArray path, where IsSrcPCS()/IsDestPCS() are IsSpacePCS(m_nSrcSpace/m_nDestSpace) (IccCmm.cpp:7944-7975) -- true for a spectral signature independent of any array's actual configured PCS. A fully populated CIccArrayNamedColor (tag array, named-colour structs, spectral members) to reach that branch honestly was priced and is out of proportion to what this pins. Replaces the fixture with SpectralSideNamedColor, a small CIccXformNamedColor subclass that sets the protected m_pArray to a bare, never-Begin()'d CIccArrayNamedColor. That's enough for the real, unmodified IsSrcPCS()/IsDestPCS() to route through the array branch, since that branch only null-checks m_pArray and never reads its contents -- no mocking of the predicate logic itself. Confirmed IsSrcPCS()/IsDestPCS() are not virtual, so the originally suggested "override IsSrcPCS()/IsDestPCS() in the subclass" shape would not have worked: NeedsSrcPcsAdjust()'s call to IsSrcPCS() is bound at compile time to CIccXformNamedColor::IsSrcPCS() regardless of the dynamic type, since it's a non-virtual call from within a base class member function. Begin() is virtual, so it genuinely can be (and is) overridden to skip CIccXformNamedColor::Begin()'s array tail, which would otherwise call the synthetic array's own Begin() and dereference a null tag array; m_bAdjustPCS/m_bInput -- all NeedsSrcPcsAdjust()/NeedsDstPcsAdjust() need -- come from the inherited CIccXform::Begin() alone. Verified by mutation: deleted "&& !IsSpaceSpectralPCS(...)" from both CIccXformNamedColor overrides, rebuilt, and confirmed exactly the two new "excluded from adjustment" assertions fail (unlike round 2's fixture, which passed the equivalent mutation check only because it happened to also change IsSrcPCS()/IsDestPCS() findings in that same unreachable state). Restored afterward; IccCmm.cpp/IccCmm.h are unchanged in this commit. Nothing consumes the predicates yet, so no behavior changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Clear the src/dst PCS-conversion suppression flags at the leading and trailing chain edges in CheckPCSConnections(), exactly as the interior loop already does, and switch the Connect()/ConnectFirst()/ConnectLast() guards from the flag-gated NeedAdjustSrcPCS()/NeedAdjustDstPCS() to the direction-only NeedsSrcPcsAdjust()/NeedsDstPcsAdjust() predicates. The device xform at each edge no longer performs its own PCS adjustment in Apply(); the newly-built edge CIccPcsXform does it instead. This exposed a latent, previously-unreachable bug in ConnectLast(): its adjustment branch converts actual XYZ to internal XYZ unconditionally, but only converts internal XYZ back to actual XYZ first when the incoming space is Lab. When it's XYZ, the pixel is already internal (CIccXform::Begin()'s PCSOffset is expressed in that domain, and every XYZ-emitting Apply() pre-scales into it), so the missing conversion silently double-scaled the result. Added the missing srcSpace==icSigXYZData branch, mirroring the existing Lab branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Task 4's review established that Task 4's postcondition does not hold for spectral-PCS ports: GetDstSpace() reports spectral while the edge conditions gate on IsSpaceColorimetricPCS(), and Begin() sets m_bAdjustPCS from m_Header.pcs with no spectral exclusion. Such an xform still adjusts inside Apply(). Task 5 was written to delete that path on the strength of the postcondition, and no fixture in the suite is spectral, so the deletion would be silent. Task 5 must now pin the case by test first and stop if it is live. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…verage The comment explaining ConnectLast()'s new icSigXYZData branch cited pushLabToXyz()'s doc comment as the reason pushScale3()/pushOffset3() operate on actual-domain XYZ. That's not what makes the arithmetic correct: it's pushOffset3()'s own bConvertIntXyzOffset default (true), which takes the internal-domain m_PCSOffset and rescales it into the actual-domain step this chain runs on. Rewrote the comment to cite that contract instead, so a later change to the default (or a call site passing false to match the old comment's mental model) doesn't silently halve every perceptual offset with nothing outside pcs-adjust-placement.cpp catching it. Also fixed the Doxygen header immediately above pushOffset3()'s definition, mislabelled pushXyzToXyzIn (pre-existing, one-line, unrelated to this branch otherwise). Added leading-edge numeric coverage in pcs-adjust-placement.cpp, mirroring trailingEdgeAdjustmentApplies(): ConnectFirst()'s three adjust branches were dead code before Task 4 exactly as ConnectLast()'s was, and the structural unhandedAdjustCount() == 0 check alone can't catch a dropped adjustment (a cleared flag plus an identity ConnectFirst() still counts as handed over). Covers both a Lab CMM source (the common shape) and an XYZ CMM source (the shape whose ConnectLast() counterpart was actually broken), each at perceptual and absolute intent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Task 5's precondition asks whether CIccXform::Apply() still performs a PCS adjustment for a spectral-PCS port, which decides whether the in-xform adjustment path can be deleted as dead code. No fixture in the suite was spectral, so the question could not be answered by the existing tests. This adds the fixture the analysis names: a v5 input profile declaring both a Lab PCS and a 6-sample reflectance spectralPCS, carrying only a DToB1 tag, media white != the header illuminant, applied at absolute intent with the spectral signature as the CMM destination. The DToB1 pipeline is a 3->6 matrix with zero coefficients and constant terms, so the unadjusted answer is exact. It is live. Samples 0..2 come out 0.193897/0.301255/0.406660 against pipeline constants 0.200/0.300/0.400; samples 3..5 are untouched. A relative-intent control over the identical fixture emits all six constants exactly, which attributes the head-only deviation to CheckDstAbs() and nothing else in the pipeline. The structural half is pinned too: at the spectral edge CheckPCSConnections() inserts no CIccPcsXform and clears no conversion flag, because both edge blocks gate on IsSpaceColorimetricPCS(GetDstSpace()) while GetDstSpace() returns the spectral signature. No production code is changed. The deletion Task 5 was to perform is blocked on the repository owner deciding what a spectral pixel should receive here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ment Add xyzPcsChainStillAdjusts() to pcs-adjust-placement.cpp, pinning that an XYZ-PCS matrix/TRC chain still applies its media-white adjustment now that the adjustment lives in CIccPcsXform's pure affine step chain rather than in AdjustPCS()'s NegClip-guarded XYZ path. Per spec R8 no clip step is added, so the test records the absolute/relative delta via info: lines rather than asserting a clipping delta -- the built-in adjustments can only produce a non-negative offset, so that clip is unreachable from this fixture; only an IIccAdjustPCSXform hint with Scale > 1 (CIccApplyBPC) can drive it negative, and Task 7 covers that path directly. Add docs/pcs-adjustment-placement.md recording where each adjustment now lands, what moved numerically, the XYZ clip's real reach, and the still-open spectral-PCS edge gap Task 5 found (Apply() still runs AdjustPCS() on a spectral vector's first three samples at chain edges, pending a decision from the repository owner).

Measured intents 40 (perceptual+BPC) and 41 (relative+BPC) on Testing/CMYK-3DLUTs/CMYK-3DLUTs.icc between master (96507a4) and this branch (01b0328). Both intents are byte-identical between baseline and branch, well within the 1e-4 CMYK-percent tolerance. Confirmed the branch binary genuinely loads the rebuilt DLL via a differentiation probe (a non-identity PCS adjustment path showing ~1e-5 Lab differences), then explained the exact-zero BPC result: the fixture round-trips a profile through itself, so the PCS adjustment at each edge is the identity transform on both sides of the refactor and there is no rounding for the removed Lab<->XYZ round trip to introduce.

Review found the original CMYK-3DLUTs.icc-only measurement structurally guaranteed a null result: that v5 profile's getBlackXfm() chain could plausibly never engage m_bAdjustPCS, making the zero delta uninterpretable. Add Testing/V2/v2CmykLut16.icc (in-repo v2.10 CMYK output, lut16Type) as the primary fixture. Before trusting any diff, add a temporary guarded diagnostic to CIccXform::Begin() (IccProfLib/IccCmm.cpp), rebuild, and confirm at runtime that m_bAdjustPCS fires with real non-identity scale/offset for this fixture at both intents 40 and 41 -- then revert the instrumentation and rebuild clean before recording any measurement. The runtime probe also showed the original fixture's claim was not fully accurate: CMYK-3DLUTs.icc does exercise m_bAdjustPCS on 2 of 6 edges via the absolute-adjustment branch (not the perceptual-legacy branch the review's static analysis considered), so it is kept as a secondary case relabeled to match what was actually observed rather than the "unaffected by construction" framing. Both fixtures at both intents remain bit-identical between master (96507a4) and branch (01b0328), but the null is now backed by proof of liveness rather than inferred from an unrelated code path. Rewrote the classification paragraph accordingly and dropped the self-contradictory Lab<->XYZ clause flagged in review.

…ion test The file carried the legacy "ICC Software License, Version 0.2" block copied from IccProfLib/IccApplyBPC.cpp, asserting a 2003-2012 copyright on work written in 2026. That is not the licence this repository asks new files to carry: the pull request template requires "the ICC copyright and BSD 3-Clause license header", CONTRIBUTING.md states ICC software is BSD 3-Clause, and the 24 files under .github/ci/regression/ that carry any header use that block. Now byte-identical to the sibling convention. No code change; 176 assertions still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

xsscx · 2026-08-27T16:46:04Z

Post Merge Report - CI

2026-08-27 16:43:31 UTC

CTest Fail with Hard Coded Profile Number for initial ci-pr-action sibling ci-json-roundtrip
Modified on the fly to Remove the HardCode Reference
Expecting Copilot to Remark the Issue which may hide profile generation failures, which is Correct

Resolution

CI Modify back to hardcoding after manual Review & Confirmation

Copilot

Pull request overview

Refactors PCS adjustment handling into CIccPcsXform, enabling connection-step optimization and spectral white-point conversion.

Changes:

Moves PCS adjustment logic out of per-pixel Apply() paths.
Adds spectral conversion and comprehensive regression coverage.
Updates CTest, CI, benchmark notes, and design documentation.

Review identified a standalone CIccXform regression, weakened profile-count gates, and missing test-catalog documentation.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`AGENTS.md`	Links the PCS placement design record.
`IccProfLib/IccCmm.cpp`	Implements centralized PCS adjustment and spectral conversion.
`IccProfLib/IccCmm.h`	Adds adjustment predicates and port caches; removes legacy APIs.
`Tools/CmdLine/IccBenchApply/BenchCases.cpp`	Updates benchmark commentary.
`Build/Cmake/Testing/CMakeLists.txt`	Registers the regression test and changes profile-count validation.
`.github/ci/regression/pcs-adjust-placement.cpp`	Adds focused PCS placement regression coverage.
`.github/workflows/ci-json-roundtrip.yml`	Changes generated-profile validation.
`docs/pcs-adjustment-placement.md`	Documents the resulting architecture and behavior.
`docs/superpowers/plans/2026-08-26-pcs-adjust-in-pcsxform-spec.md`	Records requirements.
`docs/superpowers/plans/2026-08-26-pcs-adjust-in-pcsxform.md`	Records implementation planning.
`docs/superpowers/plans/2026-08-26-pcs-adjust-RESUME.md`	Records development handoff state.
`docs/superpowers/plans/2026-08-26-spectral-pcs-white-point-conversion.md`	Specifies spectral conversion.
`docs/superpowers/plans/2026-08-26-pcs-adjust-bpc-deltas.md`	Records BPC measurements and contracts.

Suppressed comments (1)

Build/Cmake/Testing/CMakeLists.txt:6116

Removing this expected count lets the Windows profile fixture succeed after generating only a partial corpus. The batch harness already supports EXPECTED_GENERATED_PROFILE_COUNT, and no profile-generator change accompanies this PR, so retain the 135-profile invariant rather than weakening this platform's gate.

      "${ICCDEV_TESTING_DIR}/CreateAllProfiles.bat"

xsscx · 2026-08-27T16:50:50Z

Review Resolution Plan

2026-08-27 16:49:32 UTC

Copilot Review did correctly flag the Issues mention in Comment refactor(cmm): perform PCS adjustments in CIccPcsXform, not CIccXform::Apply() #2332 (comment)
Expected, Normal
TODO @xsscx

xsscx · 2026-08-27T17:20:53Z

Suppressed review disposition: b33103e replaces the Windows fixed-count assertion with verification that every generated entry in Testing/qa-profile-manifest.tsv exists after CreateAllProfiles.bat. This detects partial corpus generation without maintaining a brittle numeric literal.

maxderhak requested review from ChrisCoxArt, colourbill-ctrl, dwtza and xsscx as code owners August 27, 2026 16:07

xsscx self-assigned this Aug 27, 2026

xsscx approved these changes Aug 27, 2026

View reviewed changes

maxderhak and others added 16 commits August 27, 2026 12:08

test: characterize PCS adjustment behavior before moving it

955000e

fix: build a leading-edge CIccPcsXform when the xform needs a PCS adjust

e2357c8

xsscx added this to the v2.3.2.4 milestone Aug 27, 2026

xsscx linked an issue Aug 27, 2026 that may be closed by this pull request

PR Summary #2332 #2333

Closed

13 tasks

ci: remove fixed Windows profile count

75d839f

xsscx removed the Resolving Merge Conflict Maintainer indicates in process Resolution of Merge Conflict. HOLD CHANGES label Aug 27, 2026

xsscx merged commit 12aefbe into master Aug 27, 2026
46 checks passed

xsscx requested a balanced review from Copilot August 27, 2026 16:42

Copilot started reviewing on behalf of xsscx August 27, 2026 16:42 View session

xsscx mentioned this pull request Aug 27, 2026

CI: CTest Fail with Hard Coded Profile Number in #2232 #2334

Closed

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread IccProfLib/IccCmm.h

Comment thread .github/workflows/ci-json-roundtrip.yml

Comment thread Build/Cmake/Testing/CMakeLists.txt

xsscx mentioned this pull request Aug 27, 2026

CI: QA Punch List #2232 #2335

Closed

Navigation Menu

Uh oh!

refactor(cmm): perform PCS adjustments in CIccPcsXform, not CIccXform::Apply() - #2332

refactor(cmm): perform PCS adjustments in CIccPcsXform, not CIccXform::Apply()#2332
xsscx merged 36 commits into
masterfrom
refactor/pcs-adjust-in-pcsxform

maxderhak commented Aug 27, 2026 •

edited

Loading

Uh oh!

xsscx left a comment

Uh oh!

Uh oh!

xsscx commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xsscx commented Aug 27, 2026

Uh oh!

xsscx commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

maxderhak commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Why

What changed

Two latent bugs fixed, both in code that had never executed

Behaviour changes

Verification

Checklist

Notes for review

Uh oh!

xsscx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xsscx commented Aug 27, 2026

Post Merge Report - CI

Resolution

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xsscx commented Aug 27, 2026

Review Resolution Plan

Uh oh!

xsscx commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maxderhak commented Aug 27, 2026 •

edited

Loading