Skip to content

Commit

Permalink
[BUGFIX] fix qgsRound for negative numbers. Fixes #20861
Browse files Browse the repository at this point in the history
  • Loading branch information
lbartoletti authored and nyalldawson committed Jan 29, 2019
1 parent 774f025 commit cfdc8c2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/core/qgis.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,9 @@ inline bool qgsDoubleNearSig( double a, double b, int significantDigits = 10 )
*/
inline double qgsRound( double number, double places )
{
double m = ( number < 0.0 ) ? -1.0 : 1.0;
double scaleFactor = std::pow( 10.0, places );
return std::trunc( number * scaleFactor + 0.5 ) / scaleFactor;

This comment has been minimized.

Copy link
@elpaso

elpaso Apr 9, 2019

Contributor

@lbartoletti the test passes on my machine even without the m factor.
I dind't really measure how much overhead this two lines are adding, but if we can avoid them let's do it. (use case: massive json generation where each and every coordinate goes through this function)

This comment has been minimized.

Copy link
@lbartoletti

lbartoletti Apr 12, 2019

Author Member

@elpaso It was a fix for an incorrect rouding. Example on QGIS 3.2:

>>> qgsRound(-1423.567, 2) # pyqgis round
-1423.56
>>> qgsRound(-1423.561, 2)
-1423.55
>>> round(-1423.567, 2) # python round
-1423.57
>>> round(-1423.561, 2)
-1423.56

C++ tests are OK (except a floating precision error on -9.876532198765) but PyQGIS QgsRound returns a false result.
For this case. I just tested a std::round without m and it's works. I'll add some pytest next week.

return ( std::round( number * m * scaleFactor ) / scaleFactor ) * m;
}


Expand Down
6 changes: 4 additions & 2 deletions tests/src/core/testqgis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ void TestQgis::testQgsAsConst()

void TestQgis::testQgsRound()
{
QGSCOMPARENEAR( qgsRound( 1234.567, 2 ), 1234.57, 0.01 );
QGSCOMPARENEAR( qgsRound( -1234.567, 2 ), -1234.57, 0.01 );
QGSCOMPARENEAR( qgsRound( 98765432198, 8 ), 98765432198, 1.0 );
QGSCOMPARENEAR( qgsRound( 98765432198, 9 ), 98765432198, 1.0 );
QGSCOMPARENEAR( qgsRound( 98765432198, 10 ), 98765432198, 1.0 );
Expand All @@ -357,9 +359,9 @@ void TestQgis::testQgsRound()
QGSCOMPARENEAR( qgsRound( 9.8765432198765, 5 ), 9.87654, 0.000001 );
QGSCOMPARENEAR( qgsRound( 9.8765432198765, 6 ), 9.876543, 0.0000001 );
QGSCOMPARENEAR( qgsRound( 9.8765432198765, 7 ), 9.8765432, 0.00000001 );
QGSCOMPARENEAR( qgsRound( -9.8765432198765, 7 ), -9.876543, 0.000001 );
QGSCOMPARENEAR( qgsRound( -9.8765432198765, 7 ), -9.8765432, 0.0000001 );
QGSCOMPARENEAR( qgsRound( 9876543.2198765, 5 ), 9876543.219880, 0.000001 );
QGSCOMPARENEAR( qgsRound( -9876543.2198765, 5 ), -9876543.219870, 0.000001 );
QGSCOMPARENEAR( qgsRound( -9876543.2198765, 5 ), -9876543.219880, 0.000001 );
QGSCOMPARENEAR( qgsRound( 9.87654321987654321, 13 ), 9.87654321987654, 0.0000000000001 );
QGSCOMPARENEAR( qgsRound( 9.87654321987654321, 14 ), 9.876543219876543, 0.00000000000001 );
QGSCOMPARENEAR( qgsRound( 9998.87654321987654321, 14 ), 9998.876543219876543, 0.00000000000001 );
Expand Down

0 comments on commit cfdc8c2

Please sign in to comment.