Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #2467 bmp encoding issue for BMP with 1 bit per pixel and more pixels per row than divisible by 8. #2471

Merged
merged 2 commits into from Jun 8, 2023
Merged

Conversation

synercoder
Copy link
Sponsor Contributor

@synercoder synercoder commented Jun 7, 2023

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 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

Fix for issue #2467.

If you have a BMP with 1 bit per pixel, and more pixels per row than divisible by 8, the last byte of the row will need to be padded with extra zeros.

So if you have a black/white image of 10 pixels in a row, and the row looks like:
bwwbbbwbww
The bytes should look like:"
0x01100010 0x11000000

However, the current implementation of Write1BitPixelData will when it reaches the last byte looks like:

if (quantizedPixelRow.Length % 8 != 0)
{
    int startIdx = quantizedPixelRow.Length - 7;
    endIdx = quantizedPixelRow.Length;
    Write1BitPalette(stream, startIdx, endIdx, quantizedPixelRow);
}

So quantizedPixelRow.Length will be 10, resulting in a startIdx of 10-7 = 3
It will then write pixels to the byte starting at index 3 again, meaning that the last bytes looks like:
0x00010110, the last byte will always contain the last 7 pixels, instead of the amount of pixels remaining.

This results in that when the image in opened again, the row bytes looks like:
0x01100010 0x00010110

If you check the images in #2467, you can see that the corrupted image shows column 4 & 5 again in column 9 & 10.

The fix

The fix is to read the last X pixels from the quantizedPixelRow when X equals the remaining pixels instead of always the last 7 pixels. So the startIdx needs to be adjusted:
From:
int startIdx = quantizedPixelRow.Length - 7;
To:
int startIdx = quantizedPixelRow.Length - (quantizedPixelRow.Length % 8);

Small note

I am unsure of my test name, so I named it after my issue. I hope this is fine.

@CLAassistant
Copy link

CLAassistant commented Jun 7, 2023

CLA assistant check
All committers have signed the CLA.

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.

Legend!

I would have been wasting time looking for issues in the quantizer. Thanks for providing a fix.

@JimBobSquarePants JimBobSquarePants merged commit 6a7e5ff into SixLabors:main Jun 8, 2023
8 checks passed
@synercoder synercoder deleted the fixes/bit1-pallet-bmp-bug branch June 8, 2023 09:34
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.

None yet

3 participants