[truetype] Increase precision while applying VF deltas.

It turned out that we incorrectly round CVT and glyph point deltas
before accumulation, leading to severe positioning errors if there
are many delta values to sum up.

Problem reported by Akiem Helmling <akiem@underware.nl> and analyzed
by Behdad.

* src/truetype/ttgxvar.c (ft_var_readpackeddelta): Return deltas in
16.16 format.
(tt_face_var_cvt): Collect deltas in `cvt_deltas', which is a 16.16
format array, and add the accumulated values to the CVT at the end
of the function.
(TT_Vary_Apply_Glyph_Deltas): Store data in `points_org' and
`points_out' in 16.16 format.
Collect deltas in `point_deltas_x' and `point_deltas_y', which are
16.16 format arrays, and add the accumulated values to the glyph
coordinates at the end of the function.
This commit is contained in:
Werner Lemberg 2018-06-24 15:22:10 +02:00
parent cc3333902b
commit a632fb547e
2 changed files with 140 additions and 63 deletions

View File

@ -1,3 +1,25 @@
2018-06-24 Werner Lemberg <wl@gnu.org>
[truetype] Increase precision while applying VF deltas.
It turned out that we incorrectly round CVT and glyph point deltas
before accumulation, leading to severe positioning errors if there
are many delta values to sum up.
Problem reported by Akiem Helmling <akiem@underware.nl> and analyzed
by Behdad.
* src/truetype/ttgxvar.c (ft_var_readpackeddelta): Return deltas in
16.16 format.
(tt_face_var_cvt): Collect deltas in `cvt_deltas', which is a 16.16
format array, and add the accumulated values to the CVT at the end
of the function.
(TT_Vary_Apply_Glyph_Deltas): Store data in `points_org' and
`points_out' in 16.16 format.
Collect deltas in `point_deltas_x' and `point_deltas_y', which are
16.16 format arrays, and add the accumulated values to the glyph
coordinates at the end of the function.
2018-06-24 Werner Lemberg <wl@gnu.org>
New base function `FT_Matrix_Check' (#54019).

View File

@ -67,6 +67,17 @@
: (stream)->limit
/* some macros we need */
#define FT_FIXED_ONE ( (FT_Fixed)0x10000 )
#define FT_fdot14ToFixed( x ) \
( (FT_Fixed)( (FT_ULong)(x) << 2 ) )
#define FT_intToFixed( i ) \
( (FT_Fixed)( (FT_ULong)(i) << 16 ) )
#define FT_fixedToInt( x ) \
( (FT_Short)( ( (FT_UInt32)(x) + 0x8000U ) >> 16 ) )
/**************************************************************************
*
* The macro FT_COMPONENT is used in trace mode. It is an implicit
@ -234,17 +245,21 @@
* The number of deltas to be read.
*
* @Return:
* An array of FT_Short containing the deltas for the affected
* An array of FT_Fixed containing the deltas for the affected
* points. (This only gets the deltas for one dimension. It will
* generally be called twice, once for x, once for y. When used in
* cvt table, it will only be called once.)
*
* We use FT_Fixed to avoid accumulation errors while summing up all
* deltas (the rounding to integer values happens as the very last
* step).
*/
static FT_Short*
static FT_Fixed*
ft_var_readpackeddeltas( FT_Stream stream,
FT_ULong size,
FT_UInt delta_cnt )
{
FT_Short *deltas = NULL;
FT_Fixed *deltas = NULL;
FT_UInt runcnt, cnt;
FT_UInt i, j;
FT_Memory memory = stream->memory;
@ -278,13 +293,13 @@
{
/* `runcnt' shorts from the stack */
for ( j = 0; j <= cnt && i < delta_cnt; j++ )
deltas[i++] = FT_GET_SHORT();
deltas[i++] = FT_intToFixed( FT_GET_SHORT() );
}
else
{
/* `runcnt' signed bytes from the stack */
for ( j = 0; j <= cnt && i < delta_cnt; j++ )
deltas[i++] = FT_GET_CHAR();
deltas[i++] = FT_intToFixed( FT_GET_CHAR() );
}
if ( j <= cnt )
@ -401,17 +416,6 @@
}
/* some macros we need */
#define FT_FIXED_ONE ( (FT_Fixed)0x10000 )
#define FT_fdot14ToFixed( x ) \
( (FT_Fixed)( (FT_ULong)(x) << 2 ) )
#define FT_intToFixed( i ) \
( (FT_Fixed)( (FT_ULong)(i) << 16 ) )
#define FT_fixedToInt( x ) \
( (FT_Short)( ( (FT_UInt32)(x) + 0x8000U ) >> 16 ) )
static FT_Error
ft_var_load_item_variation_store( TT_Face face,
FT_ULong offset,
@ -3101,7 +3105,9 @@
FT_UShort* sharedpoints = NULL;
FT_UShort* localpoints = NULL;
FT_UShort* points;
FT_Short* deltas;
FT_Fixed* deltas;
FT_Fixed* cvt_deltas;
FT_TRACE2(( "CVAR " ));
@ -3188,6 +3194,9 @@
tupleCount & 0xFFF,
( tupleCount & 0xFFF ) == 1 ? "" : "s" ));
if ( FT_NEW_ARRAY( cvt_deltas, face->cvt_size ) )
goto FExit;
for ( i = 0; i < ( tupleCount & 0xFFF ); i++ )
{
FT_UInt tupleDataSize;
@ -3279,17 +3288,21 @@
/* this means that there are deltas for every entry in cvt */
for ( j = 0; j < face->cvt_size; j++ )
{
FT_Long orig_cvt = face->cvt[j];
FT_Fixed old_cvt_delta;
face->cvt[j] = (FT_Short)( orig_cvt +
FT_MulFix( deltas[j], apply ) );
old_cvt_delta = cvt_deltas[j];
cvt_deltas[j] = old_cvt_delta + FT_MulFix( deltas[j], apply );
#ifdef FT_DEBUG_LEVEL_TRACE
if ( orig_cvt != face->cvt[j] )
if ( old_cvt_delta != cvt_deltas[j] )
{
FT_TRACE7(( " %d: %d -> %d\n",
j, orig_cvt, face->cvt[j] ));
FT_TRACE7(( " %d: %f -> %f\n",
j,
( FT_intToFixed( face->cvt[j] ) +
old_cvt_delta ) / 65536.0,
( FT_intToFixed( face->cvt[j] ) +
cvt_deltas[j] ) / 65536.0 ));
count++;
}
#endif
@ -3312,23 +3325,26 @@
for ( j = 0; j < point_count; j++ )
{
int pindex;
FT_Long orig_cvt;
int pindex;
FT_Fixed old_cvt_delta;
pindex = points[j];
if ( (FT_ULong)pindex >= face->cvt_size )
continue;
orig_cvt = face->cvt[pindex];
face->cvt[pindex] = (FT_Short)( orig_cvt +
FT_MulFix( deltas[j], apply ) );
old_cvt_delta = cvt_deltas[pindex];
cvt_deltas[pindex] = old_cvt_delta + FT_MulFix( deltas[j], apply );
#ifdef FT_DEBUG_LEVEL_TRACE
if ( orig_cvt != face->cvt[pindex] )
if ( old_cvt_delta != cvt_deltas[pindex] )
{
FT_TRACE7(( " %d: %d -> %d\n",
pindex, orig_cvt, face->cvt[pindex] ));
FT_TRACE7(( " %d: %f -> %f\n",
pindex,
( FT_intToFixed( face->cvt[pindex] ) +
old_cvt_delta ) / 65536.0,
( FT_intToFixed( face->cvt[pindex] ) +
cvt_deltas[pindex] ) / 65536.0 ));
count++;
}
#endif
@ -3351,6 +3367,9 @@
FT_TRACE5(( "\n" ));
for ( i = 0; i < face->cvt_size; i++ )
face->cvt[i] += FT_fixedToInt( cvt_deltas[i] );
FExit:
FT_FRAME_EXIT();
@ -3360,6 +3379,7 @@
FT_FREE( tuple_coords );
FT_FREE( im_start_coords );
FT_FREE( im_end_coords );
FT_FREE( cvt_deltas );
return error;
}
@ -3602,8 +3622,8 @@
FT_Memory memory = stream->memory;
GX_Blend blend = face->blend;
FT_Vector* points_org = NULL;
FT_Vector* points_out = NULL;
FT_Vector* points_org = NULL; /* coordinates in 16.16 format */
FT_Vector* points_out = NULL; /* coordinates in 16.16 format */
FT_Bool* has_delta = NULL;
FT_Error error;
@ -3619,7 +3639,11 @@
FT_UShort* sharedpoints = NULL;
FT_UShort* localpoints = NULL;
FT_UShort* points;
FT_Short *deltas_x, *deltas_y;
FT_Fixed* deltas_x;
FT_Fixed* deltas_y;
FT_Fixed* point_deltas_x;
FT_Fixed* point_deltas_y;
if ( !face->doblend || !blend )
@ -3688,8 +3712,15 @@
tupleCount & GX_TC_TUPLE_COUNT_MASK,
( tupleCount & GX_TC_TUPLE_COUNT_MASK ) == 1 ? "" : "s" ));
if ( FT_NEW_ARRAY( point_deltas_x, n_points ) ||
FT_NEW_ARRAY( point_deltas_y, n_points ) )
goto Fail3;
for ( j = 0; j < n_points; j++ )
points_org[j] = outline->points[j];
{
points_org[j].x = FT_intToFixed( outline->points[j].x );
points_org[j].y = FT_intToFixed( outline->points[j].y );
}
for ( i = 0; i < ( tupleCount & GX_TC_TUPLE_COUNT_MASK ); i++ )
{
@ -3784,14 +3815,17 @@
/* this means that there are deltas for every point in the glyph */
for ( j = 0; j < n_points; j++ )
{
FT_Pos delta_x = FT_MulFix( deltas_x[j], apply );
FT_Pos delta_y = FT_MulFix( deltas_y[j], apply );
FT_Fixed old_point_delta_x = point_deltas_x[j];
FT_Fixed old_point_delta_y = point_deltas_y[j];
FT_Fixed point_delta_x = FT_MulFix( deltas_x[j], apply );
FT_Fixed point_delta_y = FT_MulFix( deltas_y[j], apply );
if ( j < n_points - 4 )
{
outline->points[j].x += delta_x;
outline->points[j].y += delta_y;
point_deltas_x[j] = old_point_delta_x + point_delta_x;
point_deltas_y[j] = old_point_delta_y + point_delta_y;
}
else
{
@ -3801,33 +3835,37 @@
if ( j == ( n_points - 4 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_LSB ) )
outline->points[j].x += delta_x;
point_deltas_x[j] = old_point_delta_x + point_delta_x;
else if ( j == ( n_points - 3 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_HADVANCE ) )
outline->points[j].x += delta_x;
point_deltas_x[j] = old_point_delta_x + point_delta_x;
else if ( j == ( n_points - 2 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_TSB ) )
outline->points[j].y += delta_y;
point_deltas_y[j] = old_point_delta_y + point_delta_y;
else if ( j == ( n_points - 1 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_VADVANCE ) )
outline->points[j].y += delta_y;
point_deltas_y[j] = old_point_delta_y + point_delta_y;
}
#ifdef FT_DEBUG_LEVEL_TRACE
if ( delta_x || delta_y )
if ( point_delta_x || point_delta_y )
{
FT_TRACE7(( " %d: (%d, %d) -> (%d, %d)\n",
FT_TRACE7(( " %d: (%f, %f) -> (%f, %f)\n",
j,
outline->points[j].x - delta_x,
outline->points[j].y - delta_y,
outline->points[j].x,
outline->points[j].y ));
( FT_intToFixed( outline->points[j].x ) +
old_point_delta_x ) / 65536.0,
( FT_intToFixed( outline->points[j].y ) +
old_point_delta_y ) / 65536.0,
( FT_intToFixed( outline->points[j].x ) +
point_deltas_x[j] ) / 65536.0,
( FT_intToFixed( outline->points[j].y ) +
point_deltas_y[j] ) / 65536.0 ));
count++;
}
#endif
@ -3879,14 +3917,17 @@
for ( j = 0; j < n_points; j++ )
{
FT_Pos delta_x = points_out[j].x - points_org[j].x;
FT_Pos delta_y = points_out[j].y - points_org[j].y;
FT_Fixed old_point_delta_x = point_deltas_x[j];
FT_Fixed old_point_delta_y = point_deltas_y[j];
FT_Pos point_delta_x = points_out[j].x - points_org[j].x;
FT_Pos point_delta_y = points_out[j].y - points_org[j].y;
if ( j < n_points - 4 )
{
outline->points[j].x += delta_x;
outline->points[j].y += delta_y;
point_deltas_x[j] = old_point_delta_x + point_delta_x;
point_deltas_y[j] = old_point_delta_y + point_delta_y;
}
else
{
@ -3896,33 +3937,37 @@
if ( j == ( n_points - 4 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_LSB ) )
outline->points[j].x += delta_x;
point_deltas_x[j] = old_point_delta_x + point_delta_x;
else if ( j == ( n_points - 3 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_HADVANCE ) )
outline->points[j].x += delta_x;
point_deltas_x[j] = old_point_delta_x + point_delta_x;
else if ( j == ( n_points - 2 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_TSB ) )
outline->points[j].y += delta_y;
point_deltas_y[j] = old_point_delta_y + point_delta_y;
else if ( j == ( n_points - 1 ) &&
!( face->variation_support &
TT_FACE_FLAG_VAR_VADVANCE ) )
outline->points[j].y += delta_y;
point_deltas_y[j] = old_point_delta_y + point_delta_y;
}
#ifdef FT_DEBUG_LEVEL_TRACE
if ( delta_x || delta_y )
if ( point_delta_x || point_delta_y )
{
FT_TRACE7(( " %d: (%d, %d) -> (%d, %d)\n",
FT_TRACE7(( " %d: (%f, %f) -> (%f, %f)\n",
j,
outline->points[j].x - delta_x,
outline->points[j].y - delta_y,
outline->points[j].x,
outline->points[j].y ));
( FT_intToFixed( outline->points[j].x ) +
old_point_delta_x ) / 65536.0,
( FT_intToFixed( outline->points[j].y ) +
old_point_delta_y ) / 65536.0,
( FT_intToFixed( outline->points[j].x ) +
point_deltas_x[j] ) / 65536.0,
( FT_intToFixed( outline->points[j].y ) +
point_deltas_y[j] ) / 65536.0 ));
count++;
}
#endif
@ -3946,6 +3991,16 @@
FT_TRACE5(( "\n" ));
for ( i = 0; i < n_points; i++ )
{
outline->points[i].x += FT_fixedToInt( point_deltas_x[i] );
outline->points[i].y += FT_fixedToInt( point_deltas_y[i] );
}
Fail3:
FT_FREE( point_deltas_x );
FT_FREE( point_deltas_y );
Fail2:
if ( sharedpoints != ALL_POINTS )
FT_FREE( sharedpoints );