Werner LEMBERG wrote:
> While looking at the whole issue more closely, I've identified at
> least four bugs :-( One is extremely serious, making
> `FT_Get_Advance' return wrongly scaled values if fast advance
> loading isn't available (now fixed in git).
Yesterday I was away from my mail, but I also found this bug, and used
about the same fix as you committed. The only difference is a minor nit,
in the case FT_LOAD_NO_SCALE is used and the quick way failed with
ErrNotImplem, <<10 scaling might not be done.
Also, see below at the end for another point.
> The next two are more subtle and not directly related to
> `FT_Get_Advance' (I get wrong metrics from some fonts), and I need
> more investigation to fix them.
Details please. I did found something myself, most likely a double
rounding issue (more below), but I cannot figure where is the problem
exactly because I do not know the internals enough :-(.
>> Another thing we need to document is that the ->glyph instance
>> inside the face object might be erased after a call through
>> FT_Get_Advance().
>
> Yes. Can you provide a patch?
Will do. I noticed there are some places (about GlyphSlotRec) where it
is mentioned the API functions which modify them, I should update that.
>> Well, there is also the "problem" that they used the FT_Get_Advance
>> API specifying light rendering, just to found out later that the
>> lightly hinted advance was actually different...
>
> One important issue has been missed completely by me: Advance widths
> don't change during the light hinting process, however, they get
> rounded to integer values, similar to TrueType.
Yeah, and I understand this was the problem Chromium guys did encounter
in the first place.
> After some thinking I now believe that it is a bug that
> `FT_Get_Advance' returns unrounded advance widths for the light
> hinting mode.
Okay, but then we have to mimic exactly the rounding process used by the
Load_Glyph logic*s*... More below.
>>>> [...] in TrueType there is a way to get the hinted advances
>>>> quicker than executing the full bytecode program, even if it is
>>>> not alluded above: when the requested ppem has an associated
>>>> hdmx/VDMX table within the font.
>>>
>>> Do you volunteer to implement support for those two tables?
>>
>> Yes, I will do that (but not today.)
I looked at it, but it does not appear to be very easy to do,
particularly because the current (*...clazz->get_advances)() functions
returns _unscaled_ values (which are to be scaled within FT_Get_Advance
while "hdmx" values are pixels, hence are already scaled.
A possible way to deal with that could be for (*clazz->get_advances)()
to return FT_PseudoErr_InternalAlreadyScaled (it's internal after all),
and dealing with that case at FT_Get_Advance.
How do you feel it?
>> I wonder also about a new demo program to check whether
>> FT_Get_Advance() gives, or not, the same results as FT_Load_Glyph().
>
> This might be useful, yes.
Attached. -f and -i options behaves the same as with ftbench: so -f
10000 forces the use of the light hinting mode; -i is useful when you
got many failures, to skip up to "interesting" glyphs. The -a option
checks the value of glyph->advance.x; without -a, it checks the value of
glyph->linearHoriAdvance (which are supposedly the base; more below.)
I added the -r option after discovering as you did the issue with
grid-fitting: so -r rounds the 16.16 values before doing the comparison.
The problem I detected then was the behaviour of invocations like
C>ftchkadv -f 10000 -a -r 147 \Windows\Fonts\times.ttf
\Windows\Fonts\times.ttf: glyph 376: Load_Glyph->->advance: 92.0000 !=
Get_Advance: 91.7e20
glyph 377: Load_Glyph->->advance: 92.0000 != Get_Advance: 91.7e20
glyph 378: Load_Glyph->->advance: 92.0000 != Get_Advance: 91.7e20
glyph 379: Load_Glyph->->advance: 92.0000 != Get_Advance: 91.7e20
glyph 1620: Load_Glyph->->advance: 1d.0000 != Get_Advance: 1c.7ee0
glyph 1962: Load_Glyph->->advance: 2b.0000 != Get_Advance: 2a.7e00
glyph 2974: Load_Glyph->->advance: 2c.0000 != Get_Advance: 2b.7f40
glyph 3065: Load_Glyph->->advance: 2b.0000 != Get_Advance: 2a.7e00
8 fails.
Changing ppem only changes the problematic glyphs :-/.
Notice that every problem are always with Get_Advance being slightly
below the xx.8000 mark. I know the path for Get_Advance: grabs the raw
font units in 26.6 then MulDiv(,x_scale,64), and the result is in 16.16;
my current guess is that within the Load_Glyph codepath for ->advance,
there is two rounding steps... but I do not where/how; also the fact I
was not able to spot a similar problem with Type1 fonts suggests there
is something specific to TrueType happening.
But these experimentations with the two values (with -a or without)
reminds me with the first point, about the computation of FT_Get_Advance
in the fall-back case, and the use of the computed advance.x<<10 values
as references: would it make more sense to use linearHoriAdvance<<0 there?
I tried my test program with that change to ftadvanc.c, and detected
another variation, this time with FT_LOAD_NO_HINTING
C>ftchkadv -f 2 -a 147 \Windows\Fonts\times.ttf
\Windows\Fonts\times.ttf: glyph 0: Load_Glyph->->advance: 72.5800 !=
Get_Advance: 72.5760
glyph 4: Load_Glyph->->advance: 30.f400 != Get_Advance: 30.f3c0
glyph 5: Load_Glyph->->advance: 3c.0000 != Get_Advance: 3c.0180
glyph 8: Load_Glyph->->advance: 7a.7400 != Get_Advance: 7a.73c0
glyph 9: Load_Glyph->->advance: 72.5800 != Get_Advance: 72.5760
glyph 10: Load_Glyph->->advance: 1a.7c00 != Get_Advance: 1a.7c60
glyph 11: Load_Glyph->->advance: 30.f400 != Get_Advance: 30.f3c0
glyph 12: Load_Glyph->->advance: 30.f400 != Get_Advance: 30.f3c0
glyph 14: Load_Glyph->->advance: 52.e800 != Get_Advance: 52.e720
glyph 16: Load_Glyph->->advance: 30.f400 != Get_Advance: 30.f3c0
2549 fails.
Clearly, the problem now is with the precision: FT_Get_Advance gives
16.16 values, same as the precision of linearHoriAdvance; but advance.x
is a 26.6 value, so there is a loss of precision which the test program
detects here. I believe these results are to be expected, and they all
concur to the point that FT_Get_Advance is a quick way to find out the
"linearHoriAdvance" value, _not_ advance.x.
Antoine
/****************************************************************************/
/* */
/* The FreeType project -- a free and portable quality font engine */
/* */
/* Copyright 1996-2011 by A. Leca, */
/* D. Turner, R. Wilhelm, and W. Lemberg */
/* */
/* ftchkadv: test FT_Get_Advance. This program compares the advances */
/* computed in various ways for all the glyphs of a given font. */
/* */
/* NOTE: This is just a test program that is used to debug the */
/* current engine. */
/* */
/****************************************************************************/
#include <ft2build.h>
#include FT_FREETYPE_H
#include FT_ADVANCES_H
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "common.h"
#define gettext( x ) ( x )
FT_Error error;
FT_Library library;
FT_Face face;
FT_Size size;
FT_GlyphSlot slot;
int check_advance_x = 0;
unsigned int first_index = 0;
FT_Int32 load_flags = FT_LOAD_DEFAULT;
FT_Fixed* advances;
unsigned int num_glyphs;
int ptsize;
int Fail;
int Num;
FT_Fixed NoRoundFix( FT_Fixed a )
{
return a;
}
/* By default, do not round. */
/* FT_RoundFix, FT_CeilFix, or FT_FloorFix are valid alternates. */
FT_Fixed (*roundFix_func)( FT_Fixed ) = NoRoundFix;
static void Usage( char* name )
{
printf( "ftchkadv: FT_Get_Advance tester -- part of the FreeType project\n"
);
printf( "---------------------------------------------------------------\n"
);
printf( "\n" );
printf( "Usage: %s [options] ppem fontname[.ttf|.ttc] [fontname2..]\n",
name );
fprintf( stderr, "options:\n");
fprintf( stderr, " -a : check advance.x instead of linearHoriAdvance\n"
" -f HHHH : load flags (hexadecimal)\n"
" -i NNN : first index to start with (default is 0)\n"
" -r : rounds advances before comparison\n");
printf( "\n" );
exit( 1 );
}
static void Panic( const char* message )
{
fprintf( stderr, "%s\n error code = 0x%04x\n", message, error );
exit(1);
}
int main( int argc, char** argv )
{
int i, file_index;
unsigned int id;
char filename[128 + 4];
char alt_filename[128 + 4];
char* execname;
char* fname;
FT_Fixed advance;
execname = argv[0];
while ( 1 )
{
int opt;
opt = getopt( argc, argv, "af:i:r" );
if ( opt == -1 )
break;
switch ( opt )
{
case 'a':
check_advance_x = 1;
break;
case 'f':
load_flags = strtol( optarg, NULL, 16 );
break;
case 'i':
first_index = atoi( optarg );
break;
case 'r':
roundFix_func = FT_RoundFix;
break;
default:
Usage( execname );
break;
}
}
argc -= optind;
argv += optind;
if ( argc < 2 )
Usage( execname );
if ( sscanf( argv[0], "%d", &ptsize ) != 1 )
Usage( execname );
error = FT_Init_FreeType( &library );
if (error) Panic( "Could not create library object" );
/* Now check all files */
for ( file_index = 1; file_index < argc; file_index++ )
{
fname = argv[file_index];
/* try to open the file with no extra extension first */
error = FT_New_Face( library, fname, 0, &face );
if (!error)
{
printf( "%s: ", fname );
goto Success;
}
if ( error == FT_Err_Unknown_File_Format )
{
printf( "unknown format\n" );
continue;
}
/* ok, we could not load the file, try to add an extension to */
/* its name if possible.. */
i = strlen( fname );
while ( i > 0 && fname[i] != '\\' && fname[i] != '/' )
{
if ( fname[i] == '.' )
i = 0;
i--;
}
filename[128] = '\0';
alt_filename[128] = '\0';
strncpy( filename, fname, 128 );
strncpy( alt_filename, fname, 128 );
#ifndef macintosh
if ( i >= 0 )
{
strncpy( filename + strlen( filename ), ".ttf", 4 );
strncpy( alt_filename + strlen( alt_filename ), ".ttc", 4 );
}
#endif
i = strlen( filename );
fname = filename;
while ( i >= 0 )
#ifndef macintosh
if ( filename[i] == '/' || filename[i] == '\\' )
#else
if ( filename[i] == ':' )
#endif
{
fname = filename + i + 1;
i = -1;
}
else
i--;
printf( "%s: ", fname );
/* Load face */
error = FT_New_Face( library, filename, 0, &face );
if (error)
{
if (error == FT_Err_Unknown_File_Format)
printf( "unknown format\n" );
else
printf( "could not find/open file (error: %d)\n", error );
continue;
}
if (error) Panic( "Could not open file" );
Success:
num_glyphs = face->num_glyphs;
if ( !check_advance_x &&
load_flags & FT_LOAD_NO_SCALE &&
FT_IS_SCALABLE(face) &&
face->units_per_EM != ptsize )
{
printf( "unscaled values, design EM=%d != %d!!! ",
face->units_per_EM, ptsize );
}
error = FT_Set_Char_Size( face, ptsize << 6, ptsize << 6, 72, 72 );
if (error) Panic( "Could not set character size" );
advances = (FT_Fixed *)calloc( sizeof ( FT_Fixed ), num_glyphs );
if (!advances) Panic( "Out of memory" );
error = FT_Get_Advances( face, first_index, num_glyphs - first_index,
FT_ADVANCE_FLAG_FAST_ONLY|load_flags, advances );
if (error)
{
if ( error == FT_Err_Unimplemented_Feature )
{
printf( "(no fast-computed advances) " );
error = FT_Get_Advances( face, first_index, num_glyphs - first_index,
load_flags, advances );
}
if ( error )
{
printf( "FT_Get_Advances failed 0x%04x\n" , error );
goto Done;
}
}
Fail = 0;
for ( id = 0; id < num_glyphs - first_index; id++ )
{
error = FT_Load_Glyph( face, id + first_index, load_flags );
if (error)
{
if ( Fail < 10 )
printf( "glyph %4u: fail to load 0x%04x\n" , id, error );
Fail++;
}
else
{
if ( check_advance_x )
{
advance = ( load_flags & FT_LOAD_VERTICAL_LAYOUT )
? (FT_Fixed)face->glyph->advance.y
: (FT_Fixed)face->glyph->advance.x;
if ( load_flags & FT_LOAD_NO_SCALE )
{
/* Glyph advance is in font units, advances also :-) */
}
else
{
/* Glyph advance is in FUnit (26.6), advances are in 16.16 */
advance <<= (16 - 6);
}
}
else
{
advance = ( load_flags & FT_LOAD_VERTICAL_LAYOUT )
? (FT_Fixed)face->glyph->linearVertAdvance
: (FT_Fixed)face->glyph->linearHoriAdvance;
if ( ( load_flags & FT_LOAD_NO_SCALE ) == 0 )
{
/* Glyph linear advance is in 16.16, advances also :-) */
}
else
{
/* advances are unscaled in font units, while glyph linear */
/* advance is scaled in 16.16... The requested size better */
/* have to be the same as the design EM size! */
/* Should we round before loosing precision? */
advance = advance >> 16;
}
}
/* Note that if hinted, glyph->advance is grid-fitted, so will */
/* not match the value returned by FT_Get_Advance unless rounded. */
if ( roundFix_func(advance) != roundFix_func(advances[id]) )
{
if ( Fail < 10 )
printf( "glyph %4u: Load_Glyph->->%s: %x.%04x != Get_Advance:
%x.%04x\n" ,
id + first_index, check_advance_x ? "advance" :
"linearAdv",
advance >> 16, (advance & 0xFFFF),
advances[id] >> 16, (advances[id] & 0xFFFF) );
Fail++;
}
}
}
if ( Fail == 0 )
printf( "OK.\n" );
else
if ( Fail == 1 )
printf( "1 fail.\n" );
else
printf( "%d fails.\n", Fail );
Done:
free( advances );
FT_Done_Face( face );
}
FT_Done_FreeType(library);
exit( 0 ); /* for safety reasons */
return 0; /* never reached */
}
/* End */
_______________________________________________
Freetype-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/freetype-devel
