Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Deprecate setting text kerning factor to any non-None value#30322
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:text-overhaul
Are you sure you want to change the base?
Conversation
76132af to30280d0Compare| ..plot:: | ||
| import matplotlib.pyplot as plt | ||
| # Use old kerning values: | ||
| plt.rcParams['text.kerning_factor'] = 6 | ||
| fig, ax = plt.subplots() | ||
| ax.text(0.0, 0.05, 'BRAVO\nAWKWARD\nVAT\nW.Test', fontsize=56) | ||
| ax.set_title('Before (text.kerning_factor = 6)') | ||
| Note how the spacing between characters is uniform between their bounding boxes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I decided to remove this plot from the old what's new, as it will no longer show any difference between it and the plot below (with the correct results) once we switch to libraqm, and so it might be misleading.
| @@ -188,7 +191,7 @@ def test_ft2font_clear(): | |||
| def test_ft2font_set_size(): | |||
| file = fm.findfont('DejaVu Sans') | |||
| font = ft2font.FT2Font(file, hinting_factor=1, _kerning_factor=1) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This being 1 seems to have been a typo. Since the test checks relative results, it made no difference here.
The rcParams depreciation machinery was removed per#29563. But you can still dreprecate rcParams if you want to
|
OK, I've added a deprecation for the rcParam directly, then. |
| Due to internal changes to support complex text rendering, the kerning factor on fonts is | ||
| no longer used. Setting the ``text.kerning_factor`` rcParam (which existed only for | ||
| backwards-compatibility) to any value other than 0 is deprecated, and the rcParam will be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It looks like in the deprecation warning it warns for a value!=None, not a value!=0. Am I missing something, or is this changelog entry wrong with respect to the current code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No, quite right. It originally warned on non-0, but once I added the rcParam as well, I switched to non-None instead.
This factor existed only to preserve test images, but as ofmatplotlib#29816, itis set to 0 (i.e., disabled and providing default behaviour). In thefuture, with libraqm, it will have no effect no matter its setting(because we won't be applying kerning ourselves at all.)
PR summary
This factor existed only to preserve test images, but as of#29816, it is set to 0 (i.e., disabled and providing default behaviour). In the future, with libraqm, it will have no effect no matter its setting (because we won't be applying kerning ourselves at all.)
Since we seem to have removed most of the rcParam deprecation machinery, instead of warning from there, warn from the
FT2Fontconstructor, which is always passed_kerning_factorin the normal code path (i.e., anywhere outside of direct tests.) This does mean the warning won't occur directly where you set it, nor if you don't use any text, but I think that's a reasonable compromise.PR checklist