std::lround advice ...
Michael Meeks
michael.meeks at collabora.com
Wed May 16 09:25:02 UTC 2018
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 at collabora.com <><, Pseudo Engineer, itinerant idiot
More information about the LibreOffice
mailing list