Closed Bug 403239 Opened 17 years ago Closed 17 years ago

Update libpng to version 1.2.23 and reduce libpng footprint

Categories

(Core :: Graphics: ImageLib, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: glennrp+bmo, Assigned: glennrp+bmo)

References

()

Details

(Keywords: memory-footprint)

Attachments

(3 files, 4 obsolete files)

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 .
Summary: Update libpng to version 1.2.23 and reduce libpng footprint (use patch -E) → Update libpng to version 1.2.23 and reduce libpng footprint
Attachment #288039 - Attachment description: Update trunk to libpng-1.2.23 and reduce libpng footprint → Update trunk to libpng-1.2.23 and reduce libpng footprint (use patch -E)
The README looks like it has been reverted to some old version in this patch.
Also updates CHANGES, README, and libpng.txt
Attachment #288039 - Attachment is obsolete: true
The v1 patch updates LICENSE, which was out of date, not README.
Oops, the v1 patch updates LICENSE, not CHANGES.
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?
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".
Blocks: 191033
Applies /dev/null trick; updates MOZCHANGES.
Attachment #288049 - Attachment is obsolete: true
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
Fixes typos: MOZ_png_err -> MOZ_PNG_err
Attachment #288139 - Attachment is obsolete: true
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.
Attachment #288142 - Attachment is obsolete: true
Attachment #288145 - Flags: review?(pavlov)
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.
Odd, they're removed fine on Windows when doing patch -p0. Either way, you did have the /dev/null in the wrong spot.
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
Attachment #288145 - Flags: review?(pavlov) → review+
Target Milestone: --- → mozilla1.9 M10
Attachment #288145 - Flags: superreview?(tor)
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.
(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).
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.
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?
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.
I opened bug #403423 about the sRGB color_management problem.
Attachment #288145 - Flags: superreview?(tor) → superreview+
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.
Attachment #288145 - Flags: approval1.9?
Attachment #288145 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Attachment #288145 - Attachment description: Update trunk to libpng-1.2.23 and reduce libpng footprint, v3a → Update trunk to libpng-1.2.23 and reduce libpng footprint, v3a (checked in to trunk)
You need to log in before you can comment on or make changes to this bug.