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

FEATURE: Automatic conversion of images from CMYK into RGB colorspace #376

Merged
merged 9 commits into from Apr 26, 2016

Conversation

dlubitz
Copy link
Contributor

@dlubitz dlubitz commented Feb 18, 2016

Images in CMYK colorspace will be converted into RGB colorspace during the processing of images. The original image will still been kept in the original colorspace.

The conversion could be disabled by setting the configuration TYPO3.Media.image.convertCMYKToRGB to false.

Not applicable when using GD as it doesn't support CMYK.

NEOS-1778 #close

@hlubek
Copy link
Contributor

hlubek commented Feb 18, 2016

Nice

👍 by reading

@dfeyer
Copy link
Contributor

dfeyer commented Feb 18, 2016

Nice one, but not sure, why we don't enable it by default ... it's on master, and 99.99 % of Neos project are web centric, and it make no sense to publish CMYK image on the web. We need to document how to disable it for the 0.1% from my POV.

@dlubitz
Copy link
Contributor Author

dlubitz commented Feb 18, 2016

I would also set it as active by default. Had discussed before with @kitsunet, but he suggested to set inactive by default because it's new.

@hlubek
Copy link
Contributor

hlubek commented Feb 19, 2016

I imagine that the only case where this is a problem would be a download offering of a CMYK image (e.g. image for print). That could easily be handled by offering the original resource. But it would be a breaking change in this regard (would have to test the case, though).

What I'd question when thinking more about this is, that global settings are not enough for image conversion / quality parameters. We always had problems in TYPO3 projects because we wanted different image quality for different use cases. So if we could override the global image options in the actual use of Image * Helpers that would be a very useful new feature where we could easily use the RGB conversion as a default because it's easy to override in any special case. I'll create a new issue about the override feature in JIRA.

@dlubitz
Copy link
Contributor Author

dlubitz commented Feb 19, 2016

Also thought abhout similar, but thougt the impact is very high. E.g. you need to persist it for the thumnails. But like this, we have also some places where I would e.g. reduce the image quality if I could.

In this case, it's maybe a better idea to move the option convertCMYKToRGB into the defaultOptions config path?

Convert CMYK images into RGB
============================

If you are working with CMYK images and like to convert them automatically for web usage, you can activate this with in your Settings.yaml:
Copy link
Member

Choose a reason for hiding this comment

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

with in -> within

@daniellienert
Copy link
Member

Nice improvement. Reading the previous comments, I would vote for making it active by default, as I also assume that you seldom want to have generated CMYK images.
I also think, it fits into the defaultOptions.

@@ -0,0 +1,28 @@
==========================
Copy link
Contributor

Choose a reason for hiding this comment

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

This documentation should be added to the media package documentation instead of the Neos documentation. See https://github.com/neos/neos-development-collection/tree/master/TYPO3.Media/Documentation (http://neos-media.readthedocs.org)

@aertmann
Copy link
Contributor

Hey Denny. Nice one, thanks! +1 for making it default and part of defaultOptions from my side too. Left a comment about where the documentation is added, which I think should be fixed before merging.

Also adjusted the commit comment a little since it's used in the changelog. More information can be found at https://discuss.neos.io/t/commit-message-style/507

It would be good if someone would confirm this on Gmagick as well (already tested on Imagick and GD).

@dlubitz dlubitz changed the title FEATURE: Option to automatically convert images from CMYK into RGB colorspace FEATURE: Automatic convertion of images from CMYK into RGB colorspace Feb 28, 2016
@dlubitz dlubitz changed the title FEATURE: Automatic convertion of images from CMYK into RGB colorspace FEATURE: Automatic conversion of images from CMYK into RGB colorspace Feb 28, 2016
@dlubitz
Copy link
Contributor Author

dlubitz commented Feb 28, 2016

Done.

I've moved the documentation into the Media package (Documentation root):
Packages/Neos/TYPO3.Media/Documentation/ConfigureImageGeneration.rst

I think it needs to be integrated into the general documentation structure.

@aertmann
Copy link
Contributor

aertmann commented Mar 2, 2016

@dlubitz thanks, just needs to be added to the index.rst in the documentation folder to show up in the overview

@dlubitz
Copy link
Contributor Author

dlubitz commented Mar 9, 2016

Documentation is added to the index of media packages documentation.

@aertmann
Copy link
Contributor

aertmann commented Mar 9, 2016

thanks @dlubitz, looks good 👍 from my side

@aertmann aertmann added the T: PHP label Mar 9, 2016
@kdambekalns kdambekalns merged commit 6121964 into neos:master Apr 26, 2016
@dlubitz dlubitz deleted the NEOS-1778_CMYK-to-RGB branch April 26, 2016 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants