-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Rendering multi line text in bounding box, text bbox coordinates issue #8092
Comments
I think the unfortunate reality is that each font can set the tallest character to be whatever it wants. Here's a font where 'A' is taller. It's also feasible that a font doesn't have the '[' character. 'A' was chosen in #1574
If I understand correctly, you're not saying that the co-ordinates returned by
So your suggestion is to remove the vertical offset that the font chooses to apply to the text. I'm not sold on the idea that we should be ignoring the font author's idea about how much vertical gap should be above characters. If you'd like to do so in your code, that is fine. |
We already ignore the font author's idea about line spacing, see #1646 / #6469 (comment). My suggestion from #6469 (comment) is: def _multiline_spacing(self, font, spacing, stroke_width):
return font.font.height I just don't know how we could make this change without breaking backwards compatibility for a lot of people. |
I understand "A" can be taller than "[".
Sorry for confusing. I think draw.textbbox() and draw.text() are both incorrect for multiline text. because def _multiline_spacing(self, font, spacing, stroke_width):
return (
self.textbbox((0, 0), "A", font, stroke_width=stroke_width)[3]
+ stroke_width
+ spacing
) |
|
I don't understand "backwards compatibility" I suggest 2 method. (but only def _multiline_spacing(self, font, spacing, stroke_width):
return font.font.height + 2*stroke_width + spacing I think spacing must be out of def _multiline_spacing(self, font, stroke_width):
return font.font.height + 2*stroke_width Why i don't want use def _multiline_spacing(self, font, spacing, stroke_width):
return font.font.height + 2*stroke_width |
Pillow would like to continue working as expected when users upgrade to newer versions. If text changes position, this would break expectations. Alternatively, we could
Neither of these are very elegant though. |
No, Lines 753 to 756 in 95a69ec
And font.getbbox only adds stroke_width once:Lines 402 to 404 in 95a69ec
Removing all except the last return value, we are left with: top = offset[1] - stroke_width
height = size[1] + 2 * stroke_width
return ..., top + height Inline the variables: return ..., (offset[1] - stroke_width) + (size[1] + 2 * stroke_width) Rearrange: return ..., (offset[1] + size[1]) + (2 * stroke_width - stroke_width) Simplify: return ..., (offset[1] + size[1] + stroke_width)
No,
Yeah, that is what I've been leaning towards, using the version from #8092 (comment) (but perhaps as a font object member function, since we'll need a different API if we ever want to finish #6926), but I suspect the API might get quite ugly for this. |
Appreciate for explaination. I understand "backwards compatibility", "font.font.height is not height of the glyphs", "font.getbbox() add stroke_width once" But, if stroke_width > offset[1]. top coords of font.getbbox() can be minus value. so, draw.textbbox() left,top can be minus value. and finally def _multiline_spacing() is not what we def _multiline_spacing(self, font, spacing, stroke_width):
return (
self.textbbox((0, 0), "A", font, stroke_width=stroke_width)[3]
+ stroke_width
+ spacing
) == offset[1] + size[1] + 2*stroke_width + spacing It doesn't mean line_spacing to me, I expected, def _multiline_spacing(self, font, stroke_width, spacing):
size, offset = font.font.getsize('A')
return (
max(offset[1], stroke_width) # upside
+ size[1]
+ stroke_width # downside
+ spacing or consider def _multiline_spacing(self, font, stroke_width, spacing):
size, offset = font.font.getsize('A')
return (
max(font.font.height,
max(offset[1], stroke_width) # upside
+ size[1]
+ stroke_width # downside
+ spacing
)
) |
Have a look at this diagram: https://pillow.readthedocs.io/en/stable/handbook/text-anchors.html#quick-reference We currently calculate the line spacing as the sum of:
It is not a good idea to use the def _multiline_spacing(self, font, stroke_width, spacing):
size, offset = font.font.getsize('A')
return (
offset[1] + stroke_width # top
+ size[1] + stroke_width # bottom
+ spacing This would be similar to what we currently use, but instead of measuring the letter 'A' from the ascender line, it would be measuring it from the top line (specific to the letter 'A'), so even less predictable than currently. The value
I believe this would be better than what we use since:
However, as stated above, we can't easily make this change without breaking backwards compatibility. |
Shall I close this and let the problem be followed in #1646 then? |
What did you do?
Rendering (multi line) text in left(horizontal) center(vertical) of bounding box.
What did you expect to happen?
Textbox located at left(horizontal), center(vertical)
What actually happened?
Textbox height is wrong. I don't understand
line_spacing = self._multiline_spacing(font, spacing, stroke_width)
inImageDraw.multiline_textbbox()
In
def multiline_textbbox
,self.multiline_spacing
functionself.textbbox((0, 0), "A", font, stroke_width=stroke_width)
is(x1,y1,x2,y2)
,y1
is not started from zero(In my case, 2), but using(y2+stroke_width+spacing)
is not general line_spacing I know.(even "A" is not the highest character!! ex."[", "}" ). line spacing can be different line by line. I suggestWhat are your OS, Python and Pillow versions?
I ignore line spacing, and calculate line height line by line instead of max_line_h(maybe pillow does), and using
anchor="lt"
(not "la") and adding first y adding stroke_widthThe text was updated successfully, but these errors were encountered: