[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [ft-devel] FT_Vector_NormLen: faster vector normalization
From: |
Werner LEMBERG |
Subject: |
Re: [ft-devel] FT_Vector_NormLen: faster vector normalization |
Date: |
Tue, 30 Jun 2015 09:47:00 +0200 (CEST) |
> 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
{