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

[Qt] optimize PNG files #5489

Closed

Conversation

jonasschnelli
Copy link
Contributor

According to report on #5488 there can be libpng warnings while starting bitcoin-qt.
This PR will strip out unnecessary data (especially color profiles) from all PNG files.

@laanwj
Copy link
Member

laanwj commented Dec 16, 2014

Can you give the exact command line used? I'd like to reproduce this.

@jonasschnelli
Copy link
Contributor Author

i used convert -strip <filename>.png <filename>.png.

@paveljanik
Copy link
Contributor

Doesn't print the warning as the master itself.
For the reference, the warning was:

libpng warning: iCCP: known incorrect sRGB profile

@sipa
Copy link
Member

sipa commented Dec 17, 2014

The result of that command is not identical here. Running the command on top of your output gives an even different result.

@jonasschnelli
Copy link
Contributor Author

Analyzed.
It needs two (or even three runs). After then that file SHA1 keep the same.
I also provided a script under ./contrib/strip_pngs.sh

I also assume it could depend on installed imagemagick and libpng version.

Here you can see the process of one file.
The DIFF is from imagemagicks identify -verbose
(full bash dump of all files is here: http://paste.ubuntu.com/9548096/)

Jonass-MacBook-Pro-2:bitcoin jonasschnelli$ convert --version
Version: ImageMagick 6.8.9-8 Q16 x86_64 2014-10-23 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2014 ImageMagick Studio LLC
Features: DPC Modules
Delegates: bzlib freetype jng jpeg ltdl lzma png xml zlib

Jonass-MacBook-Pro-2:bitcoin jonasschnelli$ contrib/strip_pngs.sh src/qt/res/icons
stripping ./about.png
SHA1 before: 
SHA1(./about.png)= e6f23d98c69892f0a26613c23a68a610030f745b
SHA1 after: 
SHA1(./about.png)= 8b56adeec521bba7bc6d49e7dc54f96216af0f7f

DIFF:
8c8
<   Base type: GrayscaleAlpha
---
>   Base type: Grayscale
532,538c532,533
<   Rendering intent: Perceptual
<   Gamma: 0.454545
<   Chromaticity:
<     red primary: (0.64,0.33)
<     green primary: (0.3,0.6)
<     blue primary: (0.15,0.06)
<     white point: (0.3127,0.329)
---
>   Rendering intent: Undefined
>   Gamma: 1
544c539
<   Intensity: Undefined
---
>   Intensity: Rec709Luminance
552,553c547,550
<     date:create: 2014-12-17T08:51:59+01:00
<     date:modify: 2014-12-17T08:51:59+01:00
---
>     date:create: 2014-12-17T08:52:20+01:00
>     date:modify: 2014-12-17T08:52:20+01:00
>     png:bKGD: chunk was found (see Background color, above)
>     png:cHRM: chunk was found (see Chromaticity, above)
556,557c553,554
<     png:IHDR.color-type-orig: 6
<     png:IHDR.color_type: 6 (RGBA)
---
>     png:IHDR.color-type-orig: 4
>     png:IHDR.color_type: 4 (GrayAlpha)
560,561d556
<     png:sRGB: intent=0 (Perceptual Intent)
<     png:text: 1 tEXt/zTXt/iTXt chunks were found
563d557
<     Software: Adobe ImageReady
567,568c561,562
<   Tainted: True
<   Filesize: 5.92KB
---
>   Tainted: False
>   Filesize: 4.35KB

Jonass-MacBook-Pro-2:bitcoin jonasschnelli$ contrib/strip_pngs.sh src/qt/res/icons
stripping ./about.png
SHA1 before: 
SHA1(./about.png)= 8b56adeec521bba7bc6d49e7dc54f96216af0f7f
SHA1 after: 
SHA1(./about.png)= 4a7719bc47e073de262fddf940e774712d999c8e

DIFF:
547,548c547,548
<     date:create: 2014-12-17T08:52:20+01:00
<     date:modify: 2014-12-17T08:52:20+01:00
---
>     date:create: 2014-12-17T08:52:34+01:00
>     date:modify: 2014-12-17T08:52:34+01:00
550c550
<     png:cHRM: chunk was found (see Chromaticity, above)
---
>     png:gAMA: gamma=1 (See Gamma, above)
562c562
<   Filesize: 4.35KB
---
>   Filesize: 4.32KB


Jonass-MacBook-Pro-2:bitcoin jonasschnelli$ contrib/strip_pngs.sh src/qt/res/icons
stripping ./about.png
SHA1 before: 
SHA1(./about.png)= 4a7719bc47e073de262fddf940e774712d999c8e
SHA1 after: 
SHA1(./about.png)= 4a7719bc47e073de262fddf940e774712d999c8e

DIFF:
547,548c547,548
<     date:create: 2014-12-17T08:52:34+01:00
<     date:modify: 2014-12-17T08:52:34+01:00
---
>     date:create: 2014-12-17T08:52:42+01:00
>     date:modify: 2014-12-17T08:52:42+01:00

@laanwj
Copy link
Member

laanwj commented Dec 17, 2014

OK this freaks me out. My output differs from yours. From your diff above I thought the problem was an embedded timestamp, however I get no different result from

    export LD_PRELOAD=/usr/lib/x86_64-linux-gnu/faketime/libfaketime.so.1
    export FAKETIME='2012-12-12 12:12:12'
    git reset --hard
    find -name \*.png -print0 | xargs -0 -I FILENAME convert -strip FILENAME FILENAME
    find -name \*.png -print0 | xargs -0 sha256sum > /tmp/a.txt

As from

    unset LD_PRELOAD
    unset FAKETIME
    git reset --hard
    find -name \*.png -print0 | xargs -0 -I FILENAME convert -strip FILENAME FILENAME
    find -name \*.png -print0 | xargs -0 sha256sum > /tmp/b.txt

As it seems, identify.im6 just shows the current timestamp as date:create and date:modify. Confusing. But indeed I get the same output every time: http://paste.ubuntu.com/9550689/

So probably it's just a version difference. My IM is much older:

Version: ImageMagick 6.7.7-10 2014-03-06 Q16 http://www.imagemagick.org

Edit: OH, I'm running it only one time, not until fixpoint. Will try.

@laanwj
Copy link
Member

laanwj commented Dec 17, 2014

Now ran it three times, hash was not longer changing. However, still not all match the files in this pull (though some do). See diff here http://paste.ubuntu.com/9550744/

@jonasschnelli
Copy link
Contributor Author

I assume this happens because of different imagemagick/libpng versions. I mean, the core business of these tools is to deal with png/bitmap internas. So there might be some byte differences because of different handling.
Adding imagemagick and libpng to the depends would probably solve this but is a overkill IMO.

@laanwj
Copy link
Member

laanwj commented Dec 17, 2014

Yes, it's not that important. I primarily wanted to be able to reproduce this when people add files (which the script does). Determinism would be a plus, but adding imagemagick to depends would be overkill.

@fanquake
Copy link
Member

fanquake commented Jan 3, 2015

Needs Rebase

@jonasschnelli jonasschnelli force-pushed the 2014_12_color_profle_strip branch 2 times, most recently from b2a8f19 to 31a09b0 Compare January 3, 2015 19:14
@jonasschnelli
Copy link
Contributor Author

Rebased.

@laanwj laanwj added the GUI label Jan 8, 2015
@jonasschnelli jonasschnelli force-pushed the 2014_12_color_profle_strip branch 3 times, most recently from 613413b to 0572ec7 Compare January 9, 2015 15:45
@jonasschnelli
Copy link
Contributor Author

Updated the script as well as the images so it will use pngcrush to crush the convert-stripped images.

@zander
Copy link

zander commented Jan 13, 2015

Would a tool like optipng or pngcrush or trimage be more appropriate ?

@jonasschnelli
Copy link
Contributor Author

@zander: pngcrush is included. See my last comment.

@laanwj
Copy link
Member

laanwj commented Jan 13, 2015

Your script needs set -e at the top to make it fail when one of the tools fails.
(e.g. I didn't have pngcrush installed and it happily continued)

Also let's move it to contrib/devtools, leaving the script in the contrib toplevel is a bit untidy.

- provide a python script
- add optimized png files
@jonasschnelli
Copy link
Contributor Author

Rewrote script, placed it under contrib/devtools.
Output of processing is here: https://gist.github.com/jonasschnelli/dad302d9b9b8a3163259

@jonasschnelli
Copy link
Contributor Author

A second run of the script won't change the PNG sha256 (at least in my environment).

Check that image contents match pre- and post- crushing.
Also remove use of external tool to compute sha256 in favor of hashlib.
@laanwj
Copy link
Member

laanwj commented Jan 14, 2015

Looks good to me now. When I re-run this script I get the same output as well.
In laanwj@41dc934 I've improved the script a little bit:

  • Add pre/post check of image contents
  • remove use of external tool to compute sha256 in favor of hashlib

@laanwj
Copy link
Member

laanwj commented Jan 14, 2015

Added another commit laanwj@2c77c15 that replaces uses of shell=True with the Python equivalent (as shell=True is less portable, and can be a security hazard).

@jonasschnelli
Copy link
Contributor Author

Nice. Thanks for the additions.
I also added a rename commit because strip_png.py is no longer a adequate name.

laanwj added a commit that referenced this pull request Jan 14, 2015
905711f contrib: improve optimize-pngs.py (Wladimir J. van der Laan)
42f6a0c [Qt] optimize PNG files (Jonas Schnelli)
@laanwj
Copy link
Member

laanwj commented Jan 14, 2015

Merged via d1aa3c6 (squashed into two commits, one with your changes one with mine)

@jonasschnelli
Copy link
Contributor Author

Was merged. Closing.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants