Bug report #19844

Flaw in qgsround() in some circumstances

Added by Andrea Giudiceandrea over 6 years ago. Updated over 6 years ago.

Status:Closed
Priority:High
Assignee:Nyall Dawson
Category:Map Tools
Affected QGIS version:3.3(master) Regression?:Yes
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:27668

Description

qgsround() is defined in qgis.h (since 3.0) as

inline double qgsRound( double number, double places ) 
  { 
int scaleFactor = std::pow( 10, places );
return static_cast<double>( static_cast<qlonglong>( number * scaleFactor
+ 0.5 ) ) / scaleFactor;
}

and it seems only used by QgsUnitTypes:: scaledDistance and scaledArea which are used in turn by QgsUnitTypes/QgsDistanceArea formatDistance/formatArea and then by the homonymous ones of QgsMeasureDialog and QgsMapToolIdentify.

It seems to me that there is a flaw in qgsRound which leads to inconsistent results of the Measure Tool in QGIS 3. (See the difference underlined in red in the screenshot at https://agiudiceandrea.github.io/QGIS330_3.PNG between segment length and total length values respectively rounded by
QLocale().toString() and by formatDistance through qgsRound).

In fact, 'scaleFactor' is declared as Int and thus it can store up to the 9th power of 10, consequently the rounding provides incorrect results from the 9th decimal place onwards.

In QGIS 2.18 this problem is not present, since both formatDistance and formatArea rely on QString().arg(), which works similarly to
QLocale().toString(), to round measurements, instead of on qgsRound() (which is also defined differently in 2.18 API).

For further information: http://osgeo-org.1560.x6.nabble.com/QGIS-Developer-On-precision-in-qgsRound-3-3-0-master-td5377378.html and https://github.com/qgis/QGIS/pull/7836#issuecomment-419718554

Associated revisions

Revision f9015f38
Added by Nyall Dawson over 6 years ago

Make qgsRound more tolerant to large values

Fixes #19844

History

#1 Updated by Giovanni Manghi over 6 years ago

  • Priority changed from Normal to High

#2 Updated by Nyall Dawson over 6 years ago

  • Assignee set to Nyall Dawson
  • Status changed from Open to In Progress

#3 Updated by Nyall Dawson over 6 years ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Closed

Also available in: Atom PDF