Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Corrected BLP1 alpha depth handling #8651

Merged
merged 3 commits into from
Jan 1, 2025
Merged

Conversation

radarhere
Copy link
Member

Resolves #8650

https://www.hiveworkshop.com/threads/blp-specifications-wc3.279306/ describes the start of the 'BLP File Header' as

uint32_t magic;
uint32_t content;
if (version >= 2) {
    uint8_t encodingType; // not documented
    uint8_t alphaBits;
    uint8_t sampleType; // not documented
    uint8_t hasMipmaps;
} else {
    uint32_t alphaBits;
}

However, we don't currently perform different behaviour for BLP1, and only treat the data as if it was BLP2.

(self._blp_encoding,) = struct.unpack("<b", self._safe_read(1))
(self._blp_alpha_depth,) = struct.unpack("<b", self._safe_read(1))
(self._blp_alpha_encoding,) = struct.unpack("<b", self._safe_read(1))
self.fd.seek(1, os.SEEK_CUR) # mips

This PR fixes updates how the header data is handled for BLP1, for both reading and writing.

Copy link

@sminjard sminjard left a comment

Choose a reason for hiding this comment

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

Wow, I didn't expect to hear from you this soon.
Thanks!

Everything seems fine. A bit confused about why you had to change things in the _openfunction. I think there is a way to call _read_blp_header so that you don't have that logic at two places, but this will go in another PR as this one seems to resolve the issue.

Happy New Year !

@radarhere
Copy link
Member Author

Thanks for pointing out the duplicate code. As you can see, I've pushed a commit to fix it.

I can see you've approved this PR three times in the last ten minutes. I imagine you're experiencing a bug on your device and not seeing that it worked? If so, I can confirm you have approved this.

@hugovk hugovk merged commit dfb368a into python-pillow:main Jan 1, 2025
50 checks passed
@sminjard
Copy link

sminjard commented Jan 1, 2025

Somehow GitHub was not checking the "#approved-reviews-by>=1" for auto merge check.

@hugovk
Copy link
Member

hugovk commented Jan 1, 2025

This was a manual merge by me, so it didn't use automerge, which would have also needed the label adding and approval by a committer.

@radarhere radarhere deleted the blp branch January 1, 2025 21:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BLP 1 header is not read correctly
3 participants