403239 - Update libpng to version 1.2.23 and reduce libpng footprint

Status: RESOLVED FIXED

Description

[Glenn Randers-Pehrson]

Created attachment 288039 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint (use patch -E)

libpng-1.2.23 has been released.  No bugs were involved.

There is an opportunity to reduce the libpng footprint by about 11 kbytes, by removing unused warning and error text when PR_LOGGING is off.  The method is demonstrated in the contrib/pngminim directory of the libpng distribution.

pnggccrd.c and pngvcrd.c are no longer used so they can be removed, and the test compile of pnggccrd.c in configure.in can be eliminated.

The defines of MOZ_PNG_READ and MOZ_PNG_WRITE do not need to be global, so they can be defined in the png Makefiles instead of in configure.in, thus simplifying the compile line for all *.c except for those in libpng.

The png Makefile can be revised to not compile some files when png does not appear in MOZ_IMG_DECODERS and others when png does not appear in MOZ_IMG_ENCODERS.

There's a spelling error ("sais" should be "says") in nsPNGDecoder.cpp .

Comment 1

[Reed Loden [:reed] (use needinfo?)]

The README looks like it has been reverted to some old version in this patch.

Comment 2

[Glenn Randers-Pehrson]

Created attachment 288049 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint, v1 (use patch -E)

Also updates CHANGES, README, and libpng.txt

Comment 3

[Glenn Randers-Pehrson]

The v1 patch updates LICENSE, which was out of date, not README.

Comment 4

[Glenn Randers-Pehrson]

Oops, the v1 patch updates LICENSE, not CHANGES.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Were you planning on filing a separate bug for two file removals or were you planning on doing that with the patches in this bug?

Comment 6

[Glenn Randers-Pehrson]

If you use "patch -E -i *v1.diff -p0" the two files disappear.  I forgot to apply the /dev/null trick to work around the problem with "diff".

Comment 7

[Glenn Randers-Pehrson]

Created attachment 288139 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint, v2 (use patch -E)

Applies /dev/null trick; updates MOZCHANGES.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

This doesn't compile on Windows.

png.lib(pngmem.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngrio.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngrutil.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngwutil.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngset.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(png.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngwrite.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngwio.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngget.obj) : error LNK2019: unresolved external symbol _MOZ_png_err referenced in function _MOZ_PNG_get_IHDR
png.lib(pngrtran.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _MOZ_png_err
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_png_err

Comment 9

[Glenn Randers-Pehrson]

Created attachment 288142 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint, v3 (use patch -E)

Fixes typos: MOZ_png_err -> MOZ_PNG_err

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Created attachment 288145 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint, v3a (checked in to trunk)

With the typo corrected, it compiles fine now.

I'm posting this to fix the /dev/null typo. You put it on the wrong line ;-). With that fixed, patch -E isn't needed anymore either.

Comment 11

[Glenn Randers-Pehrson]

I did "patch -i *v3a.diff -p0" (in the mozilla directory) and indeed the resulting files compile fine, but pnggccrd.c and pngvcrd.c are not removed.
When applying "patch -E -i *v3.diff -p0" (in the mozilla/.. directory) the files are removed.  This is on a FreeBSD 5.4 platform.

The side-by-side "diff" does say, incorrectly, that the files are removed.  diff for the v3 patch is a little screwy; it says they were added to CVS.

I prefer the "v3" behavior over the "v3a" behavior.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Odd, they're removed fine on Windows when doing patch -p0. Either way, you did have the /dev/null in the wrong spot.

Comment 13

[Stuart Parmenter]

Comment on attachment 288145 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint, v3a (checked in to trunk)

rs=me

Comment 14

[Glenn Randers-Pehrson]

Created attachment 288204 [details]
panel from http://pmt.sf.net/gamma_test

Reviewers: There is a problem with displaying the page
http://pmt.sf.net/gamma_test.  Look at it with color_management
enabled and disabled.

The "sRGB" images (just below the "gamma = 1/2.2" images) are displayed
incorrectly when color management is enabled.  I'm not sure
whether that's due to this patch or something else.

Comment 15

[Glenn Randers-Pehrson]

(In reply to comment #14)
The nightly build from mozilla.org (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111104 Minefield/3.0b2pre Creative ZENcast v1.02.11) does not exhibit this behavior.  The page is displayed properly (except the table layout is incorrect, but that's irrelevant here).

Comment 16

[Glenn Randers-Pehrson]

The image is 8-bit grayscale and has only IHDR, sRGB, IDAT, and IEND chunks.  From the look of the image with color_management enabled, it appears that
the pixel size has changed from 8 bits to something else.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Created attachment 288240 [details]
Before and after screenshot

Yikes, not good. This screenshot shows the image before and after color correction is turned on. However, I've also confirmed locally that the problem also occurs with libpng 1.2.22, so it doesn't appear to be related to this patch. File a new bug for it?

Comment 18

[Glenn Randers-Pehrson]

OK, I'll file a separate bug.  Reviewers, this problem need not hold up landing libpng-1.2.3, then, since the bug was already checked in with the color_management implementation.  I believe the fix is a simple change to nsPNGDecoder.cpp, line 409, to change png_valid(... gAMA) to (png_valid(... gAMA)||png_valid(... sRGB)).  Testing now.

Comment 19

[Glenn Randers-Pehrson]

I opened bug #403423 about the sRGB color_management problem.

Comment 20

[Reed Loden [:reed] (use needinfo?)]

Comment on attachment 288145 [details] [diff] [review]
Update trunk to libpng-1.2.23 and reduce libpng footprint, v3a (checked in to trunk)

Simple upgrade to the next version of libpng.

Comment 21

[Reed Loden [:reed] (use needinfo?)]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1888; previous revision: 1.1887
done
Checking in modules/libimg/png/CHANGES;
/cvsroot/mozilla/modules/libimg/png/CHANGES,v  <--  CHANGES
new revision: 3.10; previous revision: 3.9
done
Checking in modules/libimg/png/LICENSE;
/cvsroot/mozilla/modules/libimg/png/LICENSE,v  <--  LICENSE
new revision: 1.11; previous revision: 1.10
done
Checking in modules/libimg/png/MOZCHANGES;
/cvsroot/mozilla/modules/libimg/png/MOZCHANGES,v  <--  MOZCHANGES
new revision: 3.22; previous revision: 3.21
done
Checking in modules/libimg/png/Makefile.in;
/cvsroot/mozilla/modules/libimg/png/Makefile.in,v  <--  Makefile.in
new revision: 1.36; previous revision: 1.35
done
Checking in modules/libimg/png/README;
/cvsroot/mozilla/modules/libimg/png/README,v  <--  README
new revision: 3.13; previous revision: 3.12
done
Checking in modules/libimg/png/libpng.txt;
/cvsroot/mozilla/modules/libimg/png/libpng.txt,v  <--  libpng.txt
new revision: 3.10; previous revision: 3.9
done
Checking in modules/libimg/png/mozpngconf.h;
/cvsroot/mozilla/modules/libimg/png/mozpngconf.h,v  <--  mozpngconf.h
new revision: 3.15; previous revision: 3.14
done
Checking in modules/libimg/png/png.c;
/cvsroot/mozilla/modules/libimg/png/png.c,v  <--  png.c
new revision: 3.19; previous revision: 3.18
done
Checking in modules/libimg/png/png.h;
/cvsroot/mozilla/modules/libimg/png/png.h,v  <--  png.h
new revision: 3.20; previous revision: 3.19
done
Checking in modules/libimg/png/pngconf.h;
/cvsroot/mozilla/modules/libimg/png/pngconf.h,v  <--  pngconf.h
new revision: 3.25; previous revision: 3.24
done
Checking in modules/libimg/png/pngerror.c;
/cvsroot/mozilla/modules/libimg/png/pngerror.c,v  <--  pngerror.c
new revision: 3.15; previous revision: 3.14
done
Removing modules/libimg/png/pnggccrd.c;
/cvsroot/mozilla/modules/libimg/png/pnggccrd.c,v  <--  pnggccrd.c
new revision: delete; previous revision: 3.14
done
Checking in modules/libimg/png/pngpread.c;
/cvsroot/mozilla/modules/libimg/png/pngpread.c,v  <--  pngpread.c
new revision: 3.20; previous revision: 3.19
done
Checking in modules/libimg/png/pngrtran.c;
/cvsroot/mozilla/modules/libimg/png/pngrtran.c,v  <--  pngrtran.c
new revision: 3.15; previous revision: 3.14
done
Checking in modules/libimg/png/pngrutil.c;
/cvsroot/mozilla/modules/libimg/png/pngrutil.c,v  <--  pngrutil.c
new revision: 3.21; previous revision: 3.20
done
Checking in modules/libimg/png/pngset.c;
/cvsroot/mozilla/modules/libimg/png/pngset.c,v  <--  pngset.c
new revision: 3.19; previous revision: 3.18
done
Removing modules/libimg/png/pngvcrd.c;
/cvsroot/mozilla/modules/libimg/png/pngvcrd.c,v  <--  pngvcrd.c
new revision: delete; previous revision: 3.10
done
Checking in modules/libpr0n/decoders/png/Makefile.in;
/cvsroot/mozilla/modules/libpr0n/decoders/png/Makefile.in,v  <--  Makefile.in
new revision: 1.21; previous revision: 1.20
done
Checking in modules/libpr0n/decoders/png/nsPNGDecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp,v  <--  nsPNGDecoder.cpp
new revision: 1.75; previous revision: 1.74
done
Checking in modules/libpr0n/encoders/png/Makefile.in;
/cvsroot/mozilla/modules/libpr0n/encoders/png/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done