Fix broken png files: libpng 1.6.2 and newer has stricter iCCP rules

[davidovitch]

I am running on Arch Linux with libpng 1.6.16, and for a long time I have had the following error being spit out to the shell when running spyder from the command line:

libpng warning: iCCP: known incorrect sRGB profile

I never bothered to look into it, spyder just worked fine, until I wanted to figure it out today. Seems this is related broken *.png files used by spyder. The fix is trivial, and I found the same answer both at stackoverflow and tex.stackexchange:

# fix all png files in the current directory (and sub-dirs) using ImageMagick:
find . -type f -name "*.png" -exec convert {} -strip {} \;

The libpng warning dissapeared after the *.png file conversion for me. I have not tested these new *.png files with libpng versions prior to 1.6.2 (the version that is alleged to have stricter iCCP checkings).

[ccordoba12]

Thanks for your contribution. Before merging, I need to verify that this does not introduce problems with older libpng versions, as you mentioned :-)

[davidovitch]

I'll can try to test it on my system with different versions of libpng. What I will not be able to test is how this might affect the Windows and Mac environments. I would expect no problems since strictly speaking the png files should be cleaner at this stage compared to before. But I agree, better safe then sorry :-)

[ccordoba12]

Yes, please do. I'll do my part on Mac and Windows

[davidovitch]

Not an expert here, so not entirely sure, but I can't see how testing with older libpng versions (1.5 for instance) is relevant. The newer versions have stricter checks on testing if a png is compliant with the standard. Version 1.6 has more strict tests, so warns when files are not 100% according to the standard. I assume that this stricter checking would actually benefit older clients rather than the contrary. Again assuming here, but I would but very surprised indeed if png's created with libpng 1.6 would not be backwards compatible with older viewers.

Further, I am not even sure if libpng is actually used when they are rendered/shown in Spyder. I am under the impression it libpng comes into the picture when manipulating png's. See for instance Wikipedia for what libpng is supposed to do.

That being said, I am not sure testing for me is worth the trouble: I would have to recompile Qt using an older libpng version in order to test this. I think it would be sufficient for any other user with a libpng version prior to 1.6 (being 1.5, 1.4, ...) to see if these png images still work:

git clone git@github.com:davidovitch/spyder.git
cd spyder
python bootstrap.py

That sounds like much simpler testing path :-)

[Nodd]

While you're modifying the icons, could you try to use optipng on them ? it optimizes them without losing any information, I've used this program successfully many times.
It's not a big change, but reducing many icon size can speed up the startup a little bit maybe (or I'm too much a perfectionnist).

[davidovitch]

@Nodd, interesting suggestion, and I gave it a spin. For most PNG's, using:

find . -type f -name "*.png" -exec optipng -o7 {} \;

resulted in a 30% decrease in size. I have not compared all the results visually one on one, but a visual inspection of a running Spyder instance looks fine for me. Additionally, used Github's Image view modes to compare some of them head on, and couldn't see any difference at all.

@Nodd: while you are also at it, just in case you have an libpng version prior to 1.6, can you confirm these png's still all work properly?

[davidovitch]

These optimized png's consume approximately 1MB less of my disk space. I don't know if this has any effect, but at least there leaner now :-)

[Nodd]

I'll try tomorrow, we have some old hardware at work !

[goanpeca]

@davidovitch , @Nodd any progress on this one?

[davidovitch]

@goanpeca Not from my part. I think some more images have been added and I still have to add them. I hope to get back to this later this week or next week.

[goanpeca]

If this is something that we would like to do periodically to keep images in spyder clean, then maybe it would be nice to have such script inside the repo.

@ccordoba12 ?

[davidovitch]

That would makes sense. I am not sure who generates the images on what platform and with which versions. I have now used ImageMagick for the conversion on Linux. I will have to look into how to avoid this issue in the future (which will depend on what software is used on which platform). Maybe I create a unit test case that all images should be able to pass.

[Nodd]

I have problems cloning the repo on the machine I was talking about. I'm in favor of merging it because imagemagick is a well-know and stable program and I don't see any reason why the new png images would cause any problem. They are displayed fine in github for example !

The script should go to https://github.com/spyder-ide/spyder/tree/master/img_src

[ccordoba12]

@davidovitch, please add your script to the location mentioned by @Nodd.

Then I'll merge :-)

[Nodd]

@davidovitch, any updates ? Just a small script !
If you don't have the time just say it, I'll make the script myself. The libpng warnings are annoying when trying to spot errors in plugins.

[davidovitch]

@Nodd, sorry for the delays. I've named it png-convert-opt.sh. Is that an appropriate name for it? This script is Linux only, I haven't looked into a Windows equivalent. I guess it is not really that important to be able to do this on any platform, but if someone wants to have a stab at it please do (I won't ;-).

[Nodd]

Linux only is fine, thanks.
Could you use something like cd $(dirfile $0)/.. to be sure to run the script from the correct directory ?

[davidovitch]

You should run from the top level spyder repository directory in order to be sure to catch all png files. Since it will recursively run through all the sub directories, this might cause some unwanted side effects if you run it by accident from, for instance, your home directory. I think it is fair to assume the developer knows what he/she is doing.

I am not sure how to always browse to the top level spyder directory. Could you explain what you mean with dirfile, I am afraid you lost me here...

[Nodd]

$0 is the name of the script and dirname returns the directory of the argument, so dirname $0 is the directory img_src and you have to go one birectory up, hence the /.. at the end.
Edit: dirfile from my previous comment is actually dirname. sorry for the confusion.

[davidovitch]

Ah ok, I still have to learn a lot of the regular bash magic :-) Thanks for explaining.

[davidovitch]

@ccordoba12 I think this is about to be ready for merge. Do you want me to clean up and rebase + squash into fewer commits? Any more comments/suggestions?

[Nodd]

Does this conflict with #2260 ? If not, we should merge it.
If I'm not mistaken Anaconda updated the libpng version so lots of people will see the warnings, so maybe it should be backported for 2.3.5.

[SylvainCorlay]

Yes, it should conflict with the last commit of #2260.

[davidovitch]

rebased on upstream/master, there was one merge conflict, and I adopted upstream version (deleted image). Let me know if you want this squashed into 2 commits: one for the re-formatted images, and one for the utility conversion script.

[ccordoba12]

@davidovitch, thanks for the fix. Now that Anaconda has moved to libpng 1.6 we also need this fix in the stable branch.