03.02.2016, 16:11, Even Rouault kirjoitti:
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
changed into the draft
- "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 ?
the data is lost; changed into the draft
- 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.
added "fix the doc" to the draft
- "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)
Hm. It is installed. Then by definition it is not internal. If it is
changed, doesn't it make the shared object binary incompatible? (I see
it doesn't have CPL_DLL) I don't think there is a ogr_internal.h, maybe
there should be and move this function there?
- Regarding "Compatibility Issues", do you intend doing similarly to curve
support in the I* methods if MeasuredGeometries is not advertized ?
I don't have a very strong opinion, so I'm ok to follow the pattern used
for curves.
- 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
Yes.
Ari
Even
_______________________________________________
gdal-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
