Closed
Bug 1215361
Opened 9 years ago
Closed 9 years ago
Three BMP/ICO decoder tweaks
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla44
| Tracking | Status | |
|---|---|---|
| firefox44 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(3 files)
|
(deleted),
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
I have a couple of BMP decoder tweaks.
|
Assignee | |
Comment 1•9 years ago
|
||
Actually, I have three, and some are in the ICO decoder.
Summary: Two BMP decoder tweaks → Three BMP/ICO decoder tweaks
|
|
Assignee | |
Comment 2•9 years ago
|
||
Seth's "XXX" comment is correct.
Attachment #8674633 -
Flags: review?(seth)
|
|
Assignee | |
Comment 3•9 years ago
|
||
- 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)
|
|
Assignee | |
Comment 4•9 years ago
|
||
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 5•9 years ago
|
||
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 6•9 years ago
|
||
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 7•9 years ago
|
||
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+
|
|
Assignee | |
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4c60957e505a114a77646670576a03c09033f1c
Bug 1215361 (part 1) - Don't set mBPP twice in nsICODecoder.cpp. r=seth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c6abaa798f99f17827cb06088fc371336b3b879
Bug 1215361 (part 2) - Streamline nsBMPDecoder's getters. r=seth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6928d7087fedeeeb6cdcff2ddb5eba14ff96163b
Bug 1215361 (part 3) - Deconvolute nsICODecoder's handling of endianness. r=seth.
|
||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a4c60957e505
https://hg.mozilla.org/mozilla-central/rev/1c6abaa798f9
https://hg.mozilla.org/mozilla-central/rev/6928d7087fed
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in
before you can comment on or make changes to this bug.
Description
•