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
Add support for read and write of Photoshop Color Swatches #589
base: master
Are you sure you want to change the base?
Conversation
Contains utility functions for handling Adobe formats since they typically are in binary formats with big-endian byte encoding.
Copic 358 Color Swatch Photoshop.zip I tested with above palettes and parser doesn't seem to work with them. |
Sorry about that, I had a typo my last commit. I forgot to double check loading after refactoring. Fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I'm getting an IndexOutOfRangeException when saving any palette as .aco. The
colorValues
array is created with a size of 3 but a few lines later the code attempts to update it's 4th element. -
Despite of the error message being displayed after the exception is caught, a (broken) file is still written to the hard drive.
-
I exported a few of the default palettes from photoshop an imported them into PixiEditor, and some of them didn't get loaded correctly. The issue seems to be related to non-rgb color spaces. Here are the files:
testpalettes.zip
In paint samples.aco
a yellow (#ffba01 according to photoshop) HSB color is interpreted as pure white, this is likely because you are using a function that works with HSL colors (HSL and HSB aren't the same color spaces).
All other palettes aren't loaded correctly, the colors are wrong.
- It seems like you are using code from this page as reference. It's totally fine, but if I'm correct could you please add that link somewhere in the code. This applies to other PRs as well
Thanks for reviewing! I forget to change the array initialization in the save function as well. Adobe formats requires four color values to be written out. :P As for the color spaces, you are absolutely right. It's easy to get those color spaces mixed up sometimes. HSB only carries the blackness, while HSL carries both white and black . Very similar but slightly different calculations to RGB. I noticed there is an issue too with loading your test palettes from a LAB color space. Every color is being translated to black. I'll have to investigate this, as it was working with my test files. May have to revise my Also thank you pointing out the missing reference, I overlooked those those in the file headers. Will get them all added in ASAP. |
…iEditor into feat-aco-format
Removed HSL since it's not used by Adobe and rewrote HSB/LAB/XYZ conversion functions. Streamlined into single class with only required RGB conversion functions.
@Equbuxu Mind looking at that? I don't have Photoshop atm :x |
@flabbet Sorry, forgot to update the status after my changes. I also don't have Photoshop, relying on GIMP for testing. @Equbuxu The new conversion code appears ot be working correctly with CMYK and LAB. HSB looks to be working too except for that one color you mentioned is still off. I can't seem to figure out what is going on. Doesn't make sense. I checked my HSB algorithm and it's inline with other implementations I've found. |
I've pushed some changes, fixing some of the issues:
Note: I tried importing the palettes into gimp, and it skips some of the colors (I think Lab colors are the ones it skips?). The CMYK colors are also different compared to photoshop, so GIMP doesn't seem like the best reference Useful links: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
Thanks for taking a look and fixing! I finally installed a trial of Photoshop and can see where the LAB and CMYK colors are still quite a bit off. I tried adjusting the calculations using new white points for D50 10° (still pretty sure it uses 2° though), but the corresponding colors still were off from Photoshop. I think this largely has to do with the fact that its CMS handles all color conversions. I was hoping doing a manual conversion would work but I honestly don't think this is the best approach now. It would be better to implement a color manage system to achieve accurate color reproduction, particularly with LAB and CMYK color spaces. I'm going to do a bit of experimenting with Windows ICM to see if I can achieve better results. I would suggest going with Little CMS though, since it's pretty robust and open source. This is the same CMS Krita uses. I can even create .NET bindings if needed since it's a C library. |
Something really important I forgot to mention, you MUST change the color space of your current document in photoshop from Adobe RGB (the default for some reason) to sRGB 61966-2-1:1999. I don't know why photoshop defaults to their own RGB (even for pngs created in other apps, resulting in them looking different in photoshop), but it causes A LOT of the colors to be off. In my last commit, all Lab colors I tested lined up, except for a single one, which was off by one (if I recall correctly it was a green color from |
I haven't heard about Windows ICM, but since we are going cross platform in #573, please don't introduce new dependencies to stuff that only runs on windows (of course I'm only assuming that Windows ICM is windows-specific). I also haven't heard about Color management systems in general, but on the surface level it does sound like something that could be useful both here and in other places in the app |
The current version of Photoshop already defaults to that sRGB working space on my system, but still seeing colors off. I have some experience working with color management systems, so familiar with ICC profiles. I was only suggesting Windows ICM for testing as it's easier to use, but I would use Little CMS as an application-wide cross platform solution in the future. This is a good use anywhere there is color conversion being performed. If we want to move forward with ACO support, I can simply skip LAB and CMYK color mode support for the time being. |
I see, interesting
I'd say it would be better to implement the format fully, but it's open to discussion
This sounds like a good idea to me, @flabbet what do you think By the way, almost all development discussions happen on our discord server (in a less formal way, in chat), so if you use discord, you should join the server, everyone will see messages and respond a lot quicker there |
Using proper manager for color management sounds good, although I can't find any info about Windows ICM or Little LMS? Do you mind sharing some resources? |
https://littlecms.com/color-engine/ (recommended since it's not windows dependent) |
@Equbuxu I did quite a bit of experimenting with CMYK and was able to improve the algorithm and achieve what I believe to be quite a bit closer results to PS without use of an ICC profile. But the colors will never match 1:1 even with profiles because of the nature of the color spaces. |
No description provided.