rougier / freetype-gl

OpenGL text using one vertex buffer, one texture and FreeType
Other
1.63k stars 262 forks source link

Incorrect FreeType error messages, FT_Errors #226

Closed AlbertoGP closed 3 years ago

AlbertoGP commented 3 years ago

Hi, I’m debugging #210 and found that, at least in my system, the FT_Errors[] table (in texture-font.c) does not work as intended.

The error code returned by FT should work as an index in that table, but the error codes do not match their indexes. That causes incorrect error reports, for instance in #210 mentioned above, even if building such a table is one of the two methods recommended in the documentation for FreeType: https://www.freetype.org/freetype2/docs/reference/ft2-error_enumerations.html

In that case, FT_Set_Char_Size() (in texture_font_load_face()) returns the error code 0x97 (151 decimal) which corresponds to “invalid ppem value” [1] as you can see in https://www.freetype.org/freetype2/docs/reference/ft2-error_code_values.html

[1] Those codes can change between releases, but this one matches as I show below. The error message “invalid ppem value” makes sense because the missing text appears if I round down the size parameter passed to FT_Set_Char_Size().

However, the code that handles the error code from FT_Set_Char_Size() at https://github.com/rougier/freetype-gl/blob/1559dcb8480bedad106fa4113a3102b83140b9f2/texture-font.c#L88 prints the wrong error message: FT_Error (line 89, code 0xf23045c8) : GL_ARB_draw_instanced

That is the result of a buffer overread because the error table has only 92 entries, so position 151 is outside of it. As a consequence, the error code changes each time I run the markup demo. The curious error string does stay constant, though.

To verify the issue, I wrote a function that dumps the FT_Errors[] table:

static void
debug_list_all_ft_errors()
{
    int i = 0;
    while (1) {
        const struct { int code; const char* message; } *err = &FT_Errors[i];
        if (!err->message) break;
        fprintf(stderr, "%d(0x%x) 0x%02x %s\n",
                i, i, err->code, err->message);
        ++i;
    }
}

The result is the following:

0(0x0) 0x00 no error
1(0x1) 0x01 cannot open resource
2(0x2) 0x02 unknown file format
3(0x3) 0x03 broken file
4(0x4) 0x04 invalid FreeType version
5(0x5) 0x05 module version is too low
6(0x6) 0x06 invalid argument
7(0x7) 0x07 unimplemented feature
8(0x8) 0x08 broken table
9(0x9) 0x09 broken offset within table
10(0xa) 0x0a array allocation size too large
11(0xb) 0x0b missing module
12(0xc) 0x0c missing property
13(0xd) 0x10 invalid glyph index
14(0xe) 0x11 invalid character code
15(0xf) 0x12 unsupported glyph image format
16(0x10) 0x13 cannot render this glyph format
17(0x11) 0x14 invalid outline
18(0x12) 0x15 invalid composite glyph
19(0x13) 0x16 too many hints
20(0x14) 0x17 invalid pixel size
21(0x15) 0x20 invalid object handle
22(0x16) 0x21 invalid library handle
23(0x17) 0x22 invalid module handle
24(0x18) 0x23 invalid face handle
25(0x19) 0x24 invalid size handle
26(0x1a) 0x25 invalid glyph slot handle
27(0x1b) 0x26 invalid charmap handle
28(0x1c) 0x27 invalid cache manager handle
29(0x1d) 0x28 invalid stream handle
30(0x1e) 0x30 too many modules
31(0x1f) 0x31 too many extensions
32(0x20) 0x40 out of memory
33(0x21) 0x41 unlisted object
34(0x22) 0x51 cannot open stream
35(0x23) 0x52 invalid stream seek
36(0x24) 0x53 invalid stream skip
37(0x25) 0x54 invalid stream read
38(0x26) 0x55 invalid stream operation
39(0x27) 0x56 invalid frame operation
40(0x28) 0x57 nested frame access
41(0x29) 0x58 invalid frame read
42(0x2a) 0x60 raster uninitialized
43(0x2b) 0x61 raster corrupted
44(0x2c) 0x62 raster overflow
45(0x2d) 0x63 negative height while rastering
46(0x2e) 0x70 too many registered caches
47(0x2f) 0x80 invalid opcode
48(0x30) 0x81 too few arguments
49(0x31) 0x82 stack overflow
50(0x32) 0x83 code overflow
51(0x33) 0x84 bad argument
52(0x34) 0x85 division by zero
53(0x35) 0x86 invalid reference
54(0x36) 0x87 found debug opcode
55(0x37) 0x88 found ENDF opcode in execution stream
56(0x38) 0x89 nested DEFS
57(0x39) 0x8a invalid code range
58(0x3a) 0x8b execution context too long
59(0x3b) 0x8c too many function definitions
60(0x3c) 0x8d too many instruction definitions
61(0x3d) 0x8e SFNT font table missing
62(0x3e) 0x8f horizontal header (hhea) table missing
63(0x3f) 0x90 locations (loca) table missing
64(0x40) 0x91 name table missing
65(0x41) 0x92 character map (cmap) table missing
66(0x42) 0x93 horizontal metrics (hmtx) table missing
67(0x43) 0x94 PostScript (post) table missing
68(0x44) 0x95 invalid horizontal metrics
69(0x45) 0x96 invalid character map (cmap) format
70(0x46) 0x97 invalid ppem value
71(0x47) 0x98 invalid vertical metrics
72(0x48) 0x99 could not find context
73(0x49) 0x9a invalid PostScript (post) table format
74(0x4a) 0x9b invalid PostScript (post) table
75(0x4b) 0x9c found FDEF or IDEF opcode in glyf bytecode
76(0x4c) 0x9d missing bitmap in strike
77(0x4d) 0xa0 opcode syntax error
78(0x4e) 0xa1 argument stack underflow
79(0x4f) 0xa2 ignore
80(0x50) 0xa3 no Unicode glyph name found
81(0x51) 0xa4 glyph too big for hinting
82(0x52) 0xb0 `STARTFONT' field missing
83(0x53) 0xb1 `FONT' field missing
84(0x54) 0xb2 `SIZE' field missing
85(0x55) 0xb3 `FONTBOUNDINGBOX' field missing
86(0x56) 0xb4 `CHARS' field missing
87(0x57) 0xb5 `STARTCHAR' field missing
88(0x58) 0xb6 `ENCODING' field missing
89(0x59) 0xb7 `BBX' field missing
90(0x5a) 0xb8 `BBX' too big
91(0x5b) 0xb9 Font header corrupted or missing fields
92(0x5c) 0xba Font glyphs corrupted or missing fields

I think the solution would be to use the other method, the switch() block, inside a function that returns the error string: https://www.freetype.org/freetype2/docs/reference/ft2-error_enumerations.html

rougier commented 3 years ago

I understand the problem but I can't understand why the FT_errors table is wrong. Do you know what is the reason ? We can of course use the second method if it fixes the problem. Can you make a PR?

AlbertoGP commented 3 years ago

Yes, I’ll make a PR soon. I also don’t know why the table is wrong if the implementation is exactly the one proposed by FreeType. I can only guess that it did work fine at the time when they wrote the documentation, maybe the error codes were all consecutive and later they removed some of them, which made the index no longer match the error code.

AlbertoGP commented 3 years ago

This problem seems to be a misunderstanding: FreeType’s documentation proposes this error array as one option but it does not say anywhere that the array should be indexed by error code. I guess they expected it to be accessed from a function that would scan it until finding the matching error code: otherwise there is no reason to have the error code in each entry.

I’m preparing the correction.

AlbertoGP commented 3 years ago

With the changes in this pull request, running markup under the conditions for #210 shows me the correct error message:

Using GLEW 2.2.0
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans-Bold.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans-Oblique.ttf" (size=20.5)
FT_Error (line 95, code 0x97) : invalid ppem value
Unable to load "/usr/share/fonts/TTF/DejaVuSans.ttf" (size=20.5)
Houston, we've got a problem !
Houston, we've got a problem !
Houston, we've got a problem !
Houston, we've got a problem !
Houston, we've got a problem !
Houston, we've got a problem !
Houston, we've got a problem !
Houston, we've got a problem !
rougier commented 3 years ago

Thanks, PR has been merged.