Skip to content

Update color space transformations - #469

Open
CPBridge wants to merge 1 commit into
v0.29.0devfrom
bug/d50_color_transformations
Open

CPBridge wants to merge 1 commit into
v0.29.0devfrom
bug/d50_color_transformations

Conversation

@CPBridge

@CPBridge CPBridge commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Addresses #461

Generated almost entirely by Claude by pointing it at the DCMTK diff here and telling it to make the corresponding changes to the highdicom implementation.

It all seems reasonable to me, but I don't really understand this stuff. @fedorov @michaelonken anything look wrong to you, or further ideas to test? At a minimum I will wait until dcmtk changes are released I think

What follows is Claude's summary of what it did:

What changed

The old code converted sRGB → XYZ → Lab using the D65 white point throughout. DICOM encodes CIELab the same way as the ICC Profile Connection Space (PS3.3 C.10.7.1.1), which is D50-relative, so the chromatic adaptation step was missing.

The pipeline is now sRGB → linear RGB → XYZ (D65) → Bradford adaptation → XYZ (D50) → Lab, and the reverse. Specifically:

- Added _D50_WHITEPOINT_{X,Y,Z} from the CSS Color 4 chromaticities; the old hardcoded 95.047 / 100.0 / 108.883 D65 divisors are gone.
- _rgb_to_xyz / _xyz_to_rgb use the exact CSS Color 4 rational matrices and the linear Bradford adaptation matrices, replacing the 4–5 significant-figure matrices.
- XYZ is now on a 0–1 scale (D50-relative) rather than 0–100, matching dcmtk. These are private functions used only inside color.py.
- labf/labfInv now use the exact CIE constants (ε = 216/24389, κ = 24389/27) instead of the truncated 841/108, 7.787 and 0.008856 forms. Note the old code was asymmetric: _xyz_to_lab used the correct linear segment but _lab_to_xyz used the legacy 7.787-based inverse, which didn't exactly invert it.
- sRGB transfer thresholds normalized to the standard 0.04045 / 0.0031308.

Consistency with dcmtk

I validated against every assertion in dcmtk's new test file:

- All 4 scalar XYZ/Lab checks (gray 0.5 → L*=53.389, a*=b*=0; XYZ → 0.206/0.214/0.177; ICC D50 → white).
- All 40 reference colors match dcmtk's CSS Color 4 values within its 0.03 tolerance, in both directions.
- White/black encode to exactly (0xFFFF, 0x8080, 0x8080) and (0x0000, 0x8080, 0x8080).
- The three PixelMed-consistency cases produce dcmtk's exact RGB.
- All 16,777,216 RGB triples round-trip exactly through the 16-bit DICOM encoding — dcmtk's headline guarantee. I ran the exhaustive test, not just the sampled one.

One deliberate divergence

dcmtk's xyz2Rgb now clamps to [0,1] internally. I left _xyz_to_rgb unclamped, because CIELabColor.to_rgb() relies on out-of-range values to detect out-of-gamut colors and raise ValueError (or clip on request) — that's highdicom's existing public contract, separate from the D65/D50 question. Where dcmtk clips, to_rgb(clip=True) returns the identical result; I added tests pinning the three dcmtk/PixelMed cases to confirm this.

Also updated

- Doctests in CIELabColor — the output values shift. Fixed a >>>> typo that was silently disabling one of them.
- tests/test_color.py — the old expected values were sourced from colormine.org (D65). Replaced with the D50 values and added the 40-color reference table, white-point, out-of-gamut, and round-trip tests derived from dcmtk's suite.
- docs/release_notes.rst — a migration note, since stored CIELab values in files written by highdicom will now differ slightly for chromatic colors (neutrals are unaffected).

tests/test_color.py, test_pr.py, test_ann.py and test_seg.py all pass (968 tests); flake8 is clean and mypy reports the same 8 pre-existing errors as before, all in ColorManager.

@CPBridge CPBridge added the bug Something isn't working label Oct 6, 2026
@fedorov

fedorov commented Oct 6, 2026

Copy link
Copy Markdown
Member

Mike Halle @mhalle is the expert on this topic, in case he has cycles and interest...

@mhalle

mhalle commented Oct 7, 2026

Copy link
Copy Markdown

Looks like the right approach, and the result is that DICOM colors align with CSS lab() colors exactly.

I agree with your clip approach. How to deal with out of gamut colors is a whole art in itself. That's why in general, ideally, you should consider carrying the CIELab colors through the pipeline as far as possible rather than prematurely clipping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants