Skip to content

fix(#2520): read the spectral Absolute flag, and set all three emission observer scales - #2522

Merged
xsscx merged 1 commit into
masterfrom
fix/2520-spectral-absolute-flag
Sep 11, 2026
Merged

fix(#2520): read the spectral Absolute flag, and set all three emission observer scales#2522
xsscx merged 1 commit into
masterfrom
fix/2520-spectral-absolute-flag

Conversation

@colourbill-ctrl

Copy link
Copy Markdown
Contributor

Fixes #2520.

Three defects in IccProfLib/IccMpeSpectral.cpp, all from the 2015 import (1f0a9dd), all feeding the multiply in CIccMpeSpectralObserver::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

site before after
CIccMpeReflectanceCLUT::Begin(), CIccMpeSpectralObserver::Apply(), CIccMpeReflectanceObserver::Begin() m_flags & icRelativeSpectralData m_flags & icAbsoluteSpectralData
CIccMpeEmissionObserver::Begin() m_xyzscale[0] assigned three times [0], [1], [2]
CIccMpeSpectralObserver default ctor, copy ctor, copyData() m_flags never set or copied set to 0, copied
CIccMpeReflectanceCLUT::Begin() local xyzscale[3] uninitialized {1, 1, 1}
  1. Absolute flag. icRelativeSpectralData is 0 (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.
  2. Emission scale. No constructor initializes m_xyzscale, so Apply() scaled Y and Z by leftover memory on every pixel.
  3. Observer flags. An observer built in code, and every copy of one, carried leftover memory as m_flags. CIccCmm::AddXform(CIccProfile&) and CIccXform::Create(CIccProfile&) copy the profile, and the MPE tag copies each element with NewCopy(). On master only the Lab bit was exposed. Once (1) makes bit 0 live, a copied Flags=0 profile could turn absolute. The spectral CLUT constructors already set and copy the field (:825, :868, :911).
  4. Initializer. Once the flag test is no longer constant, GCC 15.2 cannot connect the !bUseAbsolute that fills xyzscale to the one that reads it. Under the ci-pr-gcc15 configuration 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-flag

A 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.

check asserts
rclt, robs, Flags=0 media white → illuminant white, full XYZ (0.5 1.0 0.5)
rclt, robs, Flags=1 media white keeps its own chromaticity, X/Y and Z/Y only (0.585417, 0.414583)
eobs, Flags=0 white emission → Y = 1 with the observer's chromaticity (Formula (26)); m_xyzscale is poisoned to NaN before Begin()
robs built in code, flags never set relative, as Flags=0
robs copied with NewCopy() / operator= from Flags=1 the copy is absolute
fixture media white and illuminant white differ by > 0.05 in both ratios, so the absolute checks cannot pass vacuously

Red/green: each check fails against its own defect

ASan+UBSan Debug (clang 18), detect_leaks=1. The test returns normally, so LeakSanitizer runs.

library ok FAIL failing
master 74579c4c 12 6 rclt abs, robs abs (X/Y 0.5 Z/Y 0.5); eobs (Y nan); flags never set (100 0 0, a Lab result); NewCopy(); operator=
flag tests reverted 14 4 rclt abs, robs abs, both copies
m_xyzscale reverted 17 1 eobs
m_flags init/copy reverted 15 3 flags never set, NewCopy(), operator=
this branch 18 0

With the m_flags fix reverted in a Release build (no ASan malloc fill), 2 fail: NewCopy() and operator=. "Flags never set" passed there because the reused memory happened to be zero. Only the operator= case is deterministic on every build. The other two read leftover memory, which ASan's fill makes wrong.

What this does not change

  • Relative (Flags=0) output. Master passes every Flags=0 check. k appears in both kCr and m_w = kCw, so it cancels out of Formulae (43) to (46).
  • Absolute magnitude. iccDEV's absolute branch normalizes the observer against the illuminant (getEmissiveObserver()), so a perfect reflector has Y = 1. Formulae (41) and (50) print k = 1 / Σ c_y,i·w_i. This PR re-enables the branch and does not rule on which k is intended. The test checks only ratios, so it will not need changing either way.
  • Emission observer Absolute. getEmissiveObserver() always applies the relative k of Formula (26), so the flag still has no effect on eobs.
  • Corpus. No tracked profile applies a reflectance or observer element. The only tracked eobs/robs are two parse-rejection fixtures in .github/ci/test-data/.

Pre-flight

lane result
ASan+UBSan Debug, clang 18, detect_leaks=1 red/green above; 0 sanitizer reports, 0 leaks
Release, clang 18, full ctest -j4 265 / 266. The one failure is iccdev.spectral-tiff-preview: ModuleNotFoundError: No module named 'imagecodecs' on this machine, raised before any iccDEV code runs
Strict clang 21.1.3, -Wall -Wextra -Wpedantic -Werror strict warnings ENABLED, 0 warnings
GCC 15.2.0 strict + LTO, ghcr.io/internationalcolorconsortium/iccdev:latest, ci-pr-gcc15 configure 0 warnings, 0 errors; new test and iccdev.reflectance-observer-illum-range pass
ci-pr-action, ci_scope=source run 34552413648 at c9e66037: success, 14 jobs green. iccdev.spectral-absolute-flag passed in the job logs of Windows MSVC, Windows ClangCL, MinGW UCRT64 and GCC Strict ASAN+UBSAN Debug (262 / 262)

…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.
@github-actions github-actions Bot added Documentation Documentation-only or documentation-related change Testing CTest, regression, or test coverage Source C or C++ source code changes Configuration Repository, CMake, YAML, JSON, or tool configuration Build Build system, CMake, compiler, or packaging labels Sep 11, 2026
@xsscx xsscx self-assigned this Sep 11, 2026
@github-actions github-actions Bot added the pending CI checks still running label Sep 11, 2026
@xsscx xsscx added this to the v2.3.2.4 milestone Sep 11, 2026

@xsscx xsscx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2026-09-11 02:06:58 UTC

@xsscx
xsscx enabled auto-merge (squash) September 11, 2026 02:07
@xsscx
xsscx merged commit ba2b13b into master Sep 11, 2026
37 checks passed
@xsscx
xsscx deleted the fix/2520-spectral-absolute-flag branch September 11, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Build system, CMake, compiler, or packaging Configuration Repository, CMake, YAML, JSON, or tool configuration Documentation Documentation-only or documentation-related change pending CI checks still running Source C or C++ source code changes Testing CTest, regression, or test coverage

Projects

None yet

2 participants