Re: [Wesnoth-dev] patch for fixed-point calculations

[Jon Daniel]

Yann Dirson <address@hidden> wrote:
> 
> I've also tried to write compatibility macros to have the previous
> behaviour, although I've not tested them yet (see util.hpp).
> 

Nice work converting all the performance critical functions to use fixed
point math.
One problem is that you cluttered the normal floating point code with
the fixed point macros. There should be an #ifdef
USE_FIXED_POINT or something like that for every modified function
header and content. So instead of just replacing all the floating point
code with the macros one should write a separate fixed point version in
the #ifdef USE_FIXED_POINT of each code snippet affected. I understand
that the current implementations was intended to allow easy profiling
without changing to much of the code.
And one thing I noticed is:
+// IN: fixed_t - OUT: int
+# define fxptoi(x) ( ((x)>0) ? ((x) >> fxp_shift) : (-((-(x)) >>
fxp_shift)) )
I thought C converts >> to an arithmetic shift if the operators are
signed int.
But I maybe wrong and you've got a reason for that if statement.

Jon Daniel