feat: Segmentize for GeosCapi geometries.

[arkirchner]

Summary

Modify geometry to have no segment longer then the max segment distance.

Feature is equal to:
ST_Segmentize
shapely segmentize

Other Information

• ffi support is planned: https://github.com/dark-panda/ffi-geos/pull/34/files
• I decided against calling the method like the Geos feature because it is popular as segmentize through PostGIS and shapely.

[keithdoggett]

@arkirchner thanks for the PR! This looks good, but I have 2 notes

1. Looks like there's some formatting issues. You can use bin/clang-format format to fix it.
2. Do you know what version of Geos this function is available in? We may want to add guards around the method.

@BuonOmo had done some work around getting C methods added in a better way so maybe he has some thoughts too

[BuonOmo]

I think we should raise if the given maximum length isn't a number. Also I don't know what happens when we give 0 or a negative value to the segment. I know that the Geos doc is not talking about any errors. But there still might be one!

Also I didn't understand your last point in the PR description (cannot quote you from my phone)

[arkirchner]

@keithdoggett

Looks like there's some formatting issues. You can use bin/clang-format format to fix it.

I was able to fix the format.

Do you know what version of Geos this function is available in? We may want to add guards around the method.

It is available from GEOS 3.10 onwards. I updated the README to mention this. How can I add a guard.

[arkirchner]

@BuonOmo
I added some tests for invalid inputs. The existing error messages are quite good. 🤔

[keithdoggett]

@BuonOmo I was referencing the work you had done on this PR #334. I know part of the goal of that was to better understand how to add more Geos functions in a standardized manner, so I thought you might have some insights here.

[keithdoggett]

@arkirchner to add a guard, in extconf.rb we would add a have_func check for GEOSDensify

if have_header("geos_c.h")
  found_geos = true if have_func("GEOSSetSRID_r", "geos_c.h")
  have_func("GEOSPreparedContains_r", "geos_c.h")
  have_func("GEOSPreparedDisjoint_r", "geos_c.h")
  have_func("GEOSUnaryUnion_r", "geos_c.h")
  have_func("GEOSCoordSeq_isCCW_r", "geos_c.h")
  # new func
  have_func("GEOSDensify", "geos_c.h")

  have_func("rb_memhash", "ruby.h")
  have_func("rb_gc_mark_movable", "ruby.h")
end

Then in preface.h we would define RGEO_GEOS_SUPPORT_GEOS_DENSIFY if have_func succeeds.

#ifdef HAVE_GEOSDENSIFY
#define RGEO_GEOS_SUPPORTS_DENSIFY
#endif

Then in geometry.c we would wrap both the method_geometry_segmentize and rb_define_method blocks with an #ifdef on our new macro.

# ifdef RGEO_GEOS_SUPPORTS_DENSIFY
rb_define_method(
    geos_geometry_methods, "segmentize", method_geometry_segmentize, 1);
}
#endif

This will prevent calls that will crash the Ruby VM if you're using an older version of Geos. Eventually we should just move to requiring a minimum version of Geos at compile time so we don't have to check individual methods. @BuonOmo thoughts on this?

[arkirchner]

@keithdoggett Thank you for the detailed explanation. I wrapped both the method definition and the function to not cause any issues when compiling.

I reverted the change to the README because Geos version requirements do not change when using this method.

How about the tests. Should they be skipped for unsupported Geos versions?

[keithdoggett]

@arkirchner thank you for implementing that! And yes if we could skip the test based on version too that would be great.

[arkirchner]

@keithdoggett If you have time could you look at my test solution?

[keithdoggett]

@arkirchner LGTM once that RuboCop error is fixed I'll merge! Thanks for the work on this and apologies about the delays in review.

[arkirchner]

@keithdoggett
Thank you very much. I fixed Rubocop and updated the History.

Should I squash everything once before merging?

[keithdoggett]

LGTM sorry about the delays. Thanks @arkirchner