Em Sex, 2013-09-27 às 22:23 -0400, Martin Owens escreveu:

[...] So I guess I need to make sure this isn't some C++ trick before I simplify it.

No C++ tricks here. Only IEEE floating points tricks. Maybe he wanted to avoid one of the *several* errors that can happen with floating points (where a + b + c could be different than c + b + a).

Without reasoning a lot, I can imagine the value could have a mantissa like "1.000000000x" and if he tries to divide by 10 (2 in binary), the x bit will go away thanks to the limit in the precision. Sure this problem could be solve by only manipulating the exponent, but maybe exponent is in the limit. If he subtracts -1.0 from the mantissa (but he is subtracting from the whole number, not manipulating the mantissa), it'll be possible to move the x bit to the left (changing the exponent) and a division by 10 wouldn't cause any data loss. BUT... this doesn't make sense, because his isn't subtracting -1^(some_numer_that_will_put_it_in_the_limit_of_exponent), then the operation "-1.0" won't help with this problem at all.

There are lots of possible errors with floating points and maybe I'm not seeing something here, but you tested the numbers and I wanted to give you confidence to simplify this code. If your test is correct, you are ready to go.

Maybe his intent was different and there was a special reason to do write this specific pattern, but I doubt (no comments around this code and something too specific to be probable).

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