Closed
Bug 564410
Opened 15 years ago
Closed 14 years ago
xpcshell tests using the PNG encoder fail when using system png library
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
mozilla6
People
(Reporter: glandium, Assigned: glandium)
Details
Attachments
(1 file)
1.03 KB,
patch
|
joe
:
review-
glennrp+bmo
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
Attachment #444072 -
Attachment is patch: true
Attachment #444072 -
Attachment mime type: application/octet-stream → text/plain
Attachment #444072 -
Flags: review?(joe)
Comment 1•15 years ago
|
||
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-
Comment 2•15 years ago
|
||
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•15 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•14 years ago
|
||
Glenn, does comment 2 count as review+ ?
Assignee: nobody → mh+mozilla
Updated•14 years ago
|
Attachment #444072 -
Flags: review?(glennrp+bmo) → review+
Assignee | ||
Comment 5•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in
before you can comment on or make changes to this bug.
Description
•