Merge lp:~widelands-dev/widelands/png_alternative_method into lp:widelands

[SirVer]

Run pngcrush pngcrush -ow -rem allb -reduce $file on all pngs in the repo.

Attempt to fix bug 1195724

[SirVer (sirver)]

questions to testers:
- does this fix iCCP: "known incorrect sRGB profile"?
- does this break any images?

[bunnybot (widelandsofficial)]

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/png_alternative_method mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_png_alternative_method

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

[bunnybot (widelandsofficial)]

Travis build 284 has changed state to: passed. Details: https://travis-ci.org/widelands/widelands/builds/102860052.

[Tino (tino79)]

I cannot detect any visual regressions.
Still getting one "libpng warning: iCCP: known incorrect sRGB profile" on exit.
Any hint how to find the corresponding image?

[SirVer (sirver)]

add a log("## %s", filename.c_str()) in load_image() in image_io I guess.

[kaputtnik (franku)]

Seems to be all ok. At least i haven't found any strange colored images in game or editor yet. May i haven't checked all images. Some are also difficult to compare with old status, because some images aren't shown correct also in former revisions. So this isn't a result of this change.

I want to make some more tests before approving.

[kaputtnik (franku)]

Some of the workarea pics have lost transparency. F.e. pics/workarea123.png which is used in the buildings menu.

The indicator for small buildings in buildhelp is darker -> pics/small.png funnily enough pics/easy.png, which shows the same symbol, doesn't got darker.

[kaputtnik (franku)]

Sorry the indicator for small buildings got not darker.

Tino, i couldn't see such messages anymore, even after exit. Have you run the game with the "--datadir" option?

I have fixed the workarea oics and pushed the changes into this branch.

For now i couldn't find more wrong images. I want to make another test later.

[kaputtnik (franku)]

Seems now all is good :-)

review: Approve

[SirVer (sirver)]

kaputtnik, as always thanks for testing :)

Tino, did you find a way to find the images with broken profile? If so, we could revert this branch and only fix the ones that are broken - this would not make this such a huge change for little gain (the size of most PNGs is < 1 block anyways, so shrinking them is not really beneficial).

[Tino (tino79)]

No, sorry.
As a test, I've run "pngcrush -ow -rem allb -reduce" (compiled with gcc 5.3.0 and libpng 1.6.20) once more over all files does not change anything.
BZR then only shows changes on the three files kaputtnik comitted last, but there is no visual difference.
Still 1-4 occurences of the "libpng warning: iCCP: known incorrect sRGB profile" when i quit widelands.

Perhaps some windows only issue, i think we can ignore safely.

review: Approve

[kaputtnik (franku)]

They are not "broken" .. they have only a color profile which is unusual. Almost images saving with adobe photoshop have such a profile.

In favor of reverting the branch, i would just correct them by hand with gimp :-)

We should consider to remove images which are double. They needn't much space, but are confusing, because without knowledge of the code one didn't know which image is used what for. Just my opinion :-)

[SirVer (sirver)]

> In favor of reverting the branch, i would just correct them by hand with gimp :-)

The problem is that we do not have a bullet proof way of figuring out which ones are broken. Do you know one? If so, go for it and fix the PNGs and we can drop this branch.

> We should consider to remove images which are double. They needn't much space, but are confusing, because without knowledge of the code one didn't know which image is used what for. Just my opinion :-)

I am all for this. Some images are doubled so that they could theoretically be changed in the future - it was just convenient to reuse a already existing image in some cases.

[Tino (tino79)]

Both optipng and pngcrush were not able to "fix" (remove the unknown profile) all images.
AdvaneCOMP was able to: http://www.advancemame.it/doc-advdef.html

Now with this branch I get no warning messages from libPNG any longer.

[kaputtnik (franku)]

That is strange... i get also no warnings when using trunk or other older branches anymore. Maybe libpng has changed to suppress this warning?

My current version of libpng is: 1.6.20-1

[kaputtnik (franku)]

I can't believe it... the messages appears again. May i have to set up my branch tree from scratch... Sorry for confusing.

[kaputtnik (franku)]

> The problem is that we do not have a bullet proof way of figuring out which ones are broken.

Maybe pngfix (part of libpng) would do the trick. pngfix does not change anything by default, so

pngfix -q -w [file]

prints warnings (-w) while suppressing normal output (-q). I ran pngfix like

find . -name '*.png' -exec pngfix -q -w {} \;

The result are 38 images with wrong profile. See the [[https://bugs.launchpad.net/widelands/+bug/1195724/+attachment/4551881/+files/pngfix.txt|output ]]

[kaputtnik (franku)]

Dammn syntax... :-D

[SirVer (sirver)]

Superseded by https://code.launchpad.net/~widelands-dev/widelands/png_fix_only_relevant/+merge/283005.

review: Disapprove