Skip to content

Commit 61f256b

Browse files
AdamWillAdamWill
committed
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.
1 parent 109251f commit 61f256b

File tree

3 files changed

+27
-17
lines changed

3 files changed

+27
-17
lines changed

‎src/ft2font.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ void FT2Font::select_charmap(unsigned long i)
574574
}
575575
}
576576

577-
int FT2Font::get_kerning(int left, int right, int mode)
577+
int FT2Font::get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode)
578578
{
579579
if (!FT_HAS_KERNING(face)) {
580580
return 0;
@@ -589,7 +589,7 @@ int FT2Font::get_kerning(int left, int right, int mode)
589589
}
590590

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

@@ -666,7 +666,7 @@ void FT2Font::set_text(
666666
}
667667
}
668668

669-
void FT2Font::load_char(long charcode, FT_UInt32 flags)
669+
void FT2Font::load_char(long charcode, FT_Int32 flags)
670670
{
671671
int error = FT_Load_Char(face, (unsigned long)charcode, flags);
672672

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

687-
void FT2Font::load_glyph(FT_UInt glyph_index, FT_UInt32 flags)
687+
void FT2Font::load_glyph(FT_UInt glyph_index, FT_Int32 flags)
688688
{
689689
int error = FT_Load_Glyph(face, glyph_index, flags);
690690

‎src/ft2font.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ class FT2Font
7272
void set_charmap(int i);
7373
void select_charmap(unsigned long i);
7474
void set_text(
75-
size_t N, uint32_t *codepoints, double angle, FT_UInt32 flags, std::vector<double> &xys);
76-
int get_kerning(int left, int right, int mode);
77-
void load_char(long charcode, FT_UInt32 flags);
78-
void load_glyph(FT_UInt glyph_index, FT_UInt32 flags);
75+
size_t N, uint32_t *codepoints, double angle, FT_Int32 flags, std::vector<double> &xys);
76+
int get_kerning(FT_UInt left, FT_UInt right, FT_UInt mode);
77+
void load_char(long charcode, FT_Int32 flags);
78+
void load_glyph(FT_UInt glyph_index, FT_Int32 flags);
7979
void get_width_height(long *width, long *height);
8080
void get_bitmap_offset(long *x, long *y);
8181
long get_descent();

‎src/ft2font_wrapper.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -621,9 +621,10 @@ const char *PyFT2Font_get_kerning__doc__ =
621621

622622
static PyObject *PyFT2Font_get_kerning(PyFT2Font *self, PyObject *args, PyObject *kwds)
623623
{
624-
int left, right, mode, result;
624+
FT_UInt left, right, mode;
625+
int result;
625626

626-
if (!PyArg_ParseTuple(args, "iii:get_kerning", &left, &right, &mode)) {
627+
if (!PyArg_ParseTuple(args, "III:get_kerning", &left, &right, &mode)) {
627628
return NULL;
628629
}
629630

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

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

@@ -712,11 +716,14 @@ const char *PyFT2Font_load_char__doc__ =
712716
static PyObject *PyFT2Font_load_char(PyFT2Font *self, PyObject *args, PyObject *kwds)
713717
{
714718
long charcode;
715-
FT_UInt32 flags = FT_LOAD_FORCE_AUTOHINT;
719+
FT_Int32 flags = FT_LOAD_FORCE_AUTOHINT;
716720
const char *names[] = { "charcode", "flags", NULL };
717721

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

@@ -747,11 +754,14 @@ const char *PyFT2Font_load_glyph__doc__ =
747754
static PyObject *PyFT2Font_load_glyph(PyFT2Font *self, PyObject *args, PyObject *kwds)
748755
{
749756
FT_UInt glyph_index;
750-
FT_UInt32 flags = FT_LOAD_FORCE_AUTOHINT;
757+
FT_Int32 flags = FT_LOAD_FORCE_AUTOHINT;
751758
const char *names[] = { "glyph_index", "flags", NULL };
752759

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

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

904-
if (!PyArg_ParseTuple(args, "i:get_glyph_name", &glyph_number)) {
914+
if (!PyArg_ParseTuple(args, "I:get_glyph_name", &glyph_number)) {
905915
return NULL;
906916
}
907917

0 commit comments

Comments
 (0)