Skip to content

Add Full Support for JPEG Tiff PhotometricInterpretation.Separated #2937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add Full Support for JPEG Tiff PhotometricInterpretation.Separated #2937

wants to merge 9 commits into from

Conversation

JimBobSquarePants
Copy link
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fixes #2454

@brianpopow did the hard stuff I just fixed the final color conversion. We support both default and ICC driven conversion with JPEG compressed TIFF files now.

Copilot
Copilot AI reviewed Jun 5, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds comprehensive TIFF "Separated" photometric interpretation support by extending JPEG-based TIFF decompression and color conversion to handle CMYK/YccK workflows.

  • Introduces Separated handling in spectral converters and decompression classes using IRawJpegData.ColorSpace.
  • Passes ICC profile metadata into JpegDecoderCore and updates JPEG decoder to apply embedded profiles.
  • Registers new TiffCmyk and TiffYccK converters in JpegColorConverterBase.

Reviewed Changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffOldJpegSpectralConverter{TPixel}.cs Added Separated mapping to select between YccK and CMYK color spaces.
src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffJpegSpectralConverter{TPixel}.cs Added Separated case and updated XML docs in GetJpegColorSpace.
src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs Injected ImageFrameMetadata and added Separated branch in switch.
src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs Injected ImageFrameMetadata and added Separated branch in switch.
src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs Extended constructor to accept ICC profile and set it on metadata.
src/ImageSharp/Formats/Jpeg/Components/ColorConverters/JpegColorConverterBase.cs Registered new TiffCmyk and TiffYccK converters; added helper method.
src/ImageSharp/Formats/Jpeg/Components/Encoder/SpectralConverter{TPixel}.cs Removed unused import and left TODO for stride conversion refactor.
Comments suppressed due to low confidence (5)

src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffOldJpegSpectralConverter{TPixel}.cs:42

  • Consider adding unit tests to cover the new handling of TiffPhotometricInterpretation.Separated in GetJpegColorSpaceFromPhotometricInterpretation to verify correct selection of TiffYccK vs TiffCmyk based on jpegData.ColorSpace.
TiffPhotometricInterpretation.Separated => data.ColorSpace == JpegColorSpace.Ycck ? JpegColorSpace.TiffYccK : JpegColorSpace.TiffCmyk,

src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffJpegSpectralConverter{TPixel}.cs:57

  • [nitpick] This TODO comment questioning why YCbCr maps to RGB could be clarified or removed—consider explaining the rationale or updating the mapping to use YCbCr if that was intended.
TiffPhotometricInterpretation.YCbCr => JpegColorSpace.RGB, // TODO: Why doesn't this use the YCbCr color space?

src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffJpegSpectralConverter{TPixel}.cs:34

  • [nitpick] Method name GetJpegColorSpace is less descriptive compared to GetJpegColorSpaceFromPhotometricInterpretation in the old converter—consider renaming for consistency.
JpegColorSpace colorSpace = GetJpegColorSpace(this.photometricInterpretation, jpegData);

src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs:79

  • A new case for TiffPhotometricInterpretation.Separated is added in the decompression switch; consider adding a test case to verify JPEG decompression path for separated images.
case TiffPhotometricInterpretation.Separated:

src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs:94

  • The Separated photometric interpretation is now handled here as well; ensure unit tests cover this new code path for JpegTiffCompression.
case TiffPhotometricInterpretation.Separated:

@JimBobSquarePants JimBobSquarePants changed the title Add Full Support for Tiff PhotometricInterpretation.Separated Add Full Support for JPEG Tiff PhotometricInterpretation.Separated Jun 5, 2025
gfoidl
gfoidl reviewed Jun 5, 2025
View reviewed changes
Copy link
Contributor

@gfoidl gfoidl left a comment

Choose a reason for hiding this comment

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

Same places to change division to multiplication.
I don't know if that transformation results in still accurate results, but I assume so.
Better names for constants (in my suggestion) can be used of course.

@JimBobSquarePants
Copy link
Member Author

Thanks @gfoidl I've changed everything to use multiplication instead.

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

@gfoidl gfoidl gfoidl left review comments

Copilot code review Copilot Copilot left review comments

@brianpopow brianpopow Awaiting requested review from brianpopow

Assignees
No one assigned
Labels
enhancement formats:tiff
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jpeg compressed tiff with photometric interpretation Separated is not supported
3 participants
@JimBobSquarePants @gfoidl @brianpopow