Fix auto-sized glyphs with BaKoMa fonts #29936
Conversation
I don't remember the exact semantics of _get_glyph off the top of my head, but from a quick look at it, did you mean "returns" instead of "expects" (emphasis mine in the quote above)? |
Right yes. In fact, I meant that its callers expect the character codes (they use |
b019f76 to
2b0fd80
Compare
|
After a bit of checking, I've found that for 12pt@100dpi, the multi-sized glyphs are 20, 30, 40, and 50 pixels, with regular text size being 16.671875 pixels (if available). So the text in the tests should look for an interior of 8.915625, 18.320833333333333, 25.156041666666667, 32.16734375, 42.03599479166666, which should hit all sizes of glyphs. Also, removed the regular sized braces |
|
I like the new implementation a lot, it's very clear (well, relative to the complexity of the task).
|
fd3409d to
c2a5501
Compare
anntzer
left a comment
There was a problem hiding this comment.
I guess it would be OK to test only one of the output formats, as we only care that the right glyphs are selected?
Do we have anything for that yet? There's |
|
There's svgastext? |
|
Since FreeType and the related PRs are merged in the Image changes are in a separate last commit so they can be stripped before merging. |
| fig = plt.figure(figsize=(5.25, 0.75)) | ||
| fig.patch.set(visible=False) # Minimize image size. | ||
| fig.text(0.5, 0.5, text, | ||
| fig.text(0.5, 0.5, text, fontsize=16, |
There was a problem hiding this comment.
Note, I increased the size here because the existing test with dashes is quite small when converted to PNG (maybe half a pixel high, which is antialiased to two grey pixels).
b3fd7d5 to
26a7d54
Compare
If the larger glyphs for an auto-sized character in `cmex10` uses a character that is in the `latex_to_bakoma` table, then it will be mapped an extra time into `cmr10` (usually). Thus we end up with a large version of a "normal" character, such as an exclamation point. Instead map these glyphs through the `latex_to_bakoma` table by using their glyph names as "commands". This ensures they don't get double-mapped to the wrong font and fixes the following issues: - slash (/) uses a comma at the larger sizes - right parenthesis uses an exclamation point at the largest size - left and right braces use parentheses at the largest size - right floor uses a percentage sign at the largest size - left ceiling uses an ampersand at the largest size Also, drop the regular size braces, as they are the same as the first `big`-sized version.
26a7d54 to
3ba2c13
Compare
|
Rebased without test images (which were moved to the |
PR summary
If the larger glyphs for an auto-sized character in
cmex10uses a character that is in thelatex_to_bakomatable, then it will be mapped an extra time intocmr10(usually). Thus we end up with a large version of a "normal" character, such as an exclamation point.Instead map these glyphs through the
latex_to_bakomatable by using their glyph names as "commands". This ensures they don't get double-mapped to the wrong font and fixes the following issues:It also found the missing
[size, so it and the corresponding]are now available again (causing the one test image change.)In the future, it may make sense to only use the glyph names (using
FT2Font.get_name_indexinstead ofFT2Font.get_char_index), and not have a mapping to character codes in our code. Unfortunately,TruetypeFonts._get_glyphexpects a font+character code, so this would be a much larger refactor than this bugfix.I'm turning the above images into tests, but I'm finding that characters that have a "regular" glyph may not be able to select the smallest "big" glyph, because it's actually smaller that the regular one. I'm not yet sure if that's a bug in the selection logic fromSince I'll be adding some test images, this PR can wait until after the FreeType 2.13 change.AutoHeightCharor whether regular glyphs shouldn't count.Fixes #5210 (and #18740 and #29886)
PR checklist