Skip to content

Conversation

@npjg
Copy link
Contributor

@npjg npjg commented Oct 21, 2022

It's an old format, but I still see it a good bit when examining old software.

@radarhere radarhere added the BMP label Oct 21, 2022
assert_image_similar_tofile(im, "Tests/images/hopper_4bit.bmp", 12)

with Image.open("Tests/images/bmp/g/pal4rle.bmp") as im:
assert_image_similar_tofile(im, "Tests/images/bmp/g/pal4.bmp", 12)
Copy link
Member

Choose a reason for hiding this comment

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

Is there any significant difference between hopper_rle4.bmp and pal4rle.bmp?

Copy link
Member

Choose a reason for hiding this comment

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

I've removed hopper_rle4.bmp, since the other test image seems sufficient.

@radarhere
Copy link
Member

I've created npjg#1 with a documentation correction, and to simplify the code by combining the two BMP RLE decoders into one.

x = len(data) % self.state.xsize
else:
# absolute mode (2 pixels per byte)
total_bytes_to_read = byte[0] // 2
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this "2" comment mean? If it means this value is always 2, that won't work, because this is already checked for on line 367.

Copy link
Member

Choose a reason for hiding this comment

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

It's not a comment. Commenting in Python is #. This is floor division.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, right. There's even a real comment right above that and I didn't notice...

@radarhere radarhere added the automerge Automatically merge PRs that are ready label Oct 24, 2022
@radarhere radarhere changed the title Add 4-bit bitmap RLE decoder Added support for reading BMP images with RLE4 compression Oct 24, 2022
@radarhere radarhere merged commit 5c9bc65 into python-pillow:main Oct 24, 2022
@npjg
Copy link
Contributor Author

npjg commented Oct 26, 2022

I was gone for a few days. Looks good, thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge PRs that are ready BMP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants