On Fri, Sep 27, 2013 at 10:23:12PM -0400, Martin Owens wrote:
Hail Devs,
In display/sodipodi-ctrl.cpp:133 has some interesting maths. It does int((float(value) - 1.0) / 2.0 + 0.5) ... which is a bit redundant. My tests in python show that this produces the same output as int(float(value) / 2.0)) for all ints and floats.
But this code has been there since MentalGuy committed rev 1. and It's passed through 2 developers since who thought it was sane. So I guess I need to make sure this isn't some C++ trick before I simplify it.
I asked MentalGuy and he says: MenTaLguY: that predates me MenTaLguY: but probably has to do with fp rounding behavior MenTaLguY: it's certainly not always true that the two expressions are equal with fp
he is probably right if you disallow any optimisations or promotion (because there is probably a number such that the -1 actually subtracts 2 due to underflow), but let's see if the compiler does the right thing. (feel free to check my test):
#include <assert.h> #include <stdio.h>
int main(int argc, char** argv) { union { unsigned int j; float f; };
assert(sizeof(int) == 4); for(long i = 0; i < (1L<<32); i+=1) { j = i; if(int((float(f) - 1.0) / 2.0 + 0.5) != int(float(f) / 2.0)) { printf("%d %g\n", j, f); return 0; } } }
and I can confirm that the two are identical. So just replace it with the simple code (I would avoid such weird code in any case because when you get close to numbers where this sort of stuff might work, you're still only addressing a few edge cases, and you are probably introducing other edge cases - and the compiler and float point are probably smarter too).
njh