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

CMYK roundtrip via pipelineColourspace should not convert to sRGB internally #3620

Closed
3 tasks done
Aminelahlou opened this issue Apr 11, 2023 · 6 comments
Closed
3 tasks done

Comments

@Aminelahlou
Copy link

Aminelahlou commented Apr 11, 2023

Possible bug

With the code I provided in this repo https://github.com/Aminelahlou/sharpCmykTest we can see that color conversion happens even if I stated that pipelinecolorspace is cmyk and to colorspace is cmyk.

when I launch my script (as described below and in the git repo I made, I should get an image full of C:100 M:0 Y:0 K:0 values (same values than input image)

But here is the current behaviour:
When I launch node test-sharp.js I get a new image with values of C: 66 M: 18 Y:0 K:1

maybe I don't understand the docs but it seems sharp is doing a conversion here (either CMYK => (s)RGB => CMYK or CMYK=> CMYK) maybe there is also something I didn't understand in the docs but i have searched in issue 218 and did not found any answer.

Is this a possible bug in a feature of sharp, unrelated to installation?

  • Running npm install sharp completes without error.
  • Running node -e "require('sharp')" completes without error.

If you cannot confirm both of these, please open an installation issue instead.

Are you using the latest version of sharp?

  • I am using the latest version of sharp as reported by npm view sharp dist-tags.latest.

npm view sharp dist-tags.latest gives me 0.32 and in package-lock:

"sharp": {
"version": "0.32.0"
...
}

If you cannot confirm this, please upgrade to the latest version and try again before opening an issue.

If you are using another package which depends on a version of sharp that is not the latest, please open an issue against that package instead.

What is the output of running npx envinfo --binaries --system --npmPackages=sharp --npmGlobalPackages=sharp?

npx: installed 1 in 3.391s

  System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 Intel(R) Core(TM) i7-9700 CPU @ 3.00GHz
    Memory: 4.07 GB / 15.81 GB
  Binaries:
    Node: 14.15.1 - C:\Program Files\nodejs\node.EXE
    npm: 6.14.8 - C:\Program Files\nodejs\npm.CMD

What are the steps to reproduce?

Code:

const sharp = require("sharp");

const sharpInstance = sharp("./input/test-sharp-input.tif")
  .pipelineColourspace("cmyk")
  .toColourspace("cmyk")
  .toFile("./output/test-sharp-output.tif");

"./input/test-sharp-input.tif" contains a tiff image without any profile and full of CMYK pixel with Cyan value of 100 (other values are 0)
I have made a public repository at https://github.com/Aminelahlou/sharpCmykTest

What is the expected behaviour?

I should not make any color conversion wether SRGB=> CMYK CMYK=> CMYK or CMYK => SRGB.
and when I launch npm i and then node test-sharp.js I should get a new image in the output folder with exact same value which are Cyan:100 M:0 Y:0 K:0

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem

https://github.com/Aminelahlou/sharpCmykTest

const sharp = require("sharp");

const sharpInstance = sharp("./input/test-sharp-input.tif")
  .pipelineColourspace("cmyk")
  .toColourspace("cmyk")
  .toFile("./output/test-sharp-output.tif");

Please provide sample image(s) that help explain this problem

sample image is in the github repo in the input folder and is called test-sharp-input.tif

@lovell
Copy link
Owner

lovell commented Apr 11, 2023

Thanks for reporting, commit b763801 fixes this and adds a unit test that would previously have failed.

It looks like the "This feature is experimental..." warning for pipelineColourspace() will stay in place for a bit longer ;)

https://sharp.pixelplumbing.com/api-colour#pipelinecolourspace

@lovell lovell changed the title Color conversion and cmyk pipelinecolorspace/tocolorspace CMYK roundtrip via pipelineColourspace should not convert to sRGB internally Apr 11, 2023
@lovell lovell added this to the v0.32.1 milestone Apr 11, 2023
@Aminelahlou
Copy link
Author

Aminelahlou commented Apr 12, 2023

Hello,
Thanks for your very quick answer! I have other similar questions/suggestions when it comes to colorSpace/pipelineColorspace:

1- Can I use modulate function on a cmyk image without converting to srgb?
2- Can I extract embedded data of the profile to better understand what the profile would do ? Taking into account the specification of ICC File format : https://www.color.org/icc32.pdf. It would really help many people I believe to better understand what a profile is actually doing. Extracting profile data (i.e. conversions matrices) and enabling to test colorspace conversions (not only CMYK) would really help.
3- Should'nt toColorSpace function always use explicit argument on what profile is actually used to make the conversion? I find it very weird that withMetadata is the actual function that controle what profile we use for conversion.
4- Does pipeline colorspace work only with CMYK or all types of colorsspaces (lab for exemple)
Thanks again for your amazing work
If I may help to make any/all of these points work, please let me know. Even though I am no expert in c (not even intermediate level tbh), I have come to get some understanding on color conversion using profiles, nodejs, printing, spectrophotometers.

@lovell
Copy link
Owner

lovell commented Apr 17, 2023

  1. Commit 5f8646d adds support for this, which will also be in v0.32.1
  2. You may be interested in https://github.com/lovell/icc
  3. It might make sense to allow this for only the scenario where CMYK is either input or output colourspace, yes. If a CMYK input image has an embedded profile, which is highly recommended, then this will already be used.
  4. It should work with any colourspace, but it is still experimental and there are likely to be some operations that don't yet work with it.

@lovell
Copy link
Owner

lovell commented Apr 27, 2023

v0.32.1 now available with these fixes.

@lovell lovell closed this as completed Apr 27, 2023
@Aminelahlou
Copy link
Author

I have checked resize and modulate it works perfectly fine! thanks for your amazing work !

@Aminelahlou
Copy link
Author

I have listened to you and checked the icc package. I started working on a pr on this package following this issue : lovell/icc#2

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

No branches or pull requests

2 participants