Skip to content

Fix some more integer type inconsistencies in Freetype code #7782

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 1 commit into from
Jan 11, 2017
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
Fix some more integer type inconsistencies in Freetype code
This straightens up some more inconsistencies in the integer
types in ft2font_wrapper.cpp and ft2font.cpp. Referring to the
Freetype 2 API guide:

https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html

1) FT_Get_Kerning's integer arguments are FT_UInt (unsigned int)
but we were passing it signed ints.

2) FT_Load_Glyph's flags argument is an FT_Int32 (signed int...
usually, see footnote) but our set_text and load_glyph were
using FT_UInt32, and converting Python values to unsigned int.

3) FT_Load_Char's flags argument is an FT_Int32 (signed int...
usually, see footnote) but our load_char was using FT_UInt32,
and converting Python values to unsigned int.

4) load_char in ft2font_wrapper declared charcode as a signed
long (and load_char in ft2font does indeed expect a signed long
from the wrapper, though it then turns it into an unsigned
long, I don't know why this is set up that way), but was
converting the Python value to an unsigned long (k) not a
signed long (l).

5) get_glyph_name in ft2font_wrapper declared glyph_number as
unsigned int, and indeed ft2font is expecting an unsigned int
(Freetype is expecting an FT_UInt, which is the same thing),
but it was converting the Python value to a signed int (i)
not an unsigned int (I).

Footnote: the FT_Int32 and FT_UInt32 types that we have to use
for some values are defined as being the signed and unsigned
variants of whichever integer type is *exactly* 32 bits in
size. Freetype defines them as int if it's 32 bits, otherwise
long if *that's* 32 bits, otherwise it gives up and errors out.
However, we simply assume they're always ints, which isn't
technically correct. We could probably fix this with a bit of
`sizeof()` use, but I'm not sure if we want to bother, because
it's almost certainly the case that the int types are 32 bits
in size on all platforms matplotlib is targeting - if there are
still any significant platforms where int is 16 bits and so these
wind up as long, I'd be surprised. I thought it was worth noting,
though.
  • Loading branch information
AdamWill committed Jan 10, 2017
commit 61f256b4f3da3815a0aa4ac15f110324b7fe3d9b
8 changes: 4 additions & 4 deletions src/ft2font.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ void FT2Font::select_charmap(unsigned long i)
}
}

int FT2Font::get_kerning(int left, int right, int mode)
int FT2Font::get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode)
{
if (!FT_HAS_KERNING(face)) {
return 0;
Expand All @@ -589,7 +589,7 @@ int FT2Font::get_kerning(int left, int right, int mode)
}

void FT2Font::set_text(
size_t N, uint32_t *codepoints, double angle, FT_UInt32 flags, std::vector<double> &xys)
size_t N, uint32_t *codepoints, double angle, FT_Int32 flags, std::vector<double> &xys)
{
angle = angle / 360.0 * 2 * M_PI;

Expand Down Expand Up @@ -666,7 +666,7 @@ void FT2Font::set_text(
}
}

void FT2Font::load_char(long charcode, FT_UInt32 flags)
void FT2Font::load_char(long charcode, FT_Int32 flags)
{
int error = FT_Load_Char(face, (unsigned long)charcode, flags);

Expand All @@ -684,7 +684,7 @@ void FT2Font::load_char(long charcode, FT_UInt32 flags)
glyphs.push_back(thisGlyph);
}

void FT2Font::load_glyph(FT_UInt glyph_index, FT_UInt32 flags)
void FT2Font::load_glyph(FT_UInt glyph_index, FT_Int32 flags)
{
int error = FT_Load_Glyph(face, glyph_index, flags);

Expand Down
8 changes: 4 additions & 4 deletions src/ft2font.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ class FT2Font
void set_charmap(int i);
void select_charmap(unsigned long i);
void set_text(
size_t N, uint32_t *codepoints, double angle, FT_UInt32 flags, std::vector<double> &xys);
int get_kerning(int left, int right, int mode);
void load_char(long charcode, FT_UInt32 flags);
void load_glyph(FT_UInt glyph_index, FT_UInt32 flags);
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 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);
void get_bitmap_offset(long *x, long *y);
long get_descent();
Expand Down
28 changes: 19 additions & 9 deletions src/ft2font_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,9 +621,10 @@ const char *PyFT2Font_get_kerning__doc__ =

static PyObject *PyFT2Font_get_kerning(PyFT2Font *self, PyObject *args, PyObject *kwds)
{
int left, right, mode, result;
FT_UInt left, right, mode;
int result;

if (!PyArg_ParseTuple(args, "iii:get_kerning", &left, &right, &mode)) {
if (!PyArg_ParseTuple(args, "III:get_kerning", &left, &right, &mode)) {
return NULL;
}

Expand All @@ -643,12 +644,15 @@ static PyObject *PyFT2Font_set_text(PyFT2Font *self, PyObject *args, PyObject *k
{
PyObject *textobj;
double angle = 0.0;
FT_UInt32 flags = FT_LOAD_FORCE_AUTOHINT;
FT_Int32 flags = FT_LOAD_FORCE_AUTOHINT;
std::vector<double> xys;
const char *names[] = { "string", "angle", "flags", NULL };

/* This makes a technically incorrect assumption that FT_Int32 is
int. In theory it can also be long, if the size of int is less
than 32 bits. This is very unlikely on modern platforms. */
if (!PyArg_ParseTupleAndKeywords(
args, kwds, "O|dI:set_text", (char **)names, &textobj, &angle, &flags)) {
args, kwds, "O|di:set_text", (char **)names, &textobj, &angle, &flags)) {
return NULL;
}

Expand Down Expand Up @@ -712,11 +716,14 @@ const char *PyFT2Font_load_char__doc__ =
static PyObject *PyFT2Font_load_char(PyFT2Font *self, PyObject *args, PyObject *kwds)
{
long charcode;
FT_UInt32 flags = FT_LOAD_FORCE_AUTOHINT;
FT_Int32 flags = FT_LOAD_FORCE_AUTOHINT;
const char *names[] = { "charcode", "flags", NULL };

/* This makes a technically incorrect assumption that FT_Int32 is
int. In theory it can also be long, if the size of int is less
than 32 bits. This is very unlikely on modern platforms. */
if (!PyArg_ParseTupleAndKeywords(
args, kwds, "k|I:load_char", (char **)names, &charcode, &flags)) {
args, kwds, "l|i:load_char", (char **)names, &charcode, &flags)) {
return NULL;
}

Expand Down Expand Up @@ -747,11 +754,14 @@ const char *PyFT2Font_load_glyph__doc__ =
static PyObject *PyFT2Font_load_glyph(PyFT2Font *self, PyObject *args, PyObject *kwds)
{
FT_UInt glyph_index;
FT_UInt32 flags = FT_LOAD_FORCE_AUTOHINT;
FT_Int32 flags = FT_LOAD_FORCE_AUTOHINT;
const char *names[] = { "glyph_index", "flags", NULL };

/* This makes a technically incorrect assumption that FT_Int32 is
int. In theory it can also be long, if the size of int is less
than 32 bits. This is very unlikely on modern platforms. */
if (!PyArg_ParseTupleAndKeywords(
args, kwds, "I|I:load_glyph", (char **)names, &glyph_index, &flags)) {
args, kwds, "I|i:load_glyph", (char **)names, &glyph_index, &flags)) {
return NULL;
}

Expand Down Expand Up @@ -901,7 +911,7 @@ static PyObject *PyFT2Font_get_glyph_name(PyFT2Font *self, PyObject *args, PyObj
unsigned int glyph_number;
char buffer[128];

if (!PyArg_ParseTuple(args, "i:get_glyph_name", &glyph_number)) {
if (!PyArg_ParseTuple(args, "I:get_glyph_name", &glyph_number)) {
return NULL;
}

Expand Down