Merge pull request #526 from peterstace/no_validation_in_geos_wrapper

Don't validate results from GEOS

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)