Skip to content

openpdf-renderer: resolve named /ColorSpace resources (ICCBased, CalGray, CalRGB, Lab) for cs/CS/scn/SCN - #1605

Open
andreasrosdalw wants to merge 4 commits into
LibrePDF:masterfrom
andreasrosdalw:feature/renderer-colorspace-resolution
Open

openpdf-renderer: resolve named /ColorSpace resources (ICCBased, CalGray, CalRGB, Lab) for cs/CS/scn/SCN#1605
andreasrosdalw wants to merge 4 commits into
LibrePDF:masterfrom
andreasrosdalw:feature/renderer-colorspace-resolution

Conversation

@andreasrosdalw

@andreasrosdalw andreasrosdalw commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cs/CS in OpenPdfCorePageRenderer previously only recognized literal device color-space names (/DeviceGray, /DeviceRGB, /DeviceCMYK and abbreviations). Any other name — most commonly a named ICCBased profile like /CS0 declared in the page's /ColorSpace resource dictionary, which is how most real-world PDF producers actually declare RGB/CMYK color spaces — fell into ColorSpaceKind.UNKNOWN, and colorFromOperands fell back to guessing gray/RGB/CMYK purely from the sc/scn operand count.
  • That heuristic happens to match ICCBased/CalGray/CalRGB by coincidence (operand count equals component count), so those cases mostly "worked" already. Lab is a real, visible bug: L*a*b* components (L in 0-100, a/b roughly -100..100) fed straight into rgb() after clamp01 collapse almost every Lab color to solid white or black instead of the intended color, since raw Lab values aren't RGB values.
  • Adds resolveColorSpaceResource() / classifyColorSpaceDefinition(), reusing the same array-classification approach imageComponents() already uses for Image XObjects (ICCBased-by-/N, CalGray, CalRGB), plus a new correct Lab → CIEXYZ → sRGB conversion (labToRgb), using the color space's own /WhitePoint when present and D50 otherwise. Verified the conversion against known reference values (Lab D50 encodings of pure red/green/black/white round-trip to the expected sRGB) before locking in test assertions.
  • Color spaces still not specifically understood (Separation, DeviceN, Indexed-as-fill, Pattern) keep falling back to the existing operand-count heuristic — unchanged behavior, out of scope for this PR.
  • Updated openpdf-renderer/README.md's color-space coverage section and the class-level Javadoc.

…for cs/CS/scn/SCN

cs/CS previously only recognized literal device color space names
(/DeviceGray, /DeviceRGB, /DeviceCMYK); any other name -- most commonly a
named ICCBased profile like /CS0 declared in the page's /ColorSpace resource
dictionary -- fell back to ColorSpaceKind.UNKNOWN, and colorFromOperands
guessed gray/RGB/CMYK from the sc/scn operand count. That heuristic happens
to match ICCBased/CalGray/CalRGB by coincidence (operand count equals
component count), but is a real correctness bug for Lab: L*a*b* values
(L 0-100, a/b roughly -100..100) fed straight into rgb() after clamping
collapse to solid white/black instead of the intended color.

Add resolveColorSpaceResource()/classifyColorSpaceDefinition(), reusing the
same array-classification approach imageComponents() already uses for
images, plus a proper Lab -> XYZ -> sRGB conversion (using the color space's
own /WhitePoint when present, D50 otherwise). Update the README to document
the new capability.
@codacy-production

codacy-production Bot commented Jul 28, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity · 0 duplication

Metric Results
Complexity 9
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

- Reduce colorFromOperands's cognitive complexity below the required
  threshold by extracting the numeric-operand counting loop and the
  unknown-color-space operand-count heuristic into their own methods, and
  using ternaries for the direct GRAY/RGB/CMYK/LAB cases instead of
  if+break.
- labWhitePointFrom() and every ColorSpaceInfo/GState call site now use a
  shared NO_LAB_WHITE_POINT empty-array sentinel instead of returning/
  storing null for "no white point", per S1168.
- ColorSpaceInfo now overrides equals/hashCode/toString itself (via
  Arrays.equals/hashCode/toString) since a record's generated versions
  compare an array component by reference rather than content (S6218).
- Move NO_LAB_WHITE_POINT / DEFAULT_LAB_WHITE_POINT / CIE_SRGB next to the
  other color-space constant fields at the top of the class instead of
  declaring them inline next to their first use.
- Wrap the README's Colors operator-table row under the 120-char line
  length limit.
andreasrosdalw added a commit to andreasrosdalw/OpenPDF that referenced this pull request Jul 28, 2026
Consolidated into LibrePDF#1607 along with the README updates for LibrePDF#1604, LibrePDF#1605,
and LibrePDF#1606, so these four independent PRs (all based on master) don't each
carry their own overlapping README.md diff and risk merge conflicts with
each other depending on merge order.
andreasrosdalw added a commit to andreasrosdalw/OpenPDF that referenced this pull request Jul 28, 2026
Consolidated into LibrePDF#1607 along with the README updates for LibrePDF#1603, LibrePDF#1605,
and LibrePDF#1606, so these four independent PRs (all based on master) don't each
carry their own overlapping README.md diff and risk merge conflicts with
each other depending on merge order.
Consolidated into LibrePDF#1607 along with the README updates for LibrePDF#1603, LibrePDF#1604,
and LibrePDF#1606, so these four independent PRs (all based on master) don't each
carry their own overlapping README.md diff and risk merge conflicts with
each other depending on merge order.
andreasrosdalw added a commit to andreasrosdalw/OpenPDF that referenced this pull request Jul 28, 2026
Consolidated into LibrePDF#1607 along with the README updates for LibrePDF#1603, LibrePDF#1604,
and LibrePDF#1605, so these four independent PRs (all based on master) don't each
carry their own overlapping README.md diff and risk merge conflicts with
each other depending on merge order.
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant