Hi guys,

        I noticed Chris has a number of commits killing the:

        tools/helpers.hxx FRound function which does:

inline long FRound( double fVal )
    return fVal > 0.0 ? static_cast<long>( fVal + 0.5 )
                : -static_cast<long>( -fVal + 0.5 );

        To use std::lround instead:


        Which seems like a worthy albeit risky goal to me: used in all the
Rectangle logic everywhere.

        Any thoughts on that from other people ?

        I appreciate rounding errors are -unbelievably- horrible to find, debug
and get rid of - taking sometimes man weeks of work to make things
consistent and align again - with subtle display-dirt from non-joined
tiles and so on that we have practically no tests for; nevertheless it
would be interesting to see if this is indeed controversial.

        Chris - I'm intrigued by the above cppreference link on the
corner-cases here:

If the implementation supports IEEE floating-point arithmetic (IEC 60559),

For the std::round function:
The current rounding mode has no effect.
If arg is ±∞, it is returned, unmodified
If arg is ±0, it is returned, unmodified
If arg is NaN, NaN is returned

        And I guess what I'd want to know is - can we create a unit test for
std::lround that copy/pastes the old FRound code into it (which of
course will be dead in your world) and allows us to verify that all of
the corner-cases work in the same way, and that the FPU state / error
conditions are working nicely.

        so eg. (pseudo-code)

        CPPUNIT_ASSERT(FRound(NaN) == std::lround(NaN))
        CPPUNIT_ASSERT(FRound(1.0+epsilon) == std::lround(1.0+epsilon))
        ... etc. ...

        and so on for both basic and complex cases =)

        Failing that poking at the implementation of std:lround to see it is
identical might be quicker & easier ;-) or looking at the generated
assembler to compare it or the implementation of the __builtin_lround
family that this compiles to ... =)

        Would be good to get this decided & then included - and (ideally) move
to an identical - but properly documented, standardized and easy-to-read
std:: function =)

        I'd love to get consensus & merge get the patches out of
gerrit =)

        Thoughts ?


michael.me...@collabora.com <><, Pseudo Engineer, itinerant idiot
LibreOffice mailing list

Reply via email to