Closed Bug 1132081 Opened 9 years ago Closed 9 years ago

Speed up ConvertHostARGBRow() in the PNG encoder

Categories

(Core :: Graphics: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

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

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(2 files, 3 obsolete files)

In image/encoders/png/nsPNGEncoder.cpp, the ConvertHostARGBRow() function does a de-premultiply operation on all pixels that aren't 100% transparent (alpha == 0), which is a wasteful operation on pixels that are 100% opaque (alpha == 255).  Also, the default zlib compression level (6) is used, while a faster, lower level (4) will still achieve reasonable compression for images that are not likely to be read more than once.  My testing shows a potential reduction of 30% to 50% in cpu time spent in ConvertHostARGBRow(), typically reducing the time for encoding a 640x300 image by 15-20 milliseconds.
Please "try" this patch.
Flags: needinfo?(ryanvm)
Keywords: perf
Comment on attachment 8562833 [details] [diff] [review]
v00-1132081-speed-up-ConvertHostARGBRow

Review of attachment 8562833 [details] [diff] [review]:
-----------------------------------------------------------------

Please separate out style changes from the functional ones.
Style changes undone.
Attachment #8562833 - Attachment is obsolete: true
Failed a bunch of apng unit tests.
Attached file failed.html
It appears that the tests were requiring the generated PNGs to be bit-for-bit identical.  They aren't, because the zlib compression optimization changed.  The attachment compares the two base64 images from the reftest, scaled up so you can see them; they're not bit-for-bit identical but they both describe the same image.
Removed the line that changes the zlib compression level.  Please "try" to make sure it passes the unit tests now.
Flags: needinfo?(ryanvm)
Attachment #8566589 - Attachment is obsolete: true
Attachment #8566589 - Attachment is obsolete: false
Attachment #8563092 - Attachment is obsolete: true
Sorry, I had marked the wrong patch as obsolete.  Naturally the v01 patch failed the unit tests again.  Need a "try" run of the v02 patch.
Flags: needinfo?(ryanvm)
Comment on attachment 8566589 [details] [diff] [review]
v02-1132081-speed-up-ConvertHostARGBRow

XPCShell tests passed, everything green except for two MacOS builds that evidently failed for some other reason.  r?
Attachment #8566589 - Flags: review?(jmuizelaar)
Patch v02 inserted some unnecessary leading zeros in hex constants.
Attachment #8566589 - Attachment is obsolete: true
Attachment #8566589 - Flags: review?(jmuizelaar)
Attachment #8567494 - Flags: review?(jmuizelaar)
Attachment #8567494 - Flags: review?(jmuizelaar) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e1c91e951034
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Blocks: 1137016
You need to log in before you can comment on or make changes to this bug.