Closed Bug 685471 Opened 9 years ago Closed 8 years ago
In PNG decoder, the allocating of m
Header Buf can be prevented .
mHeaderBuf is dynamically allocated when PNG images are scanned for size only (isSizeDecode), but it is only 24 bytes, so replacing a 4 bytes pointer with a 24 bytes array in the PNGdecoder class is already better than mallocing 24 bytes (causing fragmentation), but actually only an 4 bytes buffer buffer is needed to get the width and height.
Remove the need to allocate a dynamic buffer by only buffering the size bytes into an 8 bytes buffer. By also sorting the members of the class, the total class size remains the same: before: PRUint8 *, PRUint32, PRUint8, PRPackedBool, PRPackedBool, PRUint32, PRPackedBool = 20 bytes after: PRUint8, PRUint32, PRUint32, PRUint8, PRPackedBool, PRPackedBool, PRPackedBool = 20 bytes
I don't see why this patch adds "PRUint8 mAlphaBits;" to the header file. That might be part of the patch for a different bug.
Comment on attachment 745039 [details] [diff] [review] V2: Don't add back mAlphaBits and update to current build Green tryserver run (except for the usual intermittent issues): https://tbpl.mozilla.org/?tree=Try&rev=beec8c74c875
Comment on attachment 745039 [details] [diff] [review] V2: Don't add back mAlphaBits and update to current build Based on my code examination and your try-server results, +r. Not sure I'm a reviewer, so Joe, please review also.
Comment on attachment 745039 [details] [diff] [review] V2: Don't add back mAlphaBits and update to current build Review of attachment 745039 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. ::: image/decoders/nsPNGDecoder.cpp @@ +110,5 @@ > : Decoder(aImage), > mPNG(nullptr), mInfo(nullptr), > mCMSLine(nullptr), interlacebuf(nullptr), > mInProfile(nullptr), mTransform(nullptr), > + mHeaderBytesRead(0), mCMSMode(0), please remove the trailing whitespace @@ +305,5 @@ > + mSizeBytes[mHeaderBytesRead - WIDTH_OFFSET] = bptr[pos]; > + } > + pos ++; > + mHeaderBytesRead ++; > + } trailing whitespace all over here
Attachment #745039 - Flags: review?(joe) → review+
Please don't add PR integer types. We haven't used them since last August.
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf288039ea21 Followup patch to replace the PRUint's: https://hg.mozilla.org/integration/mozilla-inbound/rev/04abf5e608f1
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.