Add RFC99 text: Geometry coordinate precision

[rouault]

Partially rendered view at https://github.com/rouault/gdal/blob/rfc99_text/doc/source/development/rfc/rfc99_geometry_coordinate_precision.rst

[dbaston]

Can/should precision be detected in the way that column types are?

[rouault]

Obviously it could, but I'm not sure I want to do that implementation effort :-). It could be relatively easy when constructing point geometries from X,Y columns, but it becomes trickier when importing them from WKT, because our WKT import code should take note of that.

[theroggy]

If I understand correctly (which might not be the case!), the coordinates will be rounded as such...
However, simply rounding coordinates can/will lead to invalid polygons (e.g. self-intersections, polygon rings touching,...).
So, I wonder, why not use GEOS set_precision to round the coordinates in the export methods?

[rouault]

the coordinates will be rounded as such...

yes

However, simply rounding coordinates can/will lead to invalid polygons (e.g. self-intersections, polygon rings touching,...).

This may indeed be possible if users indicate a precision not consistent of the actual point spacing of their geometries

So, I wonder, why not use GEOS set_precision to round the coordinates in the export methods?

Good suggestion. However there would be a cost in doing so systematically. But I'm inclined to add a GEOSGeom_setPrecision_r() step in a ogr2ogr workflow when the user explicitly specifies -xyRes, which can indeed causes generation of invalid geometries. For other workflows, the assumption is that the coordinate precision of the source is correct.

[rouault]

RFC text and implementation updated with the above

[theroggy]

Maybe a configuration option could be introduced to give the user the option to apply set_precision systematically when writing?

[rouault]

Maybe a configuration option could be introduced to give the user the option to apply set_precision systematically when writing?

It is not obvious to me that this is needed at that point, because either people use ogr2ogr and it takes care of that, or they use individual steps of the OGR API (CreateLayer(), CreateFeature()) and can explicitly invoke OGRGeometry::SetPrecision() if needed. A config option to do that systematically (not clear at that point if that would be in each driver that honours coordinate precision, or in in the generic CreateFeature() code) could potentially be added as a further enhancement

[theroggy]

I was thinking about the drivers that honour the coordinate precision: if the precision is set and the configuration option is enabled, the validity of the geometries are guaranteed by gdal but a performance penalty can occur. If the the precision is set and configuration option is not enabled... it is the user's responsibility to make sure the geometries are rounded in a way they are valid as otherwise they can end up becoming invalid when saved.

[rouault]

oh well, I might consider doing this, but for GeoPackage (or formats that would use the same strategy), the value that is written will not be "exactly" the one after rounding to the precision, due to zeroing out least significant bits. It will be compatible with the requested precision however. That is if you ask for a precision of let's say 0.1 and you encode 0.1, the value you get when reading back will be 0.125 for example. Would that be a problem? My intuition would be that if the geometries are valid with respect to the 0.1 precision, those small changes of coordinate values < 0.1 shouldn't induce new in-validities, but I might be wrong...
If so, I was thinking about potentially also applying the precision when reading back so that 0.125 gets seen as 0.1, but that cost might be undesirable, and would only be done by the GDAL reader, so likely not a good option.
Or perhaps we should apply the zeroing out of LSBs in GeoPackage WKB only if a specific creation option is specified like DISCARD_COORD_LSB=YES ? (not sure which default should be used)

[theroggy]

Good questions. I follow the same intuition regarding the probable limited impact of zeroing LSBs, but I'm not at all a specialist in floating point representations, so not sure at all.

Applying precision when reading indeed sounds as not ideal... hopefully it can be avoided.

Maybe it is worth a test to see if zeroing out the LSB's after using GEOS set_precision is still having a significant impact?

[rouault]

Maybe it is worth a test to see if zeroing out the LSB's after using GEOS set_precision is still having a significant impact?

on a quick check, as expected, the zeroing out of the LSB's can be reversed to the initial "decimal" precision, since we are conservative on the number of zeroed LSB's, and the geometry is still valid. However I've revered the default behaviour of the GPKG driver to not do that zeroing out by default. See the last commit (4fb6233) of this PR for the updates, as well as the new OGR_APPLY_GEOM_SET_PRECISION configuration option that is taken into account by the generic code path of CreateFeature() to run OGRGeometry::SetPrecision().

[theroggy]

Looks great.

[PostholerCom]

1. It might be interesting, if not useful, to break out the -xyRes into -xRes and -yRes as to keep continuity with -zRes and -mRes (and gdal*) switches for complete control over the precision and for non strictly geo-spatial uses of ogr2ogr.

2. Say I have a CSV file with the desired highest resolution found in any geom at 4 decimal places. When I ogr2ogr that CSV to Shapefile it adds excess precision. Is it possible to have ogr detect the highest precision without implicitly specifying it? Can that be done without reading ahead the entire source file? Maybe precision can be adjusted upwards on-the-fly as the file is read?

If so, the precision can be determined from the source without specifically adding meta data or modifying the driver(s)?

[rouault]

1. It might be interesting, if not useful, to break out the -xyRes into -xRes and -yRes as to keep continuity with -zRes and -mRes (and gdal*) switches for complete control over the precision and for non strictly geo-spatial uses of ogr2ogr.

I'm not super keen on that. This could be done outside of GDAL if needed

Say I have a CSV file with the desired highest resolution found in any geom at 4 decimal places. When I ogr2ogr that CSV to Shapefile it adds excess precision. Is it possible to have ogr detect the highest precision without implicitly specifying it? Can that be done without reading ahead the entire source file?

It would require to read ahead the entire source file. Could potentially be done as a further enhancement of the CSV driver if desired (cf #9300 (review))

Maybe precision can be adjusted upwards on-the-fly as the file is read?

Wouldn't fit at all with the spirit of this RFC where the coordinate precision is, once determined, a constant value at the geometry-column level. In ogr2ogr type of scenarios, you need to declare the coordinate precision of the output layer at the time it is created

[theroggy]

Not sure if this is relevant, but just for information.

The motivation why I'm interested in applying precisions in general... is to avoid slivers. And I like that this feature offers a way to standardise/specify the precision at the layer/dataset level in a clear and transparent way. When calculating several overlays between different layers after each other, or if calculating overlays between layers where features have been snapped on line segments of the other layer, slivers or dangling nodes of typically ~1e-8 (m) or narrower will appear in the result due to rounding issues. Using a GEOS set_precision of >=1e-8 will remove such dangling nodes from larger polygons and will change sliver polygons to EMPTY geometries or NULL, making it easy to identify and remove them.

[schwehr]

Can we work towards unifying on just one of British or American English? SetFromMetre versus SetFromMeter.

I suggest we aim for American English based on that dominating the current code base.

A partial look at the code base for just meter versus metre that shows that the code base is very inconsistent for this particular word.

meter:

find frmts ogr port autotest -name '*.cpp' | xargs egrep 'meter' | egrep -i -v 'diameter|parameter' | wc -l
183
find frmts ogr port autotest -name '*.h' | xargs egrep 'meter' | egrep -i -v 'diameter|parameter' | wc -l
98
find frmts ogr port autotest -name '*.py' | xargs egrep 'meter' | egrep -i -v 'diameter|parameter' | wc -l
46

metre:

find frmts ogr port autotest -name '*.cpp' | xargs egrep 'metre' | egrep -i -v 'diameter|parameter' | wc -l
74
find frmts ogr port autotest -name '*.h' | xargs egrep 'metre' | egrep -i -v 'diameter|parameter' | wc -l
3
find frmts ogr port autotest -name '*.py' | xargs egrep 'metre' | egrep -i -v 'diameter|parameter' | wc -l
215

[rouault]

I suggest we aim for American English

done

[rouault]

Motion adopted. Will wait before merging (after updating voting history & status) that the PR with the candidate implementation is also merged