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

Refactor font_getsize and font_render #4910

Merged
merged 10 commits into from Sep 19, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file modified Tests/images/test_arabictext_features.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_complex_unicode_text.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_complex_unicode_text2.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_direction_ltr.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_direction_rtl.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_direction_ttb.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_direction_ttb_stroke.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_kerning_features.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_language.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_ligature_features.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_text.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_x_max_and_y_offset.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/test_y_offset.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_adobe.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_adobe_axes.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_adobe_name.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_adobe_older_harfbuzz.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_adobe_older_harfbuzz_axes.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_adobe_older_harfbuzz_name.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_tiny_axes.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/variation_tiny_name.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 24 additions & 8 deletions Tests/test_imagefont.py
Expand Up @@ -35,9 +35,24 @@ class TestImageFont:
# Freetype has different metrics depending on the version.
# (and, other things, but first things first)
METRICS = {
(">=2.3", "<2.4"): {"multiline": 30, "textsize": 12, "getters": (13, 16)},
(">=2.7",): {"multiline": 6.2, "textsize": 2.5, "getters": (12, 16)},
"Default": {"multiline": 0.5, "textsize": 0.5, "getters": (12, 16)},
(">=2.3", "<2.4"): {
"multiline": 30,
"textsize": 12,
"getters": (13, 16),
"mask": (107, 13),
},
(">=2.7",): {
"multiline": 6.2,
"textsize": 2.5,
"getters": (12, 16),
"mask": (108, 13),
},
"Default": {
"multiline": 0.5,
"textsize": 0.5,
"getters": (12, 16),
"mask": (108, 13),
},
}

@classmethod
Expand Down Expand Up @@ -343,7 +358,7 @@ def test_rotated_transposed_font_get_mask(self):
mask = transposed_font.getmask(text)

# Assert
assert mask.size == (13, 108)
assert mask.size == self.metrics["mask"][::-1]

def test_unrotated_transposed_font_get_mask(self):
# Arrange
Expand All @@ -356,7 +371,7 @@ def test_unrotated_transposed_font_get_mask(self):
mask = transposed_font.getmask(text)

# Assert
assert mask.size == (108, 13)
assert mask.size == self.metrics["mask"]

def test_free_type_font_get_name(self):
# Arrange
Expand Down Expand Up @@ -400,7 +415,7 @@ def test_free_type_font_get_mask(self):
mask = font.getmask(text)

# Assert
assert mask.size == (108, 13)
assert mask.size == self.metrics["mask"]

def test_load_path_not_found(self):
# Arrange
Expand Down Expand Up @@ -471,7 +486,8 @@ def test_unicode_extended(self):
d = ImageDraw.Draw(img)
d.text((10, 10), text, font=ttf)

assert_image_similar_tofile(img, target, self.metrics["multiline"])
# fails with 14.7
assert_image_similar_tofile(img, target, 6.2)

def _test_fake_loading_font(self, monkeypatch, path_to_fake, fontname):
# Make a copy of FreeTypeFont so we can patch the original
Expand Down Expand Up @@ -728,7 +744,7 @@ def test_variation_set_by_axes(self):

font = ImageFont.truetype("Tests/fonts/AdobeVFPrototype.ttf", 36)
font.set_variation_by_axes([500, 50])
self._check_text(font, "Tests/images/variation_adobe_axes.png", 5.1)
self._check_text(font, "Tests/images/variation_adobe_axes.png", 11.05)

font = ImageFont.truetype("Tests/fonts/TINY5x3GX.ttf", 36)
font.set_variation_by_axes([100])
Expand Down
3 changes: 2 additions & 1 deletion src/PIL/ImageFont.py
Expand Up @@ -259,9 +259,10 @@ def getsize(

:return: (width, height)
"""
# vertical offset is added for historical reasons, see discussion in #4789
size, offset = self.font.getsize(text, False, direction, features, language)
return (
size[0] + stroke_width * 2 + offset[0],
size[0] + stroke_width * 2,
size[1] + stroke_width * 2 + offset[1],
)
Copy link
Contributor Author

@nulano nulano Sep 10, 2020

Choose a reason for hiding this comment

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

This PR fixes an issue introduced in #2576 (Pillow 4.2.0) where the x offset was sometimes added (subtracted) twice in the C function, see #4789 (comment). This change adjusts the Python function to match this behaviour after fixing the C function.

The offset was first added in #784 to match pre-#45/#185 behaviour (and make getsize return the bottom right text coordinate, which used to match the size), but #2576 effectively reverted this change for the horizontal axis when the first character has a negative X bearing in horizontal text (otherwise the x offset is 0).

I think it is better to stick with the current behaviour (which returns the actual width but might be confusing if the first character has a negative bearing) and document it (in part 3 of #4724) rather than reverting back to the previous behaviour, as it has been this way for a long time now.

I have updated the comment to link to this explanation for future reference.


Expand Down