From b9aa15ba3eacdfafad8588f0bd37accc5ae6d02e Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Thu, 18 Nov 2021 06:02:19 +1100 Subject: [PATCH 1/5] Wrap GEOS's MakeValid --- geos/entrypoints.go | 12 ++++++++++++ geos/entrypoints_test.go | 24 ++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/geos/entrypoints.go b/geos/entrypoints.go index 6b57a6d4..76238ff3 100644 --- a/geos/entrypoints.go +++ b/geos/entrypoints.go @@ -309,3 +309,15 @@ func SymmetricDifference(a, b geom.Geometry, opts ...geom.ConstructorOption) (ge }) return result, wrap(err, "executing GEOSSymDifference_r") } + +// MakeValid can be used to convert an invalid geometry into a valid geometry. +// It does this by keeping the original control points and constructing a new +// geometry that is valid and similar (but not the same as) the original +// invalid geometry. If the input geometry is valid, then it is returned +// unaltered. +func MakeValid(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) { + result, err := unaryOpG(g, opts, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry { + return C.GEOSMakeValid_r(ctx, g) + }) + return result, wrap(err, "executing GEOSMakeValid_r") +} diff --git a/geos/entrypoints_test.go b/geos/entrypoints_test.go index ec9f79f6..43ec1a66 100644 --- a/geos/entrypoints_test.go +++ b/geos/entrypoints_test.go @@ -7,9 +7,9 @@ import ( "github.com/peterstace/simplefeatures/geom" ) -func geomFromWKT(t *testing.T, wkt string) geom.Geometry { +func geomFromWKT(t *testing.T, wkt string, opts ...geom.ConstructorOption) geom.Geometry { t.Helper() - geom, err := geom.UnmarshalWKT(wkt) + geom, err := geom.UnmarshalWKT(wkt, opts...) if err != nil { t.Fatalf("could not unmarshal WKT:\n wkt: %s\n err: %v", wkt, err) } @@ -767,3 +767,23 @@ func TestSymmetricDifference(t *testing.T) { expectGeomEq(t, got, want, geom.IgnoreOrder) } + +func TestMakeValid(t *testing.T) { + for i, tt := range []struct { + input string + wantOutput string + }{ + { + "POLYGON((0 0,2 2,2 0,0 2,0 0))", + "MULTIPOLYGON(((0 0,0 2,1 1,0 0)),((2 0,1 1,2 2,2 0)))", + }, + } { + t.Run(strconv.Itoa(i), func(t *testing.T) { + in := geomFromWKT(t, tt.input, geom.DisableAllValidations) + gotGeom, err := MakeValid(in) + expectNoErr(t, err) + wantGeom := geomFromWKT(t, tt.wantOutput) + expectGeomEq(t, gotGeom, wantGeom) + }) + } +} From 88cd41bdf157cd37a4db62c73b12b5d24583c942 Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Thu, 18 Nov 2021 06:05:53 +1100 Subject: [PATCH 2/5] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75e182fd..1650c9f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ __Special thanks to Albert Teoh and Sameera Perera for contributing to this rele - Fixes a bug in Simplify where invalid interior rings would be omitted rather than producing an error. +- Adds a wrapper in the `geos` package for the `GEOSMakeValid_r` function + (exposed as `MakeValid`). + ## v0.34.0 2021-11-02 From c8e5fa0c16f350a8cf8d2019cbd5912f7d74ac7c Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Fri, 19 Nov 2021 06:31:17 +1100 Subject: [PATCH 3/5] Use a stub implementation of MakeValid when not supported by GEOS --- geos/entrypoints.go | 15 +++++++++++++++ geos/entrypoints_test.go | 3 +++ geos/errors.go | 20 ++++++++++++++++++++ geos/handle.go | 2 +- 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/geos/entrypoints.go b/geos/entrypoints.go index 76238ff3..a6a8fb3b 100644 --- a/geos/entrypoints.go +++ b/geos/entrypoints.go @@ -4,6 +4,17 @@ package geos #cgo LDFLAGS: -lgeos_c #cgo CFLAGS: -Wall #include "geos_c.h" + +#define MAKE_VALID_MIN_VERSION "3.8.0" +#define MAKE_VALID_MISSING ( \ + GEOS_VERSION_MAJOR < 3 || \ + (GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR < 8) \ +) +#if MAKE_VALID_MISSING +// This stub implementation also fails +GEOSGeometry *GEOSMakeValid_r(GEOSContextHandle_t handle, const GEOSGeometry* g) { return NULL; } +#endif + */ import "C" @@ -316,6 +327,10 @@ func SymmetricDifference(a, b geom.Geometry, opts ...geom.ConstructorOption) (ge // invalid geometry. If the input geometry is valid, then it is returned // unaltered. func MakeValid(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) { + if C.MAKE_VALID_MISSING != 0 { + return geom.Geometry{}, unsupportedGEOSVersionError{ + C.MAKE_VALID_MIN_VERSION, "MakeValid"} + } result, err := unaryOpG(g, opts, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry { return C.GEOSMakeValid_r(ctx, g) }) diff --git a/geos/entrypoints_test.go b/geos/entrypoints_test.go index 43ec1a66..fef8096c 100644 --- a/geos/entrypoints_test.go +++ b/geos/entrypoints_test.go @@ -781,6 +781,9 @@ func TestMakeValid(t *testing.T) { t.Run(strconv.Itoa(i), func(t *testing.T) { in := geomFromWKT(t, tt.input, geom.DisableAllValidations) gotGeom, err := MakeValid(in) + if _, ok := err.(unsupportedGEOSVersionError); ok { + t.Skip(err) + } expectNoErr(t, err) wantGeom := geomFromWKT(t, tt.wantOutput) expectGeomEq(t, gotGeom, wantGeom) diff --git a/geos/errors.go b/geos/errors.go index ea29198d..a0cd66ad 100644 --- a/geos/errors.go +++ b/geos/errors.go @@ -2,9 +2,29 @@ package geos import "fmt" +// #include "geos_c.h" +import "C" + func wrap(err error, format string, args ...interface{}) error { if err == nil { return nil } return fmt.Errorf(format+": %w", append(args, err)...) } + +var currentGEOSVersion = fmt.Sprintf( + "%d.%d.%d", + C.GEOS_VERSION_MAJOR, + C.GEOS_VERSION_MINOR, + C.GEOS_VERSION_PATCH, +) + +type unsupportedGEOSVersionError struct { + requiredGEOSVersion string + operation string +} + +func (e unsupportedGEOSVersionError) Error() string { + return fmt.Sprintf("%s is unsupported in GEOS %s, requires at least GEOS %s", + e.operation, currentGEOSVersion, e.requiredGEOSVersion) +} diff --git a/geos/handle.go b/geos/handle.go index 93845d75..c7eacc1d 100644 --- a/geos/handle.go +++ b/geos/handle.go @@ -22,7 +22,6 @@ char *marshal(GEOSContextHandle_t handle, const GEOSGeometry *g, size_t *size, c GEOSGeometry const *noop(GEOSContextHandle_t handle, const GEOSGeometry *g) { return g; } - */ import "C" @@ -62,6 +61,7 @@ func newHandle() (*handle, error) { h.release() return nil, errors.New("malloc failed") } + C.memset((unsafe.Pointer)(h.errBuf), 0, 1024) h.context = C.sf_init(unsafe.Pointer(h.errBuf)) if h.context == nil { From 1fa5b980cb7b713699400758bfea82c18c87de30 Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Fri, 19 Nov 2021 06:34:54 +1100 Subject: [PATCH 4/5] Fix typo in comment --- geos/entrypoints.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/geos/entrypoints.go b/geos/entrypoints.go index a6a8fb3b..828c1a77 100644 --- a/geos/entrypoints.go +++ b/geos/entrypoints.go @@ -11,7 +11,7 @@ package geos (GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR < 8) \ ) #if MAKE_VALID_MISSING -// This stub implementation also fails +// This stub implementation always fails: GEOSGeometry *GEOSMakeValid_r(GEOSContextHandle_t handle, const GEOSGeometry* g) { return NULL; } #endif From 65c283780294dd28c604144e5a1b32be7a024a28 Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Mon, 22 Nov 2021 05:18:54 +1100 Subject: [PATCH 5/5] Check input to MakeValid tests are invalid --- geos/entrypoints_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/geos/entrypoints_test.go b/geos/entrypoints_test.go index fef8096c..32ccf8be 100644 --- a/geos/entrypoints_test.go +++ b/geos/entrypoints_test.go @@ -23,6 +23,13 @@ func expectNoErr(t *testing.T, err error) { } } +func expectErr(t *testing.T, err error) { + t.Helper() + if err == nil { + t.Fatal("unexpected error but got nil") + } +} + func expectGeomEq(t *testing.T, got, want geom.Geometry, opts ...geom.ExactEqualsOption) { t.Helper() if !geom.ExactEquals(got, want, opts...) { @@ -779,6 +786,8 @@ func TestMakeValid(t *testing.T) { }, } { t.Run(strconv.Itoa(i), func(t *testing.T) { + _, err := geom.UnmarshalWKT(tt.input) + expectErr(t, err) in := geomFromWKT(t, tt.input, geom.DisableAllValidations) gotGeom, err := MakeValid(in) if _, ok := err.(unsupportedGEOSVersionError); ok {