Last Comment Bug 564410 - xpcshell tests using the PNG encoder fail when using system png library
: xpcshell tests using the PNG encoder fail when using system png library
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla6
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-07 05:52 PDT by Mike Hommey [:glandium]
Modified: 2011-04-13 01:13 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Use png_set_filter to make libpng not use write filters (1.03 KB, patch)
2010-05-07 05:52 PDT, Mike Hommey [:glandium]
joe: review-
glennrp+bmo: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2010-05-07 05:52:31 PDT
Created attachment 444072 [details] [diff] [review]
Use png_set_filter to make libpng not use write filters

When using a system png library, or simply when enabling some features in the embedded copy (more precisely write filters), test_encoder_png.js, test_imgtools.js and test_favicons.js fail. This is due to write filters being used and thus changing the encoded png.

A simple fix is to use png_set_filter to make libpng not use filters in such cases.
Comment 1 Joe Drew (not getting mail) 2010-05-07 09:31:11 PDT
Comment on attachment 444072 [details] [diff] [review]
Use png_set_filter to make libpng not use write filters

At first blush, I think we should probably fix the test rather than only allowing us to use the NONE filter. In fact, we should probably compile with PNG_WRITE_FILTER_SUPPORTED! As is, we're creating suboptimal PNGs.

I do want to see what Glenn has to say about this, though - I could be wrong!
Comment 2 Glenn Randers-Pehrson 2010-05-07 09:47:52 PDT
If mozilla is expected to be mostly encoding screen-shots and the like, maybe the NONE filter is best anyway.

It is debatable whether to enable write-filtering in the embedded libpng.
Filtering was disabled in imglib/png/mozlibpng.h to save some code footprint
and cpu time, at the expense of larger PNG files when the NONE filter isn't best (it's best when the number of colors is small and when paletted PNG files are being written).

The patch correctly makes the system libpng act the same as the embedded libpng.
Comment 3 Mike Hommey [:glandium] 2010-05-07 09:48:36 PDT
AFAIK it was done on purpose (and in fact I saw many cases where the NONE filter gave better results than what system libpng chooses when using write filters).

But I can't find evidence of my claim above. Only that write filters have been disabled in bug 386585, while updating the embedded libpng copy to a version that had the macro to disable them for the first time.
Comment 4 Mike Hommey [:glandium] 2010-11-11 06:44:09 PST
Glenn, does comment 2 count as review+ ?
Comment 5 Mike Hommey [:glandium] 2011-04-13 01:13:17 PDT
http://hg.mozilla.org/mozilla-central/rev/11e43d2407d9

Note You need to log in before you can comment on or make changes to this bug.