| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
I don't see any obvious problems, further reviews also welcome :)
Sorry, something went wrong.
| # 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], | ||
| ) |
There was a problem hiding this comment.
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.
Sorry, something went wrong.
|
Thank you! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
First part of #4724, refactoring the main font functions.
Changes proposed in this pull request:
visually comparing the output gives an almost imperceptible spacing difference
These changes require modifying the test_imagefontctl test images, but in most cases there is only a minor spacing change, a vertical or horizontal adjustment due to #4553 and related issues, or the original image even had clipped text (e.g. test_direction_ttb)
Text clipping comparisons
I added some test code to compare alignment in getsize and render. In case of mismatch a warning is generated in test branches. I tested both the standard test suite, as well as the following extra test cases:
Test cases
For each test case I give the generated images with basic layout first, raqm second.
master
(click to expand)It was a bit tricky to figure out which variables are best to compare, but I think I got the best possible match between getsize and render, see branch master...nulano:anchor-part1-clip-test
15 extra warnings in test suite (7 due to clipping, 8 due to different alignment).
c:\git\pillow\src\PIL\ImageFont.py:490: RuntimeWarning: Text is clipped! Measured: (0, 25, [(1, 0), (21, 0), (37, 0), (55, 0), (72, 0), (90, 1), (109, 0), (127, 1), (144, 0), (162, 0)]) Rendered: (0, 24, [(1, -1), (21, -1), (37, -1), (55, -1), (72, -1), (90, 0), (109, -1), (127, 0), (144, -1), (162, -1)]) Clipped glyphs: [(0, 'top', 1), (1, 'top', 1), (2, 'top', 1), (3, 'top', 1), (4, 'top', 1), (6, 'top', 1), (8, 'top', 1), (9, 'top', 1)] warnings.warn(m, RuntimeWarning)c:\git\pillow\src\PIL\ImageFont.py:490: RuntimeWarning: Text is clipped! Measured: (0, 25, [(1, 0), (21, 0), (36, 0), (54, 0), (70, 0), (88, 1), (106, 0), (124, 1), (140, 0), (158, 0)]) Rendered: (0, 24, [(1, -1), (21, -1), (36, -1), (54, -1), (70, -1), (88, 0), (106, -1), (124, 0), (140, -1), (158, -1)]) Clipped glyphs: [(0, 'top', 1), (1, 'top', 1), (2, 'top', 1), (3, 'top', 1), (4, 'top', 1), (6, 'top', 1), (8, 'top', 1), (9, 'top', 1)] warnings.warn(m, RuntimeWarning)PR
(click to expand)There were no problems writing debug code here, as both functions now have the same variables with the same names and meaning, see branch nulano/Pillow@anchor-part1...nulano:anchor4
no warnings in test suite
c:\git\pillow\src\PIL\ImageFont.py:498: RuntimeWarning: Text is misaligned! Measured: (0, 10, [(1, 0), (11, 1), (19, 1), (28, 1), (37, 1), (46, 1), (55, 1), (64, 1), (73, 1), (82, 1), (91, 0), (101, 1), (109, 1), (118, 1), (127, 1), (136, 1), (145, 1), (154, 1), (163, 1), (172, 1)]) Rendered: (0, 9, [(1, 0), (11, 0), (19, 0), (28, 0), (37, 0), (46, 0), (55, 0), (64, 0), (73, 0), (82, 0), (91, 0), (101, 0), (109, 0), (118, 0), (127, 0), (136, 0), (145, 0), (154, 0), (163, 0), (172, 0)]) warnings.warn(m, RuntimeWarning)c:\git\pillow\src\PIL\ImageFont.py:498: RuntimeWarning: Text is misaligned! Measured: (0, 10, [(1, 0), (11, 1), (20, 1), (29, 1), (38, 1), (47, 1), (57, 1), (66, 1), (75, 1), (84, 1), (94, 0), (104, 1), (112, 1), (121, 1), (131, 1), (140, 1), (149, 1), (158, 1), (168, 1), (177, 1)]) Rendered: (0, 9, [(1, 0), (11, 0), (20, 0), (29, 0), (38, 0), (47, 0), (57, 0), (66, 0), (75, 0), (84, 0), (94, 0), (104, 0), (112, 0), (121, 0), (131, 0), (140, 0), (149, 0), (158, 0), (168, 0), (177, 0)]) warnings.warn(m, RuntimeWarning)These warnings are not surprising due to the following from the FreeType tutorial II section 4b (emphasis mine):
Fixing this would essentially require reverting #4652, so it is up for discussion. However, unlike master, the PR at least renders this case properly.
Regarding changing FT_Outline_Get_CBox with FT_Outline_Get_BBox, it is my understanding that the former always returns a larger or same bbox, so is likely unnecessary to change. From the documentation for FT_Outline_Get_CBox (emphasis mine):