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

Only call text_layout once in getmask2 #7206

Merged
merged 2 commits into from Jun 13, 2023

Conversation

radarhere
Copy link
Member

@radarhere radarhere commented Jun 10, 2023

Helps #6618

Backstory
The following is an earlier version of ImageFont's getmask2(), as described in the quoted comment.

Pillow/src/PIL/ImageFont.py

Lines 752 to 777 in 3cfdef3

if fill is _UNSPECIFIED:
fill = Image.core.fill
else:
deprecate("fill", 10)
size, offset = self.font.getsize(
text, mode, direction, features, language, anchor
)
if start is None:
start = (0, 0)
size = tuple(math.ceil(size[i] + stroke_width * 2 + start[i]) for i in range(2))
offset = offset[0] - stroke_width, offset[1] - stroke_width
Image._decompression_bomb_check(size)
im = fill("RGBA" if mode == "RGBA" else "L", size, 0)
if min(size):
self.font.render(
text,
im.id,
mode,
direction,
features,
language,
stroke_width,
ink,
start[0],
start[1],
)

#6618 (comment)

The function getmask2 performs the following steps:

  1. calls getsize to get the size of the text
  2. calls Image._decompression_bomb_check to compare size with MAX_IMAGE_PIXELS
  3. calls the fill function passed as argument to create a blank image
  4. calls render to draw text into the blank image

After Pillow 10 the deprecated fill parameter will be replaced by a direct call to the internal function. After this, the only Python function to be called between the two C functions is the decompression bomb check. If this was moved into C, the two functions could be combined to remove the duplicate call to text_layout.

#7059 removed the deprecated fill parameter. So here is the current version of ImageFont's getmask2().

Pillow/src/PIL/ImageFont.py

Lines 554 to 575 in 1fc8d82

size, offset = self.font.getsize(
text, mode, direction, features, language, anchor
)
if start is None:
start = (0, 0)
size = tuple(math.ceil(size[i] + stroke_width * 2 + start[i]) for i in range(2))
offset = offset[0] - stroke_width, offset[1] - stroke_width
Image._decompression_bomb_check(size)
im = Image.core.fill("RGBA" if mode == "RGBA" else "L", size, 0)
if min(size):
self.font.render(
text,
im.id,
mode,
direction,
features,
language,
stroke_width,
ink,
start[0],
start[1],
)

This is the only place that font.render is called.

Change
So I added a change to make font.getsize a builtin part of font.render, meaning that text_layout is not called once by each, but only once.

From my tests, this causes getmask2 to be 10% faster.

There is an awkward part of this change though - the _imagingft extension is not connected to the C code for creating new images. I couldn't call ImagingNewDirty and ImagingFill. Despite the quoted comment's expectation that this could all be done in C, it didn't realise that our C code is actually split up into these extensions. Instead, I've passed Image.core.fill into C, to then be called by PyObject_CallFunction.

@hugovk hugovk merged commit 8f3ccff into python-pillow:main Jun 13, 2023
59 of 60 checks passed
@radarhere radarhere deleted the text_layout branch June 13, 2023 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants