Opened 11 years ago
Closed 11 years ago
#4109 closed enhancement (fixed)
Add OGRWriteToShapeBin support for writing Shape binary arrays
Description
Both the PGeo and FGDB drivers need to write shape objects, divorced from the rest of the file structure. There is already OGRCreateFromShapeBin, this patch is the reverse process, a OGRWriteToShapeBin function.
Attachments (2)
Change History (10)
Changed 11 years ago by
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
I think I would prefer shapefile export to be implemented in ogrpgeogeometry.h/cpp instead of distributed through the ogr_geometry.h and the various low level geometry classes. This would be more in keeping with the OGRCreateFromShapeBin function.
comment:3 Changed 11 years ago by
I started writing it that way, but the code seemed really fugly, so I backed off and followed the exportToWkb pattern instead, which seemed a lot nicer. Preference or requirement?
comment:4 Changed 11 years ago by
Let just say, if I'm going to be the one applying the patch I'd end up rewriting it my way. If Even wants to apply it as is, he is within his rights.
I don't see such a public class as OGRGeometry as an appropriate place for a helper function to simplify writing three drivers.
comment:5 Changed 11 years ago by
I had a bit the same reaction as Frank. Plus the fact that the name exportToShp() is a bit confusing because it might imply that the blob is compatible with shapefile blobs. But when you look in details, a PolylineZ for PGEO/FGDB is exported with type 10, which is not recognized by the shapefile driver.
shapefil.h (consistant with ESRI_shapefile_technical_description.pdf page 4) :
#define SHPT_ARC 3 #define SHPT_ARCZ 13 #define SHPT_ARCM 23
ogrpgeogemetry.h (consistant with extended_shapefile_format.pdf page 3) :
#define SHPT_ARC 3 #define SHPT_ARCM 23 #define SHPT_ARCZM 13 #define SHPT_ARCZ 10
Note that from wednesday, I will be away for the next 2 weeks, so Frank might as well apply an updated patch.
comment:6 Changed 11 years ago by
Working on updated patch now.
comment:7 Changed 11 years ago by
A few remarks :
- it would be good if you could stick to the 4 spaces indentation convention used elsewhere in the GDAL/OGR base code.
- I have noticed a few cases where the ShpSize?() method uses the number of sub-geometries, but not the number of *non-empty* sub-geometries (but the serialization code does skip them appropriately). In OGRMultiLineString and OGRMultiPoint as far as I can see.
- You should also be careful when calling get_IsClosed(). I believe the current implementation will segfault when nPointCount < 2 (see OGRLineString::EndPoint?() that calls getPoint( nPointCount-1, poPoint ) and the asserts in getPoint()). Might be worth fixing it or checking the number of point before calling it as it is done elsewhere in the basecode
Changed 11 years ago by
rewrite for frank and evan
patch generated in ./gdal/ogr