Don't validate results from GEOS

Recent versions of GEOS use OverlayNG (NG stands for New Generation) for
set operations between geometries (Intersection, Union, etc.). With the
introduction of OverlayNG, invalid geometries are no longer frequently
produced by GEOS. They are either non-existent, or very rare.

Before this change, the `simplefeatures` GEOS wrapper validates results
from GEOS.

This feels like a misalignment:

- Validation isn't free, so validating GEOS results seems like the wrong
  default.

- The GEOS wrapper is doing more than strictly wrapping the GEOS library
  (it is also validating the result). This blurs the lines between what
  simplefeatures is doing and what GEOS is doing.

This change removes that validation. Users can of course still validate
manually by calling the `Validate` method on the result.

geos/benchmark_test.go

@@ -33,31 +33,14 @@ func regularPolygon(center geom.XY, radius float64, sides int) geom.Polygon {
 	return poly
 }
 
-func BenchmarkIntersectionWithoutValidation(b *testing.B) {
-	for _, sz := range []int{10, 100, 1000, 10000} {
-		b.Run(fmt.Sprintf("n=%d", sz), func(b *testing.B) {
-			inputA := regularPolygon(geom.XY{X: 0, Y: 0}, 1.0, sz).AsGeometry()
-			inputB := regularPolygon(geom.XY{X: 1, Y: 0}, 1.0, sz).AsGeometry()
-			b.ResetTimer()
-
-			for i := 0; i < b.N; i++ {
-				_, err := Intersection(inputA, inputB, geom.DisableAllValidations)
-				if err != nil {
-					b.Fatal(err)
-				}
-			}
-		})
-	}
-}
-
 func BenchmarkNoOp(b *testing.B) {
 	for _, sz := range []int{10, 100, 1000, 10000} {
 		b.Run(fmt.Sprintf("n=%d", sz), func(b *testing.B) {
 			input := regularPolygon(geom.XY{X: 0, Y: 0}, 1.0, sz).AsGeometry()
 			b.ResetTimer()
 
 			for i := 0; i < b.N; i++ {
-				_, err := noop(input, geom.DisableAllValidations)
+				_, err := noop(input)
 				if err != nil {
 					b.Fatal(err)
 				}

geos/doc.go

@@ -4,6 +4,10 @@
 // Its purpose is to provide functionality that has been implemented in GEOS,
 // but is not yet available natively in the simplefeatures library.
 //
+// Results from functions in this package are returned from GEOS unvalidated
+// and as-is. Users may call the Validate method on results if they wish to
+// check result validity.
+//
 // The operations in this package ignore Z and M values if they are present.
 //
 // To use this package, you will need to install the GEOS library.

geos/entrypoints.go

@@ -179,8 +179,10 @@ func Overlaps(a, b geom.Geometry) (bool, error) {
 // Union returns a geometry that is the union of the input geometries.
 // Formally, the returned geometry will contain a particular point X if and
 // only if X is present in either geometry (or both).
-func Union(a, b geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	g, err := binaryOpG(a, b, opts, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
+//
+// The validity of the result is not checked.
+func Union(a, b geom.Geometry) (geom.Geometry, error) {
+	g, err := binaryOpG(a, b, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSUnion_r(ctx, a, b)
 	})
 	return g, wrap(err, "executing GEOSUnion_r")
@@ -189,8 +191,10 @@ func Union(a, b geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, e
 // Intersection returns a geometry that is the intersection of the input
 // geometries. Formally, the returned geometry will contain a particular point
 // X if and only if X is present in both geometries.
-func Intersection(a, b geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	g, err := binaryOpG(a, b, opts, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
+//
+// The validity of the result is not checked.
+func Intersection(a, b geom.Geometry) (geom.Geometry, error) {
+	g, err := binaryOpG(a, b, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSIntersection_r(ctx, a, b)
 	})
 	return g, wrap(err, "executing GEOSIntersection_r")
@@ -204,7 +208,6 @@ type bufferOptionSet struct {
 	endCapStyle  int
 	joinStyle    int
 	mitreLimit   float64
-	ctorOpts     []geom.ConstructorOption
 }
 
 func newBufferOptionSet(opts []BufferOption) bufferOptionSet {
@@ -278,19 +281,13 @@ func BufferJoinStyleBevel() BufferOption {
 	}
 }
 
-// BufferConstructorOption sets constructor option that are used when
-// reconstructing the buffered geometry that is returned from the GEOS lib.
-func BufferConstructorOption(opts ...geom.ConstructorOption) BufferOption {
-	return func(bos *bufferOptionSet) {
-		bos.ctorOpts = append(bos.ctorOpts, opts...)
-	}
-}
-
 // Buffer returns a geometry that contains all points within the given radius
 // of the input geometry.
+//
+// The validity of the result is not checked.
 func Buffer(g geom.Geometry, radius float64, opts ...BufferOption) (geom.Geometry, error) {
 	optSet := newBufferOptionSet(opts)
-	result, err := unaryOpG(g, optSet.ctorOpts, func(ctx C.GEOSContextHandle_t, gh *C.GEOSGeometry) *C.GEOSGeometry {
+	result, err := unaryOpG(g, func(ctx C.GEOSContextHandle_t, gh *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSBufferWithStyle_r(
 			ctx, gh, C.double(radius),
 			C.int(optSet.quadSegments),
@@ -306,26 +303,32 @@ func Buffer(g geom.Geometry, radius float64, opts ...BufferOption) (geom.Geometr
 // Douglas-Peucker algorithm. Topological invariants may not be maintained,
 // e.g. polygons can collapse into linestrings, and holes in polygons may be
 // lost.
-func Simplify(g geom.Geometry, tolerance float64, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	result, err := unaryOpG(g, opts, func(ctx C.GEOSContextHandle_t, gh *C.GEOSGeometry) *C.GEOSGeometry {
+//
+// The validity of the result is not checked.
+func Simplify(g geom.Geometry, tolerance float64) (geom.Geometry, error) {
+	result, err := unaryOpG(g, func(ctx C.GEOSContextHandle_t, gh *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSSimplify_r(ctx, gh, C.double(tolerance))
 	})
 	return result, wrap(err, "executing GEOSSimplify_r")
 }
 
 // Difference returns the geometry that represents the parts of input geometry
 // A that are not part of input geometry B.
-func Difference(a, b geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	result, err := binaryOpG(a, b, opts, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
+//
+// The validity of the result is not checked.
+func Difference(a, b geom.Geometry) (geom.Geometry, error) {
+	result, err := binaryOpG(a, b, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSDifference_r(ctx, a, b)
 	})
 	return result, wrap(err, "executing GEOSDifference_r")
 }
 
 // SymmetricDifference returns the geometry that represents the parts of the
 // input geometries that are not part of the other input geometry.
-func SymmetricDifference(a, b geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	result, err := binaryOpG(a, b, opts, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
+//
+// The validity of the result is not checked.
+func SymmetricDifference(a, b geom.Geometry) (geom.Geometry, error) {
+	result, err := binaryOpG(a, b, func(ctx C.GEOSContextHandle_t, a, b *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSSymDifference_r(ctx, a, b)
 	})
 	return result, wrap(err, "executing GEOSSymDifference_r")
@@ -336,13 +339,15 @@ func SymmetricDifference(a, b geom.Geometry, opts ...geom.ConstructorOption) (ge
 // 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) {
+//
+// The validity of the result is not checked.
+func MakeValid(g geom.Geometry) (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 {
+	result, err := unaryOpG(g, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSMakeValid_r(ctx, g)
 	})
 	return result, wrap(err, "executing GEOSMakeValid_r")
@@ -362,13 +367,15 @@ func MakeValid(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry,
 // and return an error, but this is not guaranteed, and an invalid result may
 // be returned without an error. It is the responsibility of the caller to
 // ensure that the constraints are met before using this function.
-func CoverageUnion(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
+//
+// The validity of the result is not checked.
+func CoverageUnion(g geom.Geometry) (geom.Geometry, error) {
 	if C.COVERAGE_UNION_MISSING != 0 {
 		return geom.Geometry{}, unsupportedGEOSVersionError{
 			C.COVERAGE_UNION_MIN_VERSION, "CoverageUnion",
 		}
 	}
-	result, err := unaryOpG(g, opts, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
+	result, err := unaryOpG(g, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSCoverageUnion_r(ctx, g)
 	})
 	return result, wrap(err, "executing GEOSCoverageUnion_r")
@@ -377,8 +384,10 @@ func CoverageUnion(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geomet
 // UnaryUnion is a single argument version of Union. It is most useful when
 // supplied with a GeometryCollection, resulting in the union of the
 // GeometryCollection's child geometries.
-func UnaryUnion(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	result, err := unaryOpG(g, opts, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
+//
+// The validity of the result is not checked.
+func UnaryUnion(g geom.Geometry) (geom.Geometry, error) {
+	result, err := unaryOpG(g, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.GEOSUnaryUnion_r(ctx, g)
 	})
 	return result, wrap(err, "executing GEOSUnaryUnion_r")

geos/entrypoints_test.go

@@ -579,7 +579,7 @@ type BinaryOperationTestCase struct {
 	Out      string
 }
 
-func RunBinaryOperationTest(t *testing.T, fn func(a, b geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error), cases []BinaryOperationTestCase) {
+func RunBinaryOperationTest(t *testing.T, fn func(a, b geom.Geometry) (geom.Geometry, error), cases []BinaryOperationTestCase) {
 	for i, c := range cases {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
 			run := func(rev bool) func(t *testing.T) {

geos/generic_operations.go

@@ -43,7 +43,6 @@ func binaryOpE(
 
 func binaryOpG(
 	g1, g2 geom.Geometry,
-	opts []geom.ConstructorOption,
 	op func(C.GEOSContextHandle_t, *C.GEOSGeometry, *C.GEOSGeometry) *C.GEOSGeometry,
 ) (geom.Geometry, error) {
 	var result geom.Geometry
@@ -54,7 +53,7 @@ func binaryOpG(
 		}
 		defer C.GEOSGeom_destroy(resultGH)
 		var err error
-		result, err = h.decode(resultGH, opts)
+		result, err = h.decode(resultGH)
 		return wrap(err, "decoding result")
 	})
 	return result, err
@@ -95,7 +94,6 @@ func unaryOpE(g geom.Geometry, op func(*handle, *C.GEOSGeometry) error) error {
 
 func unaryOpG(
 	g geom.Geometry,
-	opts []geom.ConstructorOption,
 	op func(C.GEOSContextHandle_t, *C.GEOSGeometry) *C.GEOSGeometry,
 ) (geom.Geometry, error) {
 	var result geom.Geometry
@@ -111,7 +109,7 @@ func unaryOpG(
 			defer C.GEOSGeom_destroy(resultGH)
 		}
 		var err error
-		result, err = h.decode(resultGH, opts)
+		result, err = h.decode(resultGH)
 		return wrap(err, "decoding result")
 	})
 	return result, err

geos/handle.go

@@ -37,8 +37,8 @@ import (
 // noop returns the geometry unaltered, via conversion to and from GEOS. This
 // function is only for benchmarking purposes, hence it is not exported or used
 // outside of benchmark tests.
-func noop(g geom.Geometry, opts ...geom.ConstructorOption) (geom.Geometry, error) {
-	result, err := unaryOpG(g, opts, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
+func noop(g geom.Geometry) (geom.Geometry, error) {
+	result, err := unaryOpG(g, func(ctx C.GEOSContextHandle_t, g *C.GEOSGeometry) *C.GEOSGeometry {
 		return C.noop(ctx, g)
 	})
 	return result, wrap(err, "executing noop")
@@ -158,7 +158,7 @@ func (h *handle) boolErr(c C.char) (bool, error) {
 	}
 }
 
-func (h *handle) decode(gh *C.GEOSGeometry, opts []geom.ConstructorOption) (geom.Geometry, error) {
+func (h *handle) decode(gh *C.GEOSGeometry) (geom.Geometry, error) {
 	var (
 		isWKT C.char
 		size  C.size_t
@@ -171,10 +171,10 @@ func (h *handle) decode(gh *C.GEOSGeometry, opts []geom.ConstructorOption) (geom
 
 	if isWKT != 0 {
 		wkt := C.GoStringN(serialised, C.int(size))
-		g, err := geom.UnmarshalWKT(wkt, opts...)
+		g, err := geom.UnmarshalWKT(wkt, geom.DisableAllValidations)
 		return g, wrap(err, "failed to unmarshal GEOS WKT result")
 	}
 	wkb := C.GoBytes(unsafe.Pointer(serialised), C.int(size))
-	g, err := geom.UnmarshalWKB(wkb, opts...)
+	g, err := geom.UnmarshalWKB(wkb, geom.DisableAllValidations)
 	return g, wrap(err, "failed to unmarshal GEOS WKB result")
 }

internal/perf/set_op_test.go

@@ -43,7 +43,7 @@ func BenchmarkSetOperation(b *testing.B) {
 }
 
 func adaptGEOSSetOp(
-	setOp func(_, _ geom.Geometry, _ ...geom.ConstructorOption) (geom.Geometry, error),
+	setOp func(_, _ geom.Geometry) (geom.Geometry, error),
 ) func(_, _ geom.Geometry) (geom.Geometry, error) {
 	return func(g1, g2 geom.Geometry) (geom.Geometry, error) {
 		return setOp(g1, g2)