Closed Bug 1215361 Opened 4 years ago Closed 4 years ago

Three BMP/ICO decoder tweaks

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(3 files)

I have a couple of BMP decoder tweaks.
Actually, I have three, and some are in the ICO decoder.
Summary: Two BMP decoder tweaks → Three BMP/ICO decoder tweaks
Seth's "XXX" comment is correct.
Attachment #8674633 - Flags: review?(seth)
- GetBitsPerPixel() and GetWidth() are no longer used.

- GetHeight() is now only used within nsBMPDecoder and can be renamed and
  inlined into the header.

- GetImageData() can be inlined into the header.
Attachment #8674635 - Flags: review?(seth)
nsICODecoder's reading and writing of little-endian values can be simplified
greatly.

Also, ReadBPP() was highly dodgy: BMP's bpp field is 16-bit
but ReadBPP() read it as if it's 32-bit. I think this currently works because
the bpp field is followed by the 32-bit compression field which is usually 0
for BMPs within ICOs!
Attachment #8674638 - Flags: review?(seth)
Comment on attachment 8674633 [details] [diff] [review]
(part 1) - Don't set mBPP twice in nsICODecoder.cpp

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

Nice.
Attachment #8674633 - Flags: review?(seth) → review+
Comment on attachment 8674635 [details] [diff] [review]
(part 2) - Streamline nsBMPDecoder's getters

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

Looks good!
Attachment #8674635 - Flags: review?(seth) → review+
Comment on attachment 8674638 [details] [diff] [review]
(part 3) - Deconvolute nsICODecoder's handling of endianness

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

Much cleaner!
Attachment #8674638 - Flags: review?(seth) → review+
You need to log in before you can comment on or make changes to this bug.