Date: prev next · Thread: first prev next last
2018 Archives by date, by thread · List index


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:

        http://en.cppreference.com/w/cpp/numeric/math/round

        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:

[snip]
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
...
[/snip]

        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.

-- 
michael.meeks@collabora.com <><, Pseudo Engineer, itinerant idiot

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.