clickbar / laravel-magellan

A modern PostGIS toolbox for Laravel
MIT License
193 stars 11 forks source link

fix(geometries): Fix stringifyFloat to correctly format number #73

Open saibotk opened 5 months ago

saibotk commented 5 months ago

We missed the "." here and thus PHP formatted numbers with the default precision of 6 and prefixed it with whitespaces.

Additionally the trims were adjusted, as we now do not need to trim any whitespaces. Also the trimming for the "." was made a rtrim instead to avoid wrongly stripping .1 to 1, even though this should never be a possible value that sprintf would produce.

See PHP docs: https://www.php.net/manual/en/function.sprintf.php

Thanks @ronnypolloqueri for reporting this. (ref #72)

saibotk commented 5 months ago

@ahawlitschek I'm unsure whether we should fix it like this and get cases where the numbers are "more" precise, but for some is more imprecise, see the tests for example. Or we may fix this in a different way, maybe check out how postgis / geos does it under the hood.

Also i just saw that maybe such imprecision could cause invalid geometries, at least according to ST_AsText, when setting precision there a warning exists about that: https://postgis.net/docs/ST_AsText.html

And PostGis seems to handle the value 50.12345 from the test cases perfectly. So i'd love to mirror this behaviour.

saibotk commented 5 months ago

Okay update:

Seems like PostGIS handles them the same way internally: CleanShot 2024-02-10 at 22 25 46 Or TablePlus is rounding those idk

But it's still weird and we should adjust our WKT generator to try to output the same strings as PostGIS does. This part might be useful form PostGIS codebase, which takes care of printing the decimal: https://github.com/postgis/postgis/blob/5560fb5cd14162a2d170a464f9e2b13e8998b1f7/deps/ryu/d2s.c#L537