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