[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[freetype2] master a632fb5: [truetype] Increase precision while applying
From: |
Werner LEMBERG |
Subject: |
[freetype2] master a632fb5: [truetype] Increase precision while applying VF deltas. |
Date: |
Sun, 24 Jun 2018 09:24:38 -0400 (EDT) |
branch: master
commit a632fb547e23fe129a579fabf60a992fd7d98d02
Author: Werner Lemberg <address@hidden>
Commit: Werner Lemberg <address@hidden>
[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 <address@hidden> 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.
---
ChangeLog | 22 ++++++
src/truetype/ttgxvar.c | 181 ++++++++++++++++++++++++++++++++-----------------
2 files changed, 140 insertions(+), 63 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 0234e60..f1d7eab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,27 @@
2018-06-24 Werner Lemberg <address@hidden>
+ [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 <address@hidden> 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 <address@hidden>
+
New base function `FT_Matrix_Check' (#54019).
* src/base/ftcalc.c (FT_Matrix_Check): New base function to properly
diff --git a/src/truetype/ttgxvar.c b/src/truetype/ttgxvar.c
index f3c39fc..c9f0ba4 100644
--- a/src/truetype/ttgxvar.c
+++ b/src/truetype/ttgxvar.c
@@ -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 );
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [freetype2] master a632fb5: [truetype] Increase precision while applying VF deltas.,
Werner LEMBERG <=