georust / geos

Rust bindings for GEOS
https://docs.rs/geos/
MIT License
121 stars 41 forks source link

`get_coord_seq` without extra clone #137

Open kylebarron opened 11 months ago

kylebarron commented 11 months ago

From the docstring of get_coord_seq:

Note: this clones the underlying CoordSeq to avoid double free (because CoordSeq handles the object ptr and the CoordSeq is still owned by the geos geometry) if this method’s performance becomes a bottleneck, feel free to open an issue, we could skip this clone with cleaner code.

My use case is to bind GEOS algorithms to the GeoArrow memory layout (an efficient geometry layout for arrays of geometries, see geoarrow.org and my WIP rust implementation at https://github.com/geoarrow/geoarrow-rs). My current plan is to always store geometries before and after each operation in GeoArrow memory, and therefore GEOS objects are totally ephemeral during an operation. So the process goes like

  1. Take an array of e.g. polygons
  2. Iterate over the array, converting each into GEOS objects
  3. Apply a GEOS operation, say, buffering
  4. Construct a new GeoArrow array from the polygon outputs

Therefore the IO to and from GEOS objects is really important to me, because it's overhead for every operation on the array.

One option is to improve this get_coord_seq, removing a clone. The other possibility for me is to have something like into_coord_seq or into_inner. Given that I want to consume the GEOS geometry anyways, and only access its coords, this might be easier to implement?

I'd be willing to attempt a PR!

GuillaumeGomez commented 11 months ago

Both approaches sound good to me but if the into_coord_seq approach is simpler, please go ahead with this one.

Might still be worth to improve get_coord_seq in the future though.

kylebarron commented 11 months ago

Actually thinking about this a little more.. might one way to solve this be to implement a ConstCoordSeq struct just like there's a ConstGeometry?

GuillaumeGomez commented 11 months ago

That could work too.