freetype-commit
[Top][All Lists]
Advanced

[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 );



reply via email to

[Prev in Thread] Current Thread [Next in Thread]