Fix rendered characters have been chipped for some TrueType fonts

[tk0miya]

ImageFont ignores descender value of TrueType fonts (uses ascender only),
then some fonts which use descender is chipped on rendering.

This fix is already reported to PIL at http://hg.effbot.org/pil-2009-raclette/issue/13/chipped-characters-have-been-rendered

[aclark4life]

Is this in any way controversial or risky? If not, I'm going to merge it. 👍

[d-schmidt]

I woudl like to test it with a font that is actually chipped. The name of one would be helpful.

[aclark4life]

@tk0miya Can you clarify "some TrueType fonts" i.e. which ones? Thanks

[cgohlke]

IIRC, I had this applied for a short time in my PIL fork but it broke other cases. I had to revert.

[tk0miya]

I met this problem with IPA font (http://ossipedia.ipa.go.jp/ipafont/index.html#en )

BTW, In the broken case, the questioner uses HelveticaLTStd-Light.otf .
I wanna try to renderer it, but it seems adobe's non-free fonts...

[aclark4life]

I'm going to close this until we can figure out what is going on

[cgohlke]

PIL clipping fonts is a common problem, e.g.:

• http://stackoverflow.com/questions/13821882/pil-cut-off-letters
• http://stackoverflow.com/questions/1933766/fonts-clipping-with-pil

There's a patch at http://pastebin.com/jP2iLkDN :

diff -ur /tmp/orig/_imagingft.c ./_imagingft.c
--- /tmp/orig/_imagingft.c  2009-10-31 22:44:12.000000000 -0200
+++ ./_imagingft.c  2012-12-12 02:30:31.000000000 -0200
@@ -35,6 +35,9 @@
 #include <freetype/freetype.h>
 #endif

+#include FT_GLYPH_H
+
+
 #if PY_VERSION_HEX < 0x01060000
 #define PyObject_New PyObject_NEW
 #define PyObject_Del PyMem_DEL
@@ -160,7 +163,7 @@

     return (PyObject*) self;
 }
-    
+
 static int
 font_getchar(PyObject* string, int index, FT_ULong* char_out)
 {
@@ -188,7 +191,7 @@
 static PyObject*
 font_getsize(FontObject* self, PyObject* args)
 {
-    int i, x;
+    int i, x, y_max, y_min;
     FT_ULong ch;
     FT_Face face;
     int xoffset;
@@ -212,6 +215,7 @@

     face = NULL;
     xoffset = 0;
+    y_max = y_min = 0;

     for (x = i = 0; font_getchar(string, i, &ch); i++) {
         int index, error;
@@ -229,6 +233,16 @@
         if (i == 0)
             xoffset = face->glyph->metrics.horiBearingX;
         x += face->glyph->metrics.horiAdvance;
+
+        FT_BBox bbox;
+        FT_Glyph glyph;
+        FT_Get_Glyph(face->glyph, &glyph);
+        FT_Glyph_Get_CBox(glyph, FT_GLYPH_BBOX_SUBPIXELS, &bbox);
+        if (bbox.yMax > y_max)
+            y_max = bbox.yMax;
+        if (bbox.yMin < y_min)
+            y_min = bbox.yMin;
+
         last_index = index;
     }

@@ -249,7 +263,7 @@

     return Py_BuildValue(
         "(ii)(ii)",
-        PIXEL(x), PIXEL(self->face->size->metrics.height),
+        PIXEL(x), PIXEL(y_max - y_min),
         PIXEL(xoffset), 0
         );
 }
@@ -330,6 +344,19 @@
     if (mask)
         load_flags |= FT_LOAD_TARGET_MONO;

+    int temp;
+    ascender = 0;
+    for (i = 0; font_getchar(string, i, &ch); i++) {
+        index = FT_Get_Char_Index(self->face, ch);
+        error = FT_Load_Glyph(self->face, index, load_flags);
+        if (error)
+            return geterror(error);
+        glyph = self->face->glyph;
+        temp = (glyph->bitmap.rows - glyph->bitmap_top);
+        if (temp > ascender)
+            ascender = temp;
+    }
+
     for (x = i = 0; font_getchar(string, i, &ch); i++) {
         if (i == 0 && self->face->glyph->metrics.horiBearingX < 0)
             x = -PIXEL(self->face->glyph->metrics.horiBearingX);
@@ -348,7 +375,6 @@
             /* use monochrome mask (on palette images, etc) */
             int xx, x0, x1;
             source = (unsigned char*) glyph->bitmap.buffer;
-            ascender = PIXEL(self->face->size->metrics.ascender);
             xx = x + glyph->bitmap_left;
             x0 = 0;
             x1 = glyph->bitmap.width;
@@ -357,7 +383,7 @@
             if (xx + x1 > im->xsize)
                 x1 = im->xsize - xx;
             for (y = 0; y < glyph->bitmap.rows; y++) {
-                int yy = y + ascender - glyph->bitmap_top;
+                int yy = y + im->ysize - (PIXEL(glyph->metrics.horiBearingY) + ascender);
                 if (yy >= 0 && yy < im->ysize) {
                     /* blend this glyph into the buffer */
                     unsigned char *target = im->image8[yy] + xx;
@@ -377,7 +403,6 @@
             /* use antialiased rendering */
             int xx, x0, x1;
             source = (unsigned char*) glyph->bitmap.buffer;
-            ascender = PIXEL(self->face->size->metrics.ascender);
             xx = x + glyph->bitmap_left;
             x0 = 0;
             x1 = glyph->bitmap.width;
@@ -386,7 +411,7 @@
             if (xx + x1 > im->xsize)
                 x1 = im->xsize - xx;
             for (y = 0; y < glyph->bitmap.rows; y++) {
-                int yy = y + ascender - glyph->bitmap_top;
+                int yy = y + im->ysize - (PIXEL(glyph->metrics.horiBearingY) + ascender);
                 if (yy >= 0 && yy < im->ysize) {
                     /* blend this glyph into the buffer */
                     int i;

[aclark4life]

Is this the patch you had to revert? Or another one

[cgohlke]

This patch is another one. I have not tried it. I just pasted it here to keep track of things and so it doesn't get lost.

[aclark4life]

Got it, thanks

[tk0miya]

With HelveticaLTStd-Light.otf and my changes, I got clipped characters:

I'll try cgohike's fix later (maybe next weekend).

[tk0miya]

With a patch from http://pastebin.com/jP2iLkDN and HelveticaLTStd-Light.otf, I got correct image!

I think this patch works fine.

[aclark4life]

That's great! Thanks

[tk0miya]

@aclark4life sorry for confuse you. My pull request is imcomplete.
It generates clipped image with HelveticaLTStd-Light.otf.

Please use patch from http://pastebin.com/jP2iLkDN that is pasted by cgohike.

[aclark4life]

Can you send the patch as a pull request? Do you want me to revert this last merge?

[fabiomcosta]

This fixes the problem in linux, but introduces the bug in OSX.

This is before the fix in OSX:

After:

[wiredfool]

We definitely need test coverage on this one.

[aclark4life]

Sooo am I supposed to just patch on top with: http://pastebin.com/jP2iLkDN ? Or revert, then patch…

[tk0miya]

Hi @aclark4life
I send new pull request #185. this includes reverting merge and patch http://pastebin.com/jP2iLkDN .
but it does not include any testcases... (i don't know how to test)

[aclark4life]

Thanks!