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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanded .ani and .cur functionality #6606

Open
wants to merge 58 commits into
base: main
Choose a base branch
from
Open

Conversation

jlwoolf
Copy link

@jlwoolf jlwoolf commented Sep 21, 2022

Currently the BmpImagePlugin checks if a file is a .cur file by checking if the bits are 32 and a the header is 22. However, the header for cursor files is not guaranteed to be 22.

The CurImagePlugin selects the largest cursor image from the file. The largest cursor does not have to be at the front of the file. So the start of the DIB header file varies depending on the file.

The fix below initializes a parameter in the CurImagePlugin (if it is in fact a .cur file) so that transparency is considered. It might be worth modifying CurImagePlugin to work similarly to IcoImagePlugin however. I'm looking into that right now, but this is a simple fix.

@jlwoolf
Copy link
Author

jlwoolf commented Sep 22, 2022

Turns out it was much easier to implement .cur file saving (as it shares a lot with ICO). So I remodified CurImagePlugin to add that functionality. I reverted the BmpImagePlugin changes.

@radarhere radarhere changed the title The BmpImagePlugin did not work with all .cur files BmpImagePlugin does not work with all .cur files Sep 22, 2022
@jlwoolf jlwoolf changed the title BmpImagePlugin does not work with all .cur files Expanded .ani and .cur functionality Sep 23, 2022
@jlwoolf
Copy link
Author

jlwoolf commented Sep 23, 2022

Alright, made some significant modifications. I believe I have mostly resolved all the .cur file issues, but tests need to be written for a few. I have also further expanded this functionality to work with .ani files. You can now read and write them. I also plan to work on tests for them in the future.

Comment on lines 8 to 9
def test_deerstalker():
with Image.open("Tests/images/cur/deerstalker.cur") as im:
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for this change?

Copy link
Author

Choose a reason for hiding this comment

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

I moved all the .cur files into a folder so that the Images folder was not cluttered by the ones I planned to add. At the same time I decided to rename the test since I was about to test a few more cursors (from windows 98 and up)

src/PIL/CurImagePlugin.py Outdated Show resolved Hide resolved
@jlwoolf
Copy link
Author

jlwoolf commented Nov 14, 2022

@radarhere
Got busy and haven't had a chance to check up on this until. This repository is quite large with quite a few checks (understandably) so I am unsure what problems I need to fix for this merger. Any guidance would be much appreciated.

@akx
Copy link
Contributor

akx commented Mar 30, 2023

Oh, this apparently seems to be similar to my work in #6965 and https://github.com/akx/Pillow/tree/cur-support...

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

Successfully merging this pull request may close these issues.

3 participants