Ticket #4109 (closed enhancement: fixed)

Opened 2 years ago

Last modified 2 years ago

Add OGRWriteToShapeBin support for writing Shape binary arrays

Reported by: pramsey Owned by: rouault
Priority: normal Milestone: 1.9.0
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords: shape pgeo fgdb
Cc: warmerdam

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

shapewriter-20110603.patch Download (54.4 KB) - added by pramsey 2 years ago.
patch generated in ./gdal/ogr
shapewriter-20110607.patch Download (27.8 KB) - added by pramsey 2 years ago.
rewrite for frank and evan

Change History

Changed 2 years ago by pramsey

patch generated in ./gdal/ogr

Changed 2 years ago by pramsey

  • owner changed from warmerdam to rouault

Changed 2 years ago by warmerdam

  • cc warmerdam added
  • component changed from default to OGR_SF

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.

Changed 2 years ago by pramsey

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?

Changed 2 years ago by warmerdam

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.

Changed 2 years ago by rouault

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.

Changed 2 years ago by pramsey

Working on updated patch now.

Changed 2 years ago by rouault

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 2 years ago by pramsey

rewrite for frank and evan

Changed 2 years ago by rouault

  • keywords pgeo fgdb added
  • status changed from new to closed
  • resolution set to fixed
  • milestone set to 1.9.0

r22512 /trunk/gdal/ogr/ (ogrpgeogeometry.cpp ogrpgeogeometry.h): PGeo: add support for decoding multipoint/multipointz geometries; add OGRWriteToShapeBin() to serialize an OGR geometry object into a PGeo/FGDB geometry blob (patch by Paul Ramsey, #4109)

Note: See TracTickets for help on using tickets.