From 91ec958b33f30aa0374b53020628394e8dc31c47 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 29 Jul 2019 01:22:19 -0400 Subject: [PATCH 1/6] Cleanup some unnecessary FT2Font class members. --- src/ft2font.cpp | 2 ++ src/ft2font.h | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ft2font.cpp b/src/ft2font.cpp index 5fca6c3709dd..e73e8855a5b6 100644 --- a/src/ft2font.cpp +++ b/src/ft2font.cpp @@ -611,6 +611,8 @@ int FT2Font::get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode) void FT2Font::set_text( size_t N, uint32_t *codepoints, double angle, FT_Int32 flags, std::vector &xys) { + FT_Matrix matrix; /* transformation matrix */ + angle = angle / 360.0 * 2 * M_PI; // this computes width and height in subpixels so we have to divide by 64 diff --git a/src/ft2font.h b/src/ft2font.h index 24f7daaac4f9..10848fb7a01b 100644 --- a/src/ft2font.h +++ b/src/ft2font.h @@ -117,18 +117,12 @@ class FT2Font private: FT2Image image; FT_Face face; - FT_Matrix matrix; /* transformation matrix */ FT_Vector pen; /* untransformed origin */ std::vector glyphs; - std::vector pos; FT_BBox bbox; FT_Pos advance; - double ptsize; - double dpi; long hinting_factor; - void set_scalable_attributes(); - // prevent copying FT2Font(const FT2Font &); FT2Font &operator=(const FT2Font &); From a8bb386489dadf34a792227f8828e23d2216f1b3 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 29 Jul 2019 01:59:42 -0400 Subject: [PATCH 2/6] Fix minor typo in kerning example. --- examples/misc/font_indexing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/misc/font_indexing.py b/examples/misc/font_indexing.py index 5599eb313707..f629306c0aae 100644 --- a/examples/misc/font_indexing.py +++ b/examples/misc/font_indexing.py @@ -35,4 +35,4 @@ print('AV', font.get_kerning(glyphd['A'], glyphd['V'], KERNING_DEFAULT)) print('AV', font.get_kerning(glyphd['A'], glyphd['V'], KERNING_UNFITTED)) print('AV', font.get_kerning(glyphd['A'], glyphd['V'], KERNING_UNSCALED)) -print('AV', font.get_kerning(glyphd['A'], glyphd['T'], KERNING_UNSCALED)) +print('AT', font.get_kerning(glyphd['A'], glyphd['T'], KERNING_UNSCALED)) From c404b9f680217c352f52dab4a18d59d0feecf929 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 29 Jul 2019 02:00:44 -0400 Subject: [PATCH 3/6] Use slightly more correct types for FreeType. --- src/ft2font.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ft2font.cpp b/src/ft2font.cpp index e73e8855a5b6..5ef85a34c014 100644 --- a/src/ft2font.cpp +++ b/src/ft2font.cpp @@ -568,7 +568,7 @@ void FT2Font::clear() void FT2Font::set_size(double ptsize, double dpi) { FT_Error error = FT_Set_Char_Size( - face, (long)(ptsize * 64), 0, (unsigned int)(dpi * hinting_factor), (unsigned int)dpi); + face, (FT_F26Dot6)(ptsize * 64), 0, (FT_UInt)(dpi * hinting_factor), (FT_UInt)dpi); if (error) { throw_ft_error("Could not set the fontsize", error); } @@ -661,7 +661,7 @@ void FT2Font::set_text( xys.push_back(pen.x); xys.push_back(pen.y); - FT_Glyph_Get_CBox(thisGlyph, ft_glyph_bbox_subpixels, &glyph_bbox); + FT_Glyph_Get_CBox(thisGlyph, FT_GLYPH_BBOX_SUBPIXELS, &glyph_bbox); bbox.xMin = std::min(bbox.xMin, glyph_bbox.xMin); bbox.xMax = std::max(bbox.xMax, glyph_bbox.xMax); From 7f57740a62b119467895cc754ae6a8adb9bbb22f Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 29 Jul 2019 01:56:36 -0400 Subject: [PATCH 4/6] Fix calculation of kerning values. For the straight `get_kerning` call, the result should be in subpixels, since the callers (in Python) correctly scale it by 64 themselves. For the glyph advancement calculation, both `pen` (used for glyph transform and with content boxes) and kerning are in 26.6 fractional pixels. Therefore, there's no need to scale either case by 2**6 (64). --- src/ft2font.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ft2font.cpp b/src/ft2font.cpp index 5ef85a34c014..9317debdc87b 100644 --- a/src/ft2font.cpp +++ b/src/ft2font.cpp @@ -602,7 +602,7 @@ int FT2Font::get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode) FT_Vector delta; if (!FT_Get_Kerning(face, left, right, mode, &delta)) { - return (int)(delta.x) / (hinting_factor << 6); + return (int)(delta.x) / hinting_factor; } else { return 0; } @@ -640,7 +640,7 @@ void FT2Font::set_text( if (use_kerning && previous && glyph_index) { FT_Vector delta; FT_Get_Kerning(face, previous, glyph_index, FT_KERNING_DEFAULT, &delta); - pen.x += (delta.x << 10) / (hinting_factor << 16); + pen.x += delta.x / hinting_factor; } if (FT_Error error = FT_Load_Glyph(face, glyph_index, flags)) { throw_ft_error("Could not load glyph", error); From a1df6466d3d9522908cee15fc364e477a1b2a57f Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 31 Jul 2019 05:26:07 -0400 Subject: [PATCH 5/6] Add an rcParam to restore old kerning behaviour. This is enabled for the classic test style, so that the 100 or so old images do not need to be regenerated. --- lib/matplotlib/font_manager.py | 3 ++- .../mpl-data/stylelib/_classic_test_patch.mplstyle | 2 ++ lib/matplotlib/rcsetup.py | 1 + lib/matplotlib/tests/test_artist.py | 3 +++ lib/matplotlib/tests/test_axes.py | 6 ++++++ lib/matplotlib/tests/test_image.py | 3 +++ lib/matplotlib/tests/test_legend.py | 3 +++ lib/matplotlib/tests/test_text.py | 6 ++++++ lib/matplotlib/tests/test_widgets.py | 3 +++ .../tests/test_axisartist_axis_artist.py | 9 +++++++++ lib/mpl_toolkits/tests/test_axisartist_axislines.py | 6 ++++++ .../tests/test_axisartist_floating_axes.py | 3 +++ .../tests/test_axisartist_grid_helper_curvelinear.py | 6 ++++++ matplotlibrc.template | 4 ++++ src/ft2font.cpp | 12 ++++++++++-- src/ft2font.h | 2 ++ src/ft2font_wrapper.cpp | 8 ++++++-- 17 files changed, 75 insertions(+), 5 deletions(-) diff --git a/lib/matplotlib/font_manager.py b/lib/matplotlib/font_manager.py index 95e821a48af8..dfbdfb209e1c 100644 --- a/lib/matplotlib/font_manager.py +++ b/lib/matplotlib/font_manager.py @@ -1327,7 +1327,8 @@ def is_opentype_cff_font(filename): def get_font(filename, hinting_factor=None): if hinting_factor is None: hinting_factor = rcParams['text.hinting_factor'] - return _get_font(os.fspath(filename), hinting_factor) + return _get_font(os.fspath(filename), hinting_factor, + _kerning_factor=rcParams['text.kerning_factor']) def _rebuild(): diff --git a/lib/matplotlib/mpl-data/stylelib/_classic_test_patch.mplstyle b/lib/matplotlib/mpl-data/stylelib/_classic_test_patch.mplstyle index dc15b7e9a7a4..96f62f4ba592 100644 --- a/lib/matplotlib/mpl-data/stylelib/_classic_test_patch.mplstyle +++ b/lib/matplotlib/mpl-data/stylelib/_classic_test_patch.mplstyle @@ -1,4 +1,6 @@ # This patch should go on top of the "classic" style and exists solely to avoid # changing baseline images. +text.kerning_factor : 6 + ytick.alignment: center_baseline diff --git a/lib/matplotlib/rcsetup.py b/lib/matplotlib/rcsetup.py index 74a8efe365ed..e0a7d14dc747 100644 --- a/lib/matplotlib/rcsetup.py +++ b/lib/matplotlib/rcsetup.py @@ -1091,6 +1091,7 @@ def _validate_linestyle(ls): 'text.latex.preview': [False, validate_bool], 'text.hinting': ['auto', validate_hinting], 'text.hinting_factor': [8, validate_int], + 'text.kerning_factor': [0, validate_int], 'text.antialiased': [True, validate_bool], 'mathtext.cal': ['cursive', validate_font_properties], diff --git a/lib/matplotlib/tests/test_artist.py b/lib/matplotlib/tests/test_artist.py index 5a987ec01808..a598e822d2d6 100644 --- a/lib/matplotlib/tests/test_artist.py +++ b/lib/matplotlib/tests/test_artist.py @@ -201,6 +201,9 @@ def test_remove(): @image_comparison(["default_edges.png"], remove_text=True, style='default') def test_default_edges(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, [[ax1, ax2], [ax3, ax4]] = plt.subplots(2, 2) ax1.plot(np.arange(10), np.arange(10), 'x', diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index 7797f3103396..01110ae26855 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/lib/matplotlib/tests/test_axes.py @@ -48,6 +48,9 @@ def test_get_labels(): @image_comparison(['acorr.png'], style='mpl20') def test_acorr(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + np.random.seed(19680801) n = 512 x = np.random.normal(0, 1, n).cumsum() @@ -5730,6 +5733,9 @@ def test_axisbelow(): @image_comparison(['titletwiny.png'], style='mpl20') def test_titletwiny(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + # Test that title is put above xlabel if xlabel at top fig, ax = plt.subplots() fig.subplots_adjust(top=0.8) diff --git a/lib/matplotlib/tests/test_image.py b/lib/matplotlib/tests/test_image.py index 7f370c9051d1..fc05ae6894b7 100644 --- a/lib/matplotlib/tests/test_image.py +++ b/lib/matplotlib/tests/test_image.py @@ -27,6 +27,9 @@ @image_comparison(['image_interps'], style='mpl20') def test_image_interps(): 'make the basic nearest, bilinear and bicubic interps' + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + X = np.arange(100) X = X.reshape(5, 20) diff --git a/lib/matplotlib/tests/test_legend.py b/lib/matplotlib/tests/test_legend.py index aa3f51b69a83..71499da446fa 100644 --- a/lib/matplotlib/tests/test_legend.py +++ b/lib/matplotlib/tests/test_legend.py @@ -188,6 +188,9 @@ def test_legend_expand(): @image_comparison(['hatching'], remove_text=True, style='default') def test_hatching(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, ax = plt.subplots() # Patches diff --git a/lib/matplotlib/tests/test_text.py b/lib/matplotlib/tests/test_text.py index ac5d87f26d57..f10a97844e6c 100644 --- a/lib/matplotlib/tests/test_text.py +++ b/lib/matplotlib/tests/test_text.py @@ -130,6 +130,9 @@ def test_multiline(): @image_comparison(['multiline2'], style='mpl20') def test_multiline2(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, ax = plt.subplots() ax.set_xlim([0, 1.4]) @@ -580,6 +583,9 @@ def test_annotation_update(): @image_comparison(['large_subscript_title.png'], style='mpl20') def test_large_subscript_title(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, axs = plt.subplots(1, 2, figsize=(9, 2.5), constrained_layout=True) ax = axs[0] ax.set_title(r'$\sum_{i} x_i$') diff --git a/lib/matplotlib/tests/test_widgets.py b/lib/matplotlib/tests/test_widgets.py index 94356621e777..0d50389ae35f 100644 --- a/lib/matplotlib/tests/test_widgets.py +++ b/lib/matplotlib/tests/test_widgets.py @@ -264,6 +264,9 @@ def test_CheckButtons(): @image_comparison(['check_radio_buttons.png'], style='mpl20', remove_text=True) def test_check_radio_buttons_image(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + get_ax() plt.subplots_adjust(left=0.3) rax1 = plt.axes([0.05, 0.7, 0.15, 0.15]) diff --git a/lib/mpl_toolkits/tests/test_axisartist_axis_artist.py b/lib/mpl_toolkits/tests/test_axisartist_axis_artist.py index b17855392e08..1bebbfd9b81d 100644 --- a/lib/mpl_toolkits/tests/test_axisartist_axis_artist.py +++ b/lib/mpl_toolkits/tests/test_axisartist_axis_artist.py @@ -26,6 +26,9 @@ def test_ticks(): @image_comparison(['axis_artist_labelbase.png'], style='default') def test_labelbase(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, ax = plt.subplots() ax.plot([0.5], [0.5], "o") @@ -40,6 +43,9 @@ def test_labelbase(): @image_comparison(['axis_artist_ticklabels.png'], style='default') def test_ticklabels(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, ax = plt.subplots() ax.xaxis.set_visible(False) @@ -72,6 +78,9 @@ def test_ticklabels(): @image_comparison(['axis_artist.png'], style='default') def test_axis_artist(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig, ax = plt.subplots() ax.xaxis.set_visible(False) diff --git a/lib/mpl_toolkits/tests/test_axisartist_axislines.py b/lib/mpl_toolkits/tests/test_axisartist_axislines.py index 386ba7af20a5..3270e233d16f 100644 --- a/lib/mpl_toolkits/tests/test_axisartist_axislines.py +++ b/lib/mpl_toolkits/tests/test_axisartist_axislines.py @@ -11,6 +11,9 @@ @image_comparison(['SubplotZero.png'], style='default') def test_SubplotZero(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig = plt.figure() ax = SubplotZero(fig, 1, 1, 1) @@ -29,6 +32,9 @@ def test_SubplotZero(): @image_comparison(['Subplot.png'], style='default') def test_Subplot(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig = plt.figure() ax = Subplot(fig, 1, 1, 1) diff --git a/lib/mpl_toolkits/tests/test_axisartist_floating_axes.py b/lib/mpl_toolkits/tests/test_axisartist_floating_axes.py index c47de58f77c8..e69b63f99c2b 100644 --- a/lib/mpl_toolkits/tests/test_axisartist_floating_axes.py +++ b/lib/mpl_toolkits/tests/test_axisartist_floating_axes.py @@ -74,6 +74,9 @@ def test_curvelinear3(): @image_comparison(['curvelinear4.png'], style='default', tol=0.015) def test_curvelinear4(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig = plt.figure(figsize=(5, 5)) tr = (mtransforms.Affine2D().scale(np.pi / 180, 1) + diff --git a/lib/mpl_toolkits/tests/test_axisartist_grid_helper_curvelinear.py b/lib/mpl_toolkits/tests/test_axisartist_grid_helper_curvelinear.py index 632dfb195a0d..279853928568 100644 --- a/lib/mpl_toolkits/tests/test_axisartist_grid_helper_curvelinear.py +++ b/lib/mpl_toolkits/tests/test_axisartist_grid_helper_curvelinear.py @@ -85,6 +85,9 @@ def inverted(self): @image_comparison(['polar_box.png'], style='default', tol={'aarch64': 0.04}.get(platform.machine(), 0.03)) def test_polar_box(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig = plt.figure(figsize=(5, 5)) # PolarAxes.PolarTransform takes radian. However, we want our coordinate @@ -145,6 +148,9 @@ def test_polar_box(): @image_comparison(['axis_direction.png'], style='default', tol=0.03) def test_axis_direction(): + # Remove this line when this test image is regenerated. + plt.rcParams['text.kerning_factor'] = 6 + fig = plt.figure(figsize=(5, 5)) # PolarAxes.PolarTransform takes radian. However, we want our coordinate diff --git a/matplotlibrc.template b/matplotlibrc.template index 8aa38a876b88..5cd1e2d7a80d 100644 --- a/matplotlibrc.template +++ b/matplotlibrc.template @@ -316,6 +316,10 @@ #text.hinting_factor : 8 ## Specifies the amount of softness for hinting in the ## horizontal direction. A value of 1 will hint to full ## pixels. A value of 2 will hint to half pixels etc. +#text.kerning_factor : 0 ## Specifies the scaling factor for kerning values. This + ## is provided solely to allow old test images to remain + ## unchanged. Set to 6 to obtain previous behavior. Values + ## other than 0 or 6 have no defined meaning. #text.antialiased : True ## If True (default), the text will be antialiased. ## This only affects the Agg backend. diff --git a/src/ft2font.cpp b/src/ft2font.cpp index 9317debdc87b..3fc9e8bad015 100644 --- a/src/ft2font.cpp +++ b/src/ft2font.cpp @@ -525,6 +525,9 @@ FT2Font::FT2Font(FT_Open_Args &open_args, long hinting_factor_) : image(), face( throw_ft_error("Can not load face", error); } + // set default kerning factor to 0, i.e., no kerning manipulation + kerning_factor = 0; + // set a default fontsize 12 pt at 72dpi hinting_factor = hinting_factor_; @@ -602,12 +605,17 @@ int FT2Font::get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode) FT_Vector delta; if (!FT_Get_Kerning(face, left, right, mode, &delta)) { - return (int)(delta.x) / hinting_factor; + return (int)(delta.x) / (hinting_factor << kerning_factor); } else { return 0; } } +void FT2Font::set_kerning_factor(int factor) +{ + kerning_factor = factor; +} + void FT2Font::set_text( size_t N, uint32_t *codepoints, double angle, FT_Int32 flags, std::vector &xys) { @@ -640,7 +648,7 @@ void FT2Font::set_text( if (use_kerning && previous && glyph_index) { FT_Vector delta; FT_Get_Kerning(face, previous, glyph_index, FT_KERNING_DEFAULT, &delta); - pen.x += delta.x / hinting_factor; + pen.x += delta.x / (hinting_factor << kerning_factor); } if (FT_Error error = FT_Load_Glyph(face, glyph_index, flags)) { throw_ft_error("Could not load glyph", error); diff --git a/src/ft2font.h b/src/ft2font.h index 10848fb7a01b..ea48e7f49bae 100644 --- a/src/ft2font.h +++ b/src/ft2font.h @@ -74,6 +74,7 @@ class FT2Font void set_text( size_t N, uint32_t *codepoints, double angle, FT_Int32 flags, std::vector &xys); int get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode); + void set_kerning_factor(int factor); void load_char(long charcode, FT_Int32 flags); void load_glyph(FT_UInt glyph_index, FT_Int32 flags); void get_width_height(long *width, long *height); @@ -122,6 +123,7 @@ class FT2Font FT_BBox bbox; FT_Pos advance; long hinting_factor; + int kerning_factor; // prevent copying FT2Font(const FT2Font &); diff --git a/src/ft2font_wrapper.cpp b/src/ft2font_wrapper.cpp index ef2ac5fc87f8..a890c2deda08 100644 --- a/src/ft2font_wrapper.cpp +++ b/src/ft2font_wrapper.cpp @@ -553,10 +553,12 @@ static int PyFT2Font_init(PyFT2Font *self, PyObject *args, PyObject *kwds) PyObject *fname; FT_Open_Args open_args; long hinting_factor = 8; - const char *names[] = { "filename", "hinting_factor", NULL }; + int kerning_factor = 0; + const char *names[] = { "filename", "hinting_factor", "_kerning_factor", NULL }; if (!PyArg_ParseTupleAndKeywords( - args, kwds, "O|l:FT2Font", (char **)names, &fname, &hinting_factor)) { + args, kwds, "O|l$i:FT2Font", (char **)names, &fname, + &hinting_factor, &kerning_factor)) { return -1; } @@ -567,6 +569,8 @@ static int PyFT2Font_init(PyFT2Font *self, PyObject *args, PyObject *kwds) CALL_CPP_FULL( "FT2Font", (self->x = new FT2Font(open_args, hinting_factor)), PyFT2Font_fail(self), -1); + CALL_CPP("FT2Font->set_kerning_factor", (self->x->set_kerning_factor(kerning_factor))); + Py_INCREF(fname); self->fname = fname; From f10da0ce6e8d2dbf3f59e5da1d2a162a24597b01 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 14 Aug 2019 04:29:21 -0400 Subject: [PATCH 6/6] Add what's new entry about fixed kerning. --- doc/users/next_whats_new/2019-08-14-ES.rst | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 doc/users/next_whats_new/2019-08-14-ES.rst diff --git a/doc/users/next_whats_new/2019-08-14-ES.rst b/doc/users/next_whats_new/2019-08-14-ES.rst new file mode 100644 index 000000000000..b29221bc91cf --- /dev/null +++ b/doc/users/next_whats_new/2019-08-14-ES.rst @@ -0,0 +1,33 @@ +Kerning adjustments now use correct values +------------------------------------------ + +Due to an error in how kerning adjustments were applied, previous versions of +Matplotlib would under-correct kerning. This version will now correctly apply +kerning (for fonts supported by FreeType). To restore the old behavior (e.g., +for test images), you may set :rc:`text.kerning_factor` to 6 (instead of 0). +Other values have undefined behavior. + +.. 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 +(above). With corrected kerning (below), slanted characters (e.g., AV or VA) +will be spaced closer together, as well as various other character pairs, +depending on font support (e.g., T and e, or the period after the W). + +.. plot:: + + import matplotlib.pyplot as plt + + # Use new kerning values: + plt.rcParams['text.kerning_factor'] = 0 + fig, ax = plt.subplots() + ax.text(0.0, 0.05, 'BRAVO\nAWKWARD\nVAT\nW.Test', fontsize=56) + ax.set_title('After (text.kerning_factor = 0)')