Skip to content

Use FT_LOAD_TARGET_MONO in getsize#4664

Merged
hugovk merged 2 commits intopython-pillow:masterfrom
nulano:ft-getsize-mode
Jun 21, 2020
Merged

Use FT_LOAD_TARGET_MONO in getsize#4664
hugovk merged 2 commits intopython-pillow:masterfrom
nulano:ft-getsize-mode

Conversation

@nulano
Copy link
Contributor

@nulano nulano commented Jun 1, 2020

Fixes #4177.

Changes proposed in this pull request:

  • Adds the mask parameter to the internal getsize function. I did not add it to the public function, as I'm not sure what is the best way to update the API in a reasonable and backwards-compatible manner.
  • Test is disabled on FreeType-2.3 due to different metrics. This affects Centos-6 and Amazon-1. It seems to use the same hinting algorithm for both targets (is hinting even enabled on those systems?), so I can't reproduce the original issue there. All other versions give identical output.
  • Raqm is unaffected, as that layout function ignores the mask parameter. I did not include it in tests, as the epsilon among systems is greater than epsilon to a blank image.
  • This PR adds the first test for mode 1 text functions. Perhaps more should be added in the future.


PyObject* string;
if (!PyArg_ParseTuple(args, "O|zOz:getsize", &string, &dir, &features, &lang)) {
if (!PyArg_ParseTuple(args, "O|izOz:getsize", &string, &mask, &dir, &features, &lang)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add the new parameter to the end, to maintain compatibility?

People have been calling ImageFont.font.getsize directly. A quick example:

font = ImageFont.truetype(fontname, fontsize)
limits = font.getsize(text)

Copy link
Contributor Author

@nulano nulano Jun 5, 2020

Choose a reason for hiding this comment

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

The linked and quoted example is the Python-layer function. It looks confusing as the code refers to the FreeTypeFont object as font, but updated function would have to be called with font.font.getsize(...). The same is true for the search results in the first few pages.

There are three functions concerning the size of text (ignoring the multiline variants):

ImageDraw.Draw.textsize(...)
ImageFont.FreeTypeFont.getsize(...)  # the Python version; I did not update this
ImageFont.FreeTypeFont.font.getsize(...)  # the C version; updated by this PR

The equivalent code would be:

font = ImageFont.truetype(...)
font.getsize(...)  # the Python function
font.font.getsize(...)  # the C function

I did not update the Python function as I think it would confusing to have the arguments in a different order, but not backwards compatible to add it at the front (suggestions are welcome).

If you still think the C function is a concern, would something with PyArg_ParseTupleAndKeywords be a good solution (i.e. making all future additions kwarg only)?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I think we're fine to change this then. The C API is internal and doesn't need to maintain backwards compatibility.

@hugovk hugovk merged commit 390b34c into python-pillow:master Jun 21, 2020
@hugovk
Copy link
Member

hugovk commented Jun 21, 2020

Thank you!

@nulano nulano deleted the ft-getsize-mode branch August 17, 2020 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Truetype fonts not being rendered correctly with the TEXT method

2 participants