Skip to content

Conversation

@brianpopow
Copy link
Collaborator

@brianpopow brianpopow commented Dec 22, 2018

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This PR adds to the BMP decoder the ability to decode Bitmaps with the Compression type BITFIELDS. For bitmap V3 (40 bytes), color masks are followed after the BitmapInfo header.

For bitmap >= V4, this information is stored inside the BitmapInfo header. To be able to get this info, i added parsing V4 information headers.

This fixes #735 and also fixes #732

@brianpopow brianpopow changed the title WIP: Decoding Bitmaps with BITFIELDS masks Decoding Bitmaps with BITFIELDS masks Dec 24, 2018
@brianpopow brianpopow changed the title Decoding Bitmaps with BITFIELDS masks WIP: Decoding Bitmaps with BITFIELDS masks Dec 26, 2018
@codecov
Copy link

codecov bot commented Dec 26, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@abaf558). Click here to learn what that means.
The diff coverage is 98.02%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #796   +/-   ##
=========================================
  Coverage          ?   88.81%           
=========================================
  Files             ?     1012           
  Lines             ?    43447           
  Branches          ?     3130           
=========================================
  Hits              ?    38588           
  Misses            ?     4159           
  Partials          ?      700
Impacted Files Coverage Δ
src/ImageSharp/Formats/Bmp/BmpDecoder.cs 100% <ø> (ø)
src/ImageSharp/Formats/Bmp/BmpConstants.cs 100% <ø> (ø)
tests/ImageSharp.Tests/TestImages.cs 100% <100%> (ø)
...ts/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs 98.64% <100%> (ø)
src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs 91.25% <100%> (ø)
src/ImageSharp/Formats/Bmp/BmpInfoHeader.cs 92.56% <97.43%> (ø)
src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs 92.22% <97.56%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abaf558...46f0c0c. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 26, 2018

Codecov Report

Merging #796 into master will decrease coverage by 0.07%.
The diff coverage is 82.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #796      +/-   ##
==========================================
- Coverage   88.83%   88.76%   -0.08%     
==========================================
  Files        1014     1014              
  Lines       43703    43999     +296     
  Branches     3124     3165      +41     
==========================================
+ Hits        38824    39055     +231     
- Misses       4178     4221      +43     
- Partials      701      723      +22
Impacted Files Coverage Δ
src/ImageSharp/Formats/Bmp/BmpDecoder.cs 100% <ø> (ø) ⬆️
src/ImageSharp/Formats/Bmp/BmpConstants.cs 100% <ø> (ø) ⬆️
tests/ImageSharp.Tests/TestImages.cs 100% <100%> (ø) ⬆️
src/ImageSharp/Formats/Bmp/BmpMetaData.cs 100% <100%> (ø) ⬆️
...ts/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs 98.8% <100%> (+0.89%) ⬆️
src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs 92.04% <100%> (+0.9%) ⬆️
src/ImageSharp/Formats/Bmp/BmpInfoHeader.cs 71.07% <60.25%> (-15.89%) ⬇️
src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs 84.59% <86.04%> (-5.45%) ⬇️
...geSharp.Tests/TestUtilities/TestImageExtensions.cs 83.26% <0%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b74e230...f0f658e. Read the comment docs.

@brianpopow
Copy link
Collaborator Author

I think this one is ready for review now.

One note about the 16 Bits per pixel case: only bit masks with 5 or 6 bits are supported, because according to: http://www.fileformat.info/format/bmp/egff.htm. Windows 95 only supports the RGB555 and RGB565 pixel types. Bitmaps with other bitmasks may be theoretically possible, but i do not really think they can be found in the wild.

@brianpopow brianpopow changed the title WIP: Decoding Bitmaps with BITFIELDS masks Decoding Bitmaps with BITFIELDS masks Dec 27, 2018
Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome! Just a couple of questions before we can continue.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jan 18, 2019

@brianpopow Great work! This all looks good to me so once it's built I'll merge it.

I took the liberty to fix #732 here also since it was blocking your tests.

@JimBobSquarePants JimBobSquarePants merged commit 1996831 into SixLabors:master Jan 18, 2019
@brianpopow
Copy link
Collaborator Author

@JimBobSquarePants: Thank you!

We are getting close to be able to decode all bitmaps from the "good" category from: http://entropymine.com/jason/bmpsuite/bmpsuite/html/bmpsuite.html

Only RLE4 is left now. I will do that next.

@James-Jackson-South
Copy link

Ace! Looking forward to it. Thanks again!

antonfirsov pushed a commit to antonfirsov/ImageSharp that referenced this pull request Nov 11, 2019
* decoding bitmaps with Bitfields masks

* added testcases for Bitfields bitmaps

* added parsing of the complete bitmap V4 header to get the color masks infos for the BITFIELDS compression

* writing now explicitly a Bitamp v3 header (40 bytes)

* added test image for issue SixLabors#735

* fixed rescaling 5-bit / 6-bit to 0 - 255 range

* BitmapEncoder now writes BMP v4 header

* using MemoryMarshal.Cast to simplify parsing v4 header

* added testcases for bitmap v3, v4, v5

* Bitmap encoder writes again V3 header instead of V4. Additional fields for V4 are zero anyway.

* added parsing of special case for 56 bytes headers

* using the alpha mask in ReadRgb32BitFields() when its present

* added support for bitmasks greater then 8 bits per channel

* using MagickReferenceDecoder reference decoder for the test with 10 bits pixel masks

* changed bitmap constants to hex

* added enum for the bitmap info header type

* added support for 52 bytes header (same as 56 bytes without the alpha mask)

* clarified comment with difference between imagesharp and magick decoder for Rgba321010102

* BmpEncoder now writes a V4 info header and uses BITFIELDS compression

* workaround for issue that the decoder does not decode the alpha channel correctly: For V3 bitmaps, the alpha channel will be ignored during encoding

* Fix SixLabors#732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ImageSharp "Does not support this kind of bitmap files" Resized 32-bit BI_RGB BMP is completely black

3 participants