564410 - xpcshell tests using the PNG encoder fail when using system png library

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Mike Hommey [:glandium]]

Attachment: 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)]

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]

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]]

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]]

Glenn, does comment 2 count as review+ ?

Comment 5

[Mike Hommey [:glandium]]

http://hg.mozilla.org/mozilla-central/rev/11e43d2407d9