Re: [ft-devel] FT_Vector_NormLen: faster vector normalization

[Werner LEMBERG]

> I have just committed a fast vector normalization procedure based on
> Newton's iterations without square roots and divisions.  It speeds
> up emboldening by 40% and TrueType loading by 15% in my benchmarks
> with times and arial at 16ppem.

Great!  Thanks a lot for your efforts.

> There are a few places in the code that are implementation-defined,
> AFAICS, they rely on two's complement representation of negative
> numbers.  I do not think this is the only place in FreeType that
> makes this assumption.

Uh, oh, this code is very fragile and not easy to understand...  I
tried to remove some compiler warnings (from `clang
-Wsign-conversion', see below for the original compiler messages and
my bad attempt to fix them), and even such innocently looking changes
immediately cause bad rendering results.  For maintainance reasons I
thus ask you to improve that, both by adding casts and documentation.

I also ask you also to add some comments to (a) explain the
`0xAAAAAAAA' constant, and (b) give a numeric example for the
`difference with 2^32'.

Finally, please add more parentheses to this line:

  shift -= 15 + ( l >= 0xAAAAAAAAUL >> shift );


     Werner


======================================================================


ftcalc.c:808:14: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'FT_UInt32' (aka 'unsigned int') [-Wsign-conversion]
      return y;
      ~~~~~~ ^
ftcalc.c:814:14: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'FT_UInt32' (aka 'unsigned int') [-Wsign-conversion]
      return x;
      ~~~~~~ ^
ftcalc.c:818:36: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'unsigned int' [-Wsign-conversion]
    l = x > y ? (FT_UInt32)x + ( y >> 1 )
                             ~   ~~^~~~
ftcalc.c:819:36: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'unsigned int' [-Wsign-conversion]
              : (FT_UInt32)y + ( x >> 1 );
                             ~   ~~^~~~
ftcalc.c:830:38: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'unsigned int' [-Wsign-conversion]
      l = x > y ? (FT_UInt32)x + ( y >> 1 )
                               ~   ~~^~~~
ftcalc.c:831:38: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'unsigned int' [-Wsign-conversion]
                : (FT_UInt32)y + ( x >> 1 );
                               ~   ~~^~~~
ftcalc.c:860:17: warning: implicit conversion changes signedness:
  'int' to 'FT_UInt32' (aka 'unsigned int') [-Wsign-conversion]
    l = 0x10000 + (FT_Int32)( u * x + v * y ) / 0x10000;
      ~ ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ftcalc.c:860:35: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'unsigned int' [-Wsign-conversion]
    l = 0x10000 + (FT_Int32)( u * x + v * y ) / 0x10000;
                                ~ ^
ftcalc.c:860:43: warning: implicit conversion changes signedness:
  'FT_Int32' (aka 'int') to 'unsigned int' [-Wsign-conversion]
    l = 0x10000 + (FT_Int32)( u * x + v * y ) / 0x10000;
                                        ~ ^

======================================================================


diff --git a/src/base/ftcalc.c b/src/base/ftcalc.c
index f3595e7..1002db6 100644
--- a/src/base/ftcalc.c
+++ b/src/base/ftcalc.c
@@ -790,15 +790,18 @@
   FT_BASE_DEF( FT_UInt32 )
   FT_Vector_NormLen( FT_Vector*  vector )
   {
-    FT_Int32   x = vector->x;
-    FT_Int32   y = vector->y;
+    FT_Int32   x_ = vector->x;
+    FT_Int32   y_ = vector->y;
     FT_Int32   b, z;
-    FT_UInt32  u, v, l;
+    FT_UInt32  x, y, u, v, l;
     FT_Int     sx = 1, sy = 1, shift;
 
 
-    FT_MOVE_SIGN( x, sx );
-    FT_MOVE_SIGN( y, sy );
+    FT_MOVE_SIGN( x_, sx );
+    FT_MOVE_SIGN( y_, sy );
+
+    x = (FT_UInt32)x_;
+    y = (FT_UInt32)y_;
 
     /* trivial cases */
     if ( x == 0 )
@@ -815,8 +818,8 @@
     }
 
     /* estimate length and prenormalize */
-    l = x > y ? (FT_UInt32)x + ( y >> 1 )
-              : (FT_UInt32)y + ( x >> 1 );
+    l = x > y ? x + ( y >> 1 )
+              : y + ( x >> 1 );
 
     shift  = 31 - FT_MSB( l );
     shift -= 15 + ( l >= 0xAAAAAAAAUL >> shift );
@@ -826,9 +829,9 @@
       x <<= shift;
       y <<= shift;
 
-      /* reestimate length for tiny vectors */
-      l = x > y ? (FT_UInt32)x + ( y >> 1 )
-                : (FT_UInt32)y + ( x >> 1 );
+      /* re-estimate length for tiny vectors */
+      l = x > y ? x + ( y >> 1 )
+                : y + ( x >> 1 );
     }
     else
     {