Support reading CMYK, YCCK JPEGs #1246
Conversation
src/Image.cc
Outdated
| if (_data) { | ||
| delete[] _data; | ||
| _data = nullptr; | ||
| } |
There was a problem hiding this comment.
Nice 🙌
btw. I think it's safe to run delete on a nullptr, so I don't think you need the if? 🤔
There was a problem hiding this comment.
Yeah, it's defined behavior. Removed the guard.
| img2.onerror = function () { | ||
| done(new Error('Failed to load image')) | ||
| } | ||
| img2.onerror = done |
zbjornson
force-pushed
the
zb/cmykjpg
branch
from
da397df to
98f5262
Compare
|
Is there a way to fix color space (or whatever gives too much colors)? I tried this PR and got following: Correct: Browser (newest Chrome)
Wrong: Node-Canvas:
Actual image (25MB): https://drive.google.com/open?id=1LttH_DNl_Y5Dap__hQ88MhHqqb2YgJfe As you can see, the colors of the right image is a bit too colorfull. Any idea how I can debug? |
|
@marcj the EXIF data for that JPEG says it doesn't have an embedded color space, so I think that can be ruled out. Hard to say what layer that difference comes from... depending on how you got the images you embedded in your post (were they screenshots?) it could even be Chrome's display rendering vs. the MacOS Preview app or something. |
|
Its both a screenshot. The (wrong) image from node-canvas looks across multiple viewer the same, so I assume node-canvas does not read the CMYK file correctly. I use now sharp (https://github.com/lovell/sharp) to convert the CMYK image to RGB first before using it in node-canvas, which looks very good. However, I'd not like to do that as it's rather slow. |
libjpeg automatically converts YCCK to CMYK, so we get that for free by adding CMYK support. Also adds a test for grayscale and does some cleanup.
Fixes #425