Skip to content
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.

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

colour: improve ICC compatibility check for CMYK images #3565

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

kleisauke
Copy link
Member

See: #3564.

Targets the 8.14 branch.

@jcupitt
Copy link
Member

jcupitt commented Jul 16, 2023

This looks too easy! I can't help feeling there's some corner case we've overlooked, but I don't know what it is.

@kleisauke
Copy link
Member Author

As well as checking it against the test case in #3564, I also checked it against issue #3139 and the test cases in PR #3372. But indeed, perhaps we overlooked something, I will verify some old issues.

@jcupitt
Copy link
Member

jcupitt commented Jul 16, 2023

I checked it against CMYKs with sRGB profiles, RGBs with CMYK profiles, and mono images with RGB profiles, and it all seems fine. I didn't try alpha channels.

I think maybe I'm only uneasy because I remember changing this logic before and breaking everything :( It's probably fine.

@kleisauke
Copy link
Member Author

The test case mentioned in #730 (comment) that was fixed with PR #3139 also still produces the expected behavior.

I'll do some more testing with CMYK images, since that's the only behavior change in this PR (the change in commit bec4942 is also a behavior change, but that's much more uncommon, I think).

@kleisauke
Copy link
Member Author

I've double-checked it with a couple of CMYK images with ICC profiles and didn't encounter any issues. Let's merge.

@kleisauke kleisauke merged commit ee74035 into libvips:8.14 Jul 19, 2023
4 checks passed
@kleisauke kleisauke deleted the issue-3564 branch July 19, 2023 14:57
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.

None yet

2 participants