-
Notifications
You must be signed in to change notification settings - Fork 101
Windows: fix parsing of 24-bit bitmaps and prefer PNG when available #198
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Also rebased on master to fix the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems good to me now, thanks again for the PR and I doubly appreciate the added test cases.
I was curious if this affected #122 but it seems like its the same regardless of this change. I'll have to continue investigating what's going on there later... EDIT: Nevermind, my brain got two things mixed up here.
|
Great 🎊 thanks for your help too. Do you usually stick to a regular release schedule? Would be great to move my arboard dependencies back to the crates.io version. |
|
I don't have a documented schedule for releases. I usually just release once enough has changed or someone asks. Between this and the |
|
This is now released in v3.6.1 on crates.io. Thank you very much, again, for all the investigation and testing you did before and during this PR, and the collaborative effort. It made landing such a complicated/fragile change a lot easier for me. |
Fixes #192
I tested on commits before 74ece6a that copying from Chrome/snipping tool/Discord/a few more still work, and that copying from Paint/Firefox works too (with transparent backgrounds filled in with black).
I then tested on the PNG-supporting commit that everything still works, with additional support for transparency from Firefox
Aside: this doesn't feel like the perfect solution. Hypothetically, we could see a bitmap with bitCount = 32, compression = BI_RGB, non-zero alpha mask, and nonsense/zero bytes in the alpha channel that upstream (who's following the spec strictly) wants us to ignore; I think this PR breaks support for cases like this (when PNG's unavailable). But I think supporting copying from firefox/paint makes this a clear improvement, and the tests I added make it easy to iterate safely if/when the next person runs into these issues :)