[gdal-dev] Call for discussion about RFC 61: Support for measures in geometries

[Even Rouault]

Le mercredi 03 février 2016 14:36:14, Ari Jolma a écrit :
> I finally have the (existing) autotests passing with the new API -
> basically just replacing the nCoordDimension property of OGRGeometry
> with flags (for Z and M) - and a rather large set of tests for
> import/export WKT and WKB with M passing.

That's great !

> 
> https://travis-ci.org/ajolma/GDAL-XYZM/builds/106725974
> 
> Maybe we should move to a vote on the RFC?

Sounds good.

A few notes:

- in the implementation, OGR_G_NOT_EMPTY has become OGR_G_NOT_EMPTY_POINT
So the mention os "IsEmpty = !(flags & OGR_G_NOT_EMPTY)" is probably no longer 
relevant

- The signature of the following methods
	int getFlags() const;
 	int setFlags( int newFlags ) const;
  is :
	virtual int getFlags() const;
 	virtual int setFlags( int newFlags );  <<< no const

- "It needs to be decided but when any of these flags is set, the corresponding 
data becomes invalid and may be discarded." : what did you implement ?

- Regarding "There is some discrepancy in documentation. Their documentation 
says that they may return zero for empty points while in ogrpoint.cpp it says 
that negative nCoordDimension values are used for empty points and the 
getCoordinateDimension method of point returns absolute value of 
nCoordDimension - thus not zero. ". 
From what I can see from history, I'm the likely culprit to have change that 
in GDAL 2.0. I think this is a side effect of the nCoordDimension trick to have 
-3 to be able to have "POINT Z EMPTY". But your approach with OGR_G_NOT_EMPTY 
can probably now solve the issue while having getCoordinateDimension() to 
return 0, but probably not worth changing that back now. By the way, I've just 
have a look at Simple Feature Access 1.2.1 and it does not specify what the 
method should do. I've checked with PostGIS 
http://postgis.net/docs/ST_CoordDim.html and it returns 3 for POINT Z EMPTY. 
So we perhaps just need to fix the doc to remove the mention of 0 being 
returned for point empty.

- "Change the semantics of OGRReadWKBGeometryType: b3D is not used and the 
returned eGeometryType may may any valid type" : why not just dropping the b3D 
parameter ? This is an internal function (ogr_p.h is not intended to be used 
other than in core and drivers)

- Regarding "Compatibility Issues", do you intend doing similarly to curve 
support in the I* methods if MeasuredGeometries is not advertized ?

- We would also probably need a section in MIGRATION_GUIDE.TXT to mention that 
new geometry types can be returned, that shape XYM-as-XYZ hack will be 
replaced by proper XYM

Even

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com