Skip to content

Fix text kerning calculations and some FT2Font cleanup #14940

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions doc/users/next_whats_new/2019-08-14-ES.rst
Original file line number Diff line number Diff line change
@@ -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)')
2 changes: 1 addition & 1 deletion examples/misc/font_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
3 changes: 2 additions & 1 deletion lib/matplotlib/font_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Copy link
Contributor

Choose a reason for hiding this comment

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

What about folding this into rcParams['_internal.classic_mode']? (how many images would need to be regen'd?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I didn't update the non-classic tests yet, so you can see from the CI results it'd be 21 failures.

Copy link
Contributor

Choose a reason for hiding this comment

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

Had a quick look at the failures. A few of them can be improved:

  • regen test_acorr, test_image_interps without any text (remove_text)
  • test_titletwiny can be rewritten to a non-image-comparison, just checking the title position, like test_titlesetpos which is just below it
  • for test_hatching the text labels can probably removed? (can we generate legends with empty labels? not sure...)
  • I think the others images would need to be regen'd (assuming we fold the switch into _internal.classic_mode) and need to keep the text to be meaningful.

Or perhaps make this a private rcParam (leading underscore, like _internal.classic_mode)? Or perhaps I'm just overthinking this and text.kerning_factor is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I added the rcParam for downstream testing's benefits; not sure if only classic mode will be helpful there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. I guess my preferred solution would be _internal.old_kerning or something like that, but won't insist on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened #15054 to track these suggestions, but for tests here I just modified the rcParam to use the old value.



def _rebuild():
Expand Down
2 changes: 2 additions & 0 deletions lib/matplotlib/mpl-data/stylelib/_classic_test_patch.mplstyle
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lib/matplotlib/rcsetup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/tests/test_artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
6 changes: 6 additions & 0 deletions lib/matplotlib/tests/test_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/tests/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/tests/test_legend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/matplotlib/tests/test_text.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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$')
Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/tests/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
9 changes: 9 additions & 0 deletions lib/mpl_toolkits/tests/test_axisartist_axis_artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions lib/mpl_toolkits/tests/test_axisartist_axislines.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions lib/mpl_toolkits/tests/test_axisartist_floating_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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) +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions matplotlibrc.template
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
18 changes: 14 additions & 4 deletions src/ft2font.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand Down Expand Up @@ -568,7 +571,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);
}
Expand Down Expand Up @@ -602,15 +605,22 @@ 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 << 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<double> &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
Expand Down Expand Up @@ -638,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 << 10) / (hinting_factor << 16);
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);
Expand All @@ -659,7 +669,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);
Expand Down
8 changes: 2 additions & 6 deletions src/ft2font.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class FT2Font
void set_text(
size_t N, uint32_t *codepoints, double angle, FT_Int32 flags, std::vector<double> &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);
Expand Down Expand Up @@ -117,17 +118,12 @@ class FT2Font
private:
FT2Image image;
FT_Face face;
FT_Matrix matrix; /* transformation matrix */
FT_Vector pen; /* untransformed origin */
std::vector<FT_Glyph> glyphs;
std::vector<FT_Vector> pos;
FT_BBox bbox;
FT_Pos advance;
double ptsize;
double dpi;
long hinting_factor;

void set_scalable_attributes();
int kerning_factor;

// prevent copying
FT2Font(const FT2Font &);
Expand Down
8 changes: 6 additions & 2 deletions src/ft2font_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;

Expand Down