fix(#2520): read the spectral Absolute flag, and set all three emission observer scales - #2522
Merged
Merged
Conversation
…on observer scales Two defects in IccMpeSpectral.cpp, both from the 2015 import and both on the multiply in CIccMpeSpectralObserver::Apply(). The reflectance CLUT and reflectance observer elements computed bUseAbsolute as (m_flags & icRelativeSpectralData)!=0. That constant is 0, so the flag was never read, and the media white adaptation of ICC.2:2023 Formulae (43) to (46) and (51) to (54) ran even when Flags said absolute. The three predicates now test icAbsoluteSpectralData, which is bit 0. CIccMpeEmissionObserver::Begin() assigned m_xyzscale[0] three times. No constructor initializes the array, so Apply() scaled Y and Z by leftover memory on every pixel. It now sets [1] and [2]. The observers' constructor never set m_flags, and neither the copy constructor nor copyData() copied it, so an observer built in code, and every copy of one, carried leftover memory as its flags. CIccProfile's copy constructor reaches the element copy constructor through NewCopy(), as CIccCmm::AddXform(CIccProfile&) does. Apply() already read the Lab bit from there; with bit 0 now read as well, a copied Flags=0 profile could turn absolute. The constructor now sets 0 and both copies copy it, as the spectral CLUT's already do. The reflectance CLUT's local xyzscale[3] now starts at 1.0. With the flag test no longer constant, GCC 15.2 cannot connect the test that fills it to the one that reads it, and fails the strict LTO build with -Werror=maybe-uninitialized. Flags=0 output is unchanged. No tracked profile applies a reflectance or observer element, so the corpus does not move. Left as it is: the magnitude of absolute output. iccDEV normalizes the observer against the illuminant, so a perfect reflector has Y = 1, while Formulae (41) and (50) print k = 1 / sum(cy,i * wi). The new test checks absolute output by chromaticity only and takes no side on k. Absolute (k = 1) is also still not implemented for the emission observer: getEmissiveObserver() always applies the relative k. New CTest: iccdev.spectral-absolute-flag.
colourbill-ctrl
requested review from
ChrisCoxArt,
dwtza,
maxderhak and
xsscx
as code owners
September 11, 2026 02:06
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
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.
Fixes #2520.
Three defects in
IccProfLib/IccMpeSpectral.cpp, all from the 2015 import (1f0a9dd), all feeding the multiply inCIccMpeSpectralObserver::Apply(). The first two are the ones #2520 reports. The third turned up in review of the fix: without it, the fix would make things worse for copied profiles.Change
CIccMpeReflectanceCLUT::Begin(),CIccMpeSpectralObserver::Apply(),CIccMpeReflectanceObserver::Begin()m_flags & icRelativeSpectralDatam_flags & icAbsoluteSpectralDataCIccMpeEmissionObserver::Begin()m_xyzscale[0]assigned three times[0],[1],[2]CIccMpeSpectralObserverdefault ctor, copy ctor,copyData()m_flagsnever set or copiedCIccMpeReflectanceCLUT::Begin()localxyzscale[3]{1, 1, 1}icRelativeSpectralDatais0(icProfileHeader.h:1189), so all three tests were false. ICC.2:2023 §11.2.12 and §11.2.13 apply the media white adaptation (Formulae (43) to (46), (51) to (54)) only "when the Absolute flag is zero".icAbsoluteSpectralData(bit 0) was referenced nowhere in the tree.m_xyzscale, soApply()scaled Y and Z by leftover memory on every pixel.m_flags.CIccCmm::AddXform(CIccProfile&)andCIccXform::Create(CIccProfile&)copy the profile, and the MPE tag copies each element withNewCopy(). On master only the Lab bit was exposed. Once (1) makes bit 0 live, a copiedFlags=0profile could turn absolute. The spectral CLUT constructors already set and copy the field (:825,:868,:911).!bUseAbsolutethat fillsxyzscaleto the one that reads it. Under theci-pr-gcc15configuration it fails the LTO link with-Werror=maybe-uninitialized, measured in CI's image. 1.0 means "no adaptation", so neither path changes.Test:
iccdev.spectral-absolute-flagA custom 81-step PCC with a flat illuminant and a sloped observer (x rises, y flat, z falls), and a sloped white, so the media white is a different colour from the illuminant white. With the flat observer of the #1671 harness every spectrum has the same chromaticity, and relative and absolute output could differ only by a scale. Input spectrum = white.
0.5 1.0 0.5)0.585417,0.414583)m_xyzscaleis poisoned to NaN beforeBegin()NewCopy()/operator=from Flags=1Red/green: each check fails against its own defect
ASan+UBSan Debug (clang 18),
detect_leaks=1. The test returns normally, so LeakSanitizer runs.74579c4cX/Y 0.5 Z/Y 0.5); eobs (Y nan); flags never set (100 0 0, a Lab result);NewCopy();operator=m_xyzscalerevertedm_flagsinit/copy revertedNewCopy(),operator=With the
m_flagsfix reverted in a Release build (no ASan malloc fill), 2 fail:NewCopy()andoperator=. "Flags never set" passed there because the reused memory happened to be zero. Only theoperator=case is deterministic on every build. The other two read leftover memory, which ASan's fill makes wrong.What this does not change
kappears in bothkCrandm_w = kCw, so it cancels out of Formulae (43) to (46).getEmissiveObserver()), so a perfect reflector has Y = 1. Formulae (41) and (50) printk = 1 / Σ c_y,i·w_i. This PR re-enables the branch and does not rule on whichkis intended. The test checks only ratios, so it will not need changing either way.getEmissiveObserver()always applies the relativekof Formula (26), so the flag still has no effect oneobs.eobs/robsare two parse-rejection fixtures in.github/ci/test-data/.Pre-flight
detect_leaks=1ctest -j4iccdev.spectral-tiff-preview:ModuleNotFoundError: No module named 'imagecodecs'on this machine, raised before any iccDEV code runs-Wall -Wextra -Wpedantic -Werrorghcr.io/internationalcolorconsortium/iccdev:latest,ci-pr-gcc15configureiccdev.reflectance-observer-illum-rangepassci-pr-action,ci_scope=sourcec9e66037: success, 14 jobs green.iccdev.spectral-absolute-flagpassed in the job logs of Windows MSVC, Windows ClangCL, MinGW UCRT64 and GCC Strict ASAN+UBSAN Debug (262 / 262)