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

Adjusting date time text to match image size #22

Merged
merged 14 commits into from
May 21, 2024

Conversation

ZachHoppinen
Copy link
Contributor

@ZachHoppinen ZachHoppinen commented May 20, 2024

Hello,

Feel free to close if this isn't helpful but when I use this great package with large xarray dataarrays the date text in the corner get very small and illegible. This push:

  • Sets an initial text size of 6 pt
  • Checks if the resulting bounding box will be 10% of image width
  • If it is smaller it increments to the next larger font size until the date time text fills in 10% of image

Credit to this slack exchange answer: https://stackoverflow.com/questions/4902198/pil-how-to-scale-text-size-in-relation-to-the-size-of-the-image

One weird thing I found was the background box was too short vertically after these changes so I added a 1.6x multiplier to the height to make the text work but I suspect I may just be missing something there.

Cheers!
Zach

@ZachHoppinen
Copy link
Contributor Author

Just noticed this may resolve #18 to some extent as well.

gjoseph92 added 10 commits May 21, 2024 00:12
The `size` parameter isn't available in `load_default` until then.
- add `date_size` kwarg, supporting absolute font size or fraction
- hopefully handle case where Pillow doesn't support FreeType, and you get a non-resizeable font back—don't want to loop forever trying to make it bigger!
- fix text/rect positioning logic to avoid 1.6x scaling hack: turns out we can't assume the first two coords in `t_bbox` are (0, 0). Basically there seems to be some "inherent offset" in the font rendering?
Copy link
Owner

@gjoseph92 gjoseph92 left a comment

Choose a reason for hiding this comment

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

@ZachHoppinen thanks so much for working on this! The tiny font has always bothered me too. When I wrote all this originally, the built-in font with Pillow didn't support different sizes, but it seems that as of 10.1, the default font has changed and we can now do this. So thanks for looking into it.

I've played around with this a bit and am pushing a few changes:

  • actually requiring Pillow 10.1 (otherwise passing size= would throw an error with older versions)
  • exposing the size as an argument (both as a fraction, like you've done, and an absolute font size)
  • figuring out why the text wasn't getting fully covered, instead of the 1.6x height workaround
  • updating the docs with new examples showing how this looks, and how to use it

geogif/gif.py Outdated
img_fraction = 0.1

# increment font until you get large enough font
while fnt.getbbox(test_label)[2] < img_fraction*test_img.size[0]:
Copy link
Owner

Choose a reason for hiding this comment

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

The Pillow docs say this controls "The font size of Aileron Regular"—so if you don't have FreeType support and get the basic bitmap font (like previous versions of Pillow had), I think this would loop forever, since the font would never actually get bigger no matter what font size you asked for!

@gjoseph92
Copy link
Owner

@ZachHoppinen I'll leave this for you to test out on your data, but I think it's good to go, so once you confirm I'll merge and cut a release!

@ZachHoppinen
Copy link
Contributor Author

@gjoseph92 Just ran this on my data for a few different font sizes and fractions and it looks awesome now! Thanks for cleaning that all up and for this super cool project!

@gjoseph92
Copy link
Owner

Great to hear! Thanks for the contribution and figuring out the core change!

@gjoseph92 gjoseph92 merged commit 528ebd8 into gjoseph92:main May 21, 2024
1 check passed
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.

2 participants