The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla6

Status

()

Core
ImageLib
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla6
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
Attachment #444072 - Attachment is patch: true
Attachment #444072 - Attachment mime type: application/octet-stream → text/plain
Attachment #444072 - Flags: review?(joe)
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!
Attachment #444072 - Flags: review?(joe)
Attachment #444072 - Flags: review?(glennrp+bmo)
Attachment #444072 - Flags: review-
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.
Assignee: nobody → glennrp+bmo
Status: NEW → ASSIGNED
(Assignee)

Comment 3

7 years ago
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.
Assignee: glennrp+bmo → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 4

6 years ago
Glenn, does comment 2 count as review+ ?
Assignee: nobody → mh+mozilla

Updated

6 years ago
Attachment #444072 - Flags: review?(glennrp+bmo) → review+
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/11e43d2407d9
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.