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

getsize_multiline doesn't take into account characters that extend below the baseline #5816

Closed
jpetazzo opened this issue Nov 6, 2021 · 9 comments · Fixed by #6381
Closed
Labels

Comments

@jpetazzo
Copy link

jpetazzo commented Nov 6, 2021

What did you do?

I'm using getsize_multiline to compute the size of a text label that I render to an image. The image is exactly the size of the text label.

I noticed that my text labels where cropped; specifically, when there are letters like p or g (which extend below the baseline) on the last line of text, these letters get cropped.

The problem doesn't affect getsize, and can easily be exhibited by comparing the sizes reported by getsize vs getsize_multiline for a single line string like g.

Example:

from PIL import ImageFont
font = ImageFont.truetype("DroidSans", 20)
print(font.getsize("g"))
print(font.getsize_multiline("g"))

What did you expect to happen?

The getsize and getsize_multiline methods should return the same size.

What actually happened?

With DroidSans in size 20:

  • getsize returns (10,24)
  • getsize_multiline returns (10,19)

The g actually extends below if I create a label of size 10,19 and draw the string g on it, the g gets cropped.

What are your OS, Python and Pillow versions?

  • OS: Arch Linux
  • Python: 3.9
  • Pillow: 8.4.0
#!/usr/bin/env python

from PIL import Image, ImageFont, ImageDraw

font = ImageFont.truetype("DroidSans", 20)

text = "gÂp"

size = font.getsize(text)
image = Image.new("RGB", size)
draw = ImageDraw.Draw(image)
draw.text((0, 0), text, font=font)
image.save("getsize.png")

size = font.getsize_multiline(text)
image = Image.new("RGB", size)
draw = ImageDraw.Draw(image)
draw.text((0, 0), text, font=font)
image.save("getsize_multiline.png")

getsize: getsize

getsize_multiline: getsize_multiline

I imagine that the logic in getsize_multiline doesn't take into accounts characters that extend below the baseline. For now I'm going to just add a bit of padding on the last line 😅 but I guess there has to be a better way.

@jpetazzo
Copy link
Author

jpetazzo commented Nov 6, 2021

Of course, immediately after opening that issue, I find a very promising method: getmetrics(), that precisely returns the ascent and the descend of a font. It looks like all I need to do is add the "descent" at the bottom of the label. I'll check that and if it works, I'll see if I can propose a patch for getsize_multiline and associated tests.

@nulano
Copy link
Contributor

nulano commented Nov 6, 2021

Both getsize or getsize_multilne are often inaccurate (especially with italics or accents), but unfortunately they might never get fixed due to backwards compatibility (also, the definition of size can change depending on your use case, so the expected return value is unclear). I would not recommend using them in new code. The docs for getsize have this warning:

For historical reasons this function measures text height from the ascender line instead of the top, see Text anchors. If you wish to measure text height from the top, it is recommended to use the bottom value of getbbox() with anchor='lt' instead.

It seems I didn't add a warning to the multiline version, likely because I didn't add a getbbox_multiline function (it would be an exact copy-paste). However, in most cases you will be using a font to draw onto an image using ImageDraw.text, which provides the two equivalent functions textsize and multiline_textsize. The docs for multiline_textsize do have a similar warning:

For historical reasons this function measures text height as the distance between the top ascender line and bottom descender line, not the top and bottom of the text, see Text anchors. If you wish to measure text height from the top to the bottom of text, it is recommended to use multiline_textbbox() instead.

In fact, I think this might not be entirely accurate due to the way Pillow computes line spacing - glancing at the source I suspect it is measuring to the bottom baseline instead in most cases and ignoring the last descender line like you mentioned.

Instead, I believe ImageDraw.multiline_textbbox is the function you are looking for. It's documentation also provides a brief explanation:

The bounding box includes extra margins for some fonts, e.g. italics or accents.

As for why getsize and getsize_multiline are not deprecated, IIRC I didn't propose this because the new functions work only with TrueType fonts, while the old functions work correctly with old "PIL" fonts. However, it might make sense to deprecate the old functions for TrueType fonts only.

Edit: For more information see #4959 and the linked issues.

@hugovk
Copy link
Member

hugovk commented Nov 8, 2021

As for why getsize and getsize_multiline are not deprecated, IIRC I didn't propose this because the new functions work only with TrueType fonts, while the old functions work correctly with old "PIL" fonts. However, it might make sense to deprecate the old functions for TrueType fonts only.

This deprecation sounds reasonable.

What do others think?

If adding warnings now (Pillow 9.0.0, 2022-01-02), it would be eligible for removal in Pillow 10.0.0 (2023-07-01), giving a nice long deprecation period.

@hugovk hugovk added the Fonts label Nov 8, 2021
@jpetazzo
Copy link
Author

jpetazzo commented Nov 8, 2021

Instead, I believe ImageDraw.multiline_textbbox is the function you are looking for.

Yes indeed, thank you so much! I have one follow-up question: ImageDraw.multiline_textbbox requires an ImageDraw instance, but my program creates the image after computing the text size (since the image holds the text label and nothing else). I did a little experiment and it looks like I can create an image of size (0,0) and use that to call multiline_textbbox, then create my final image with the proper size. Is that the right thing to do? Or might that break in the future?

Thank you so much!

@radarhere
Copy link
Member

I don't think that what you're describing should break in the future - and yes, it sounds like a reasonable way of doing things.

@radarhere
Copy link
Member

As for why getsize and getsize_multiline are not deprecated, IIRC I didn't propose this because the new functions work only with TrueType fonts, while the old functions work correctly with old "PIL" fonts. However, it might make sense to deprecate the old functions for TrueType fonts only.

If we deprecate those, we should also then add getbbox_multliline and TransposedFont.getbbox, yes?

@nulano
Copy link
Contributor

nulano commented Apr 11, 2022

If we deprecate those, we should also then add getbbox_multliline and TransposedFont.getbbox, yes?

Probably. That is what I was unsure about and why I didn't propose deprecating earlier.

I'll take a look at this as part of #6195 (comment).

@radarhere
Copy link
Member

@nulano are you still working on a deprecation PR? I ask because we are approaching Pillow 9.2.0, the last chance to have a year of deprecation before Pillow 10.0.0.

@nulano
Copy link
Contributor

nulano commented Jun 19, 2022

Yes, should be done some time this week.

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

Successfully merging a pull request may close this issue.

4 participants