[QGIS-Developer] On precision in qgsRound() (3.3.0 master)

[andreaerdna]

Nyall Dawson wrote
> On Mon, 10 Sep 2018 at 20:04, andreaerdna <

> andreaerdna@

> > wrote:
>> 'scaleFactor' should be qlonglong in order to correctly store at least
>> the
>> 12th power of 10
> 
> Yes, that looks correct! Can you open a PR with a test case fixing this?

I'm not sure if this simple fix could completely solve the problem in the
right way, avoiding any overflow.

Taking better account of any side effects:

now in QGIS 3.3.0 master, as said,
'scaleFactor' defined as Int overflows when 'places' > 9, leading to
incorrect rounding,
while static_cast<qlonglong>((number * scaleFactor) + 0.5) and the whole
'result' already overflow when 'number', the measure to round, is at least
about:

places = 8     -> 92'233'720'369 units
places = 9     ->  9'223'372'037 units (see overflow in action [1])
places >= 10 ->  4'294'967'296 units (see [2])


instead, with the "fix" [3],
'scaleFactor' defined as qlonglong doesn't overflow at all for 'places' <=
12 (actually <=19)
but then static_cast<qulonglong>((number * scaleFactor) + 0.5) overflows
when 'number' is at least about:

places = 8     -> 184'467'440'738 units (better then now)
places = 9     ->   18'446'744'074 units (better)
places = 10   ->     1'844'674'408 units (worse)
places = 11   ->       184'467'441 units (worse)
places = 12   ->         18'446'745 units (worse)


I can not fully understand the reasons [4,5] that led to rewrite qgsRound(),
introduce scaledDistance/scaledArea and modify formatDistance/formatArea,
anywayI think now it could be better to:

- completely avoid to use qgsRound() in scaledDistance/scaledArea and rely
only on QStringLiteral().arg() in formatDistance/formatArea to correctly
round the measurements value [6]
or
- reduce to 9 (or less) the maximum precision value allowed to be set in
Setting->Options->Map Tools and in Project Properties->General [7]
or
- introduce some kind of logic in qgsRound() to handle and prevent overflows
or
- (?)

Some tests are needed, but I can not do them all by myself for now.


[1] https://agiudiceandrea.github.io/QGIS330_5.PNG
[2] https://agiudiceandrea.github.io/QGIS330_4.PNG
[3]
https://github.com/agiudiceandrea/QGIS/commit/7c16a409c911eef415f286a2af530c0163a04b26
[4] "Introduce qgsRound since std::round is available only in C++11 onwards"
https://github.com/qgis/QGIS/pull/2367
[5] "Modularize distance and area formatting"
https://github.com/qgis/QGIS/pull/4213
[6]
https://github.com/agiudiceandrea/QGIS/commit/54d719d38bc823ab5f9a2d5d47d3d52b82599b43
[7]
https://github.com/agiudiceandrea/QGIS/commit/8cc0b358a479f10edea8956700b5548691404d80



-----
Andrea Giudiceandrea
--
Sent from: http://osgeo-org.1560.x6.nabble.com/QGIS-Developer-f4099106.html