Last Comment Bug 683610 - Restore support for 1bpp RLE bitmaps
: Restore support for 1bpp RLE bitmaps
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-31 08:42 PDT by neil@parkwaycc.co.uk
Modified: 2011-09-01 21:32 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (1.07 KB, patch)
2011-08-31 08:44 PDT, neil@parkwaycc.co.uk
joe: review+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2011-08-31 08:42:14 PDT
One of the checks for 1bpp RLE bitmaps was inadvertently removed.
Comment 1 neil@parkwaycc.co.uk 2011-08-31 08:44:53 PDT
Created attachment 557190 [details] [diff] [review]
Proposed patch
Comment 2 Brian R. Bondy [:bbondy] 2011-08-31 08:46:01 PDT
RLE4 1BPP was intentionally removed. 

The new way it works is correct according to the spec.
Even know we allowed RLE4 1BPP before since 2002, I think that it is better to have correct handling based on the spec and authoritive source (MSDN).

- IE9, Windows photo viewer, and MSPaint can't read this bitmap
- Wikipedia says: "BI_RLE4 RLE 4-bit/pixel Can be used only with 4-bit/pixel bitmaps" (http://en.wikipedia.org/wiki/BMP_file_format)
- This wotsit document http://www.edm2.com/0107/os2bmp.html says "BCA_RLE4 - Run-length encoded, 4 bits/pixel 
- MSDN says "Windows supports formats for compressing bitmaps that define their colors with 8 or 4 bits-per-pixel. "
  http://msdn.microsoft.com/en-us/library/dd183383(v=VS.85).aspx
  Does not say 1 BPP which we used to allow.

I think this should be WONTFIX.
Comment 3 neil@parkwaycc.co.uk 2011-08-31 08:47:21 PDT
(In reply to Brian R. Bondy from comment #2)
> RLE4 1BPP was intentionally removed.
Then you should have done it properly.
Comment 4 Brian R. Bondy [:bbondy] 2011-08-31 08:48:54 PDT
> Then you should have done it properly.

Understood, my bad.
Comment 5 Brian R. Bondy [:bbondy] 2011-08-31 08:53:31 PDT
I intentionally didn't specify 1BPP, but didn't realize I had changed the logic there.  I understand that in the future I should put a separate bug and fix for such a situation though.
Comment 6 Joe Drew (not getting mail) 2011-08-31 08:56:23 PDT
Comment on attachment 557190 [details] [diff] [review]
Proposed patch

If it is the case that this is the only change we need to support 1bpp RLE bitmaps, I think we should support them. I wasn't entirely aware that this was being removed with the other patches, though I should've.
Comment 7 Brian R. Bondy [:bbondy] 2011-08-31 09:09:57 PDT
Neil since we are going to continue supporting this, could you please add the reference image you sent me to Bug 683205?  I'll make sure to put a reftests for RLE4 1BPP.  I can't find the link any more that you sent me earlier.
Comment 8 Brian R. Bondy [:bbondy] 2011-08-31 09:16:14 PDT
Never mind I found the link, I'll add it.
Comment 9 neil@parkwaycc.co.uk 2011-08-31 12:12:47 PDT
Pushed changeset 84233d215c4e to mozilla-central.

Note You need to log in before you can comment on or make changes to this bug.