Multiline font line spacing ignores baselines

[hugovk]

Some example code to draw three lines of text:

from PIL import Image, ImageDraw, ImageFont

font = ImageFont.truetype("/Library/Fonts/Arial.ttf", 50)

im = Image.new("RGB", (400, 300), color="white")
draw = ImageDraw.Draw(im)
draw.multiline_text((50, 50), 
                    "out of the\nfrying pan\ninto the fire", 
                    font=font, align="center", fill="black")

im.show()

Produces:

The line spacing is a bit off.

The line spacing is worked out by simply calculating the height of each line of rasterised text (then adding some optional spacing, not done in this example):

https://github.com/python-pillow/Pillow/blob/master/PIL/ImageDraw.py#L275-L278

But because the first line and last line has ascenders but no descenders, and the second has both, it gives wonky output.

Instead, the lines should be offset by the ​_baselines_​ of each line, not their absolute heights. Here's an example from the web:

Is this something that can be fixed?

---

ImageDraw.multiline_text gets the line height from ImageDraw. textsize() <- ImageFont.getsize <- Image.core.font.getsize <- and ImagingFontObject does have a baseline used internally, but not returned.

What does ImageDraw.multiline_text need for its calculations? Just height of the bitmap line and the bitmap's offset from the baseline?

cc @radarhere

[radarhere]

I think that information should be enough to fix the issue, yes. I have tried to make the change to allow access the baseline from Python, but without success.

[wiredfool]

We've had nearly this exact bug before, and even have a test for it in test_imagefont.py. (the ones that use "hey you\nyou are awesome\nthis looks awkward" )

[hugovk]

https://github.com/python-pillow/Pillow/blob/master/Tests/images/multiline_text.png:

That one actually looks ok, 20px between baselines, but I think only by circumstance of the letters used:

[hugovk]

My first example has 34px and 41px between baselines:

[wiredfool]

That particular problem of baseline shifting, was what was being tested by that string. Apparently it predates the multiline text, and the multiline text call didn't use the same method to determine line spacing vs line height that was previously fixed.

It's also possible that the test doesn't discriminate enough, because it's too small or something, and the differences are within a pixel.

[wiredfool]

Ok, checked on this a bit more, and there are some metrics we can get from Freetype, and some that we're already getting. I added height, x_ppem (pixels per em, oddly enough, it's the font size), and y_ppem, which are mostly useless. Height is the minimum line to line spacing, which isn't really what we're worried about. It's recommended to use ascent + descent in the Freetype docs.

Ascent (which we have) is the baseline to top of character. Descent is from the baseline to the bottom of the descenders.

>>> from PIL import ImageFont
>>> t = ImageFont.truetype('DejaVuSans.ttf', 40)
>>> t.getmetrics()
(38, 10)
>>> t.font.ascent
38
>>> t.font.descent
10
>>> t.font.height
47
>>> t.font.x_ppem
40
>>> t.font.y_ppem
40
>>> t.getsize('A')
(27, 38)
>>> t.getsize('AB')
(54, 38)
>>> t.getsize('M')
(35, 38)
>>> t.getsize('y')
(24, 46)
>>> t.getsize('a')
(25, 38)
>>> 

Looking at the generated sizes here, there are three things that pop out:

• Note that the size of 'A' and 'a' are the same, which is the same as the ascent.
• Also note that 'y' is bigger, signifying a descender.
• They're measured from the top of the ascender, not the top of the character. So we will get a consistent spacing if we have the top corners incremented by a line height.

So, if we follow the approach in test_render_multiline, that of using constant spacing, everything should work out correctly:

        def test_render_multiline(self):
            im = Image.new(mode='RGB', size=(300, 100))
            draw = ImageDraw.Draw(im)
            ttf = ImageFont.truetype(FONT_PATH, FONT_SIZE)
            line_spacing = draw.textsize('A', font=ttf)[1] + 4
            lines = TEST_TEXT.split("\n")
            y = 0
            for line in lines:
                draw.text((0, y), line, font=ttf)
                y += line_spacing

I've put both of these changes in my multiline_text branch: https://github.com/wiredfool/Pillow/tree/multiline_text but haven't tweaked the tests yet. I think this is the right approach.

[hugovk]

Here's the diff: master...wiredfool:multiline_text

Looks good.

[hugovk]

I've added a PR that updates the tests: #1573.

I noticed the lines are a bit close, and would benefit with some more default spacing, rather than zero. We had four pixels before, so created a parallel PR with extra spacing: #1574.

Also both PRs decrease coverage because the changes in _imagingft.c to add getters for x_ppem, y_ppem and height aren't actually used in the fix, and were just for investigation. Shall we just remove them?

[wiredfool]

I'm not sure that we need x/y_ppem. OTOH, I don't see any reason that we shouldn't expose them. A quick test should do it, and make sure that we're not shipping broken. Height is at least interesting in that it's not the same as the font size.