Merge pull request #87 from paulmach/covering-error

maptile/tilecover: polygon converings can return errors

maptile/tilecover/benchmarks_test.go

@@ -82,7 +82,7 @@ func BenchmarkRussia_z0z9(b *testing.B) {
 	b.ReportAllocs()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		tiles := Geometry(g, 9)
+		tiles, _ := Geometry(g, 9)
 		MergeUp(tiles, 0)
 	}
 }

maptile/tilecover/cover_test.go

@@ -96,11 +96,11 @@ func TestTestdata(t *testing.T) {
 
 			expected := loadFeatureCollection(t, "./testdata/"+tc.name+"_out.geojson")
 
-			tiles := Geometry(f.Geometry, tc.max)
+			tiles, _ := Geometry(f.Geometry, tc.max)
 			result := MergeUp(tiles, tc.min).ToFeatureCollection()
 			compareFeatureCollections(t, tc.name, result, expected)
 
-			tiles = Geometry(f.Geometry, tc.max)
+			tiles, _ = Geometry(f.Geometry, tc.max)
 			result = MergeUpPartial(tiles, tc.min, 4).ToFeatureCollection()
 			compareFeatureCollections(t, tc.name, result, expected)
 		})
@@ -123,7 +123,7 @@ func TestCountries(t *testing.T) {
 	for _, country := range countries {
 		t.Run(country, func(t *testing.T) {
 			f := loadFeature(t, "./testdata/world/"+country+".geo.json")
-			tiles := Geometry(f.Geometry, 6)
+			tiles, _ := Geometry(f.Geometry, 6)
 			tiles = MergeUp(tiles, 1)
 
 			expected := loadFeatureCollection(t, "./testdata/world/"+country+"_out.geojson")

maptile/tilecover/helpers.go

@@ -9,20 +9,20 @@ import (
 )
 
 // Geometry returns the covering set of tiles for the given geometry.
-func Geometry(g orb.Geometry, z maptile.Zoom) maptile.Set {
+func Geometry(g orb.Geometry, z maptile.Zoom) (maptile.Set, error) {
 	if g == nil {
-		return nil
+		return nil, nil
 	}
 
 	switch g := g.(type) {
 	case orb.Point:
-		return Point(g, z)
+		return Point(g, z), nil
 	case orb.MultiPoint:
-		return MultiPoint(g, z)
+		return MultiPoint(g, z), nil
 	case orb.LineString:
-		return LineString(g, z)
+		return LineString(g, z), nil
 	case orb.MultiLineString:
-		return MultiLineString(g, z)
+		return MultiLineString(g, z), nil
 	case orb.Ring:
 		return Ring(g, z)
 	case orb.Polygon:
@@ -32,7 +32,7 @@ func Geometry(g orb.Geometry, z maptile.Zoom) maptile.Set {
 	case orb.Collection:
 		return Collection(g, z)
 	case orb.Bound:
-		return Bound(g, z)
+		return Bound(g, z), nil
 	}
 
 	panic(fmt.Sprintf("geometry type not supported: %T", g))
@@ -75,11 +75,15 @@ func Bound(b orb.Bound, z maptile.Zoom) maptile.Set {
 
 // Collection returns the covering set of tiles for the
 // geoemtry collection.
-func Collection(c orb.Collection, z maptile.Zoom) maptile.Set {
+func Collection(c orb.Collection, z maptile.Zoom) (maptile.Set, error) {
 	set := make(maptile.Set)
 	for _, g := range c {
-		set.Merge(Geometry(g, z))
+		s, err := Geometry(g, z)
+		if err != nil {
+			return nil, err
+		}
+		set.Merge(s)
 	}
 
-	return set
+	return set, nil
 }

maptile/tilecover/merge_test.go

@@ -9,19 +9,19 @@ import (
 func TestMergeUp(t *testing.T) {
 	f := loadFeature(t, "./testdata/line.geojson")
 
-	tiles := Geometry(f.Geometry, 15)
+	tiles, _ := Geometry(f.Geometry, 15)
 	c1 := len(MergeUpPartial(tiles, 1, 1))
 
-	tiles = Geometry(f.Geometry, 15)
+	tiles, _ = Geometry(f.Geometry, 15)
 	c2 := len(MergeUpPartial(tiles, 1, 2))
 
-	tiles = Geometry(f.Geometry, 15)
+	tiles, _ = Geometry(f.Geometry, 15)
 	c3 := len(MergeUpPartial(tiles, 1, 3))
 
-	tiles = Geometry(f.Geometry, 15)
+	tiles, _ = Geometry(f.Geometry, 15)
 	c4 := len(MergeUpPartial(tiles, 1, 4))
 
-	tiles = Geometry(f.Geometry, 15)
+	tiles, _ = Geometry(f.Geometry, 15)
 	c := len(MergeUp(tiles, 1))
 
 	if c1 > c2 {
@@ -43,7 +43,7 @@ func TestMergeUp(t *testing.T) {
 
 func BenchmarkMergeUp_z0z10(b *testing.B) {
 	g := loadFeature(b, "./testdata/russia.geojson").Geometry
-	tiles := Geometry(g, 10)
+	tiles, _ := Geometry(g, 10)
 
 	b.ReportAllocs()
 	b.ResetTimer()
@@ -54,7 +54,7 @@ func BenchmarkMergeUp_z0z10(b *testing.B) {
 
 func BenchmarkMergeUp_z8z9(b *testing.B) {
 	g := loadFeature(b, "./testdata/russia.geojson").Geometry
-	tiles := Geometry(g, 9)
+	tiles, _ := Geometry(g, 9)
 
 	b.ReportAllocs()
 	b.ResetTimer()
@@ -65,7 +65,7 @@ func BenchmarkMergeUp_z8z9(b *testing.B) {
 
 func BenchmarkMergeUpPartial4_z0z10(b *testing.B) {
 	g := loadFeature(b, "./testdata/russia.geojson").Geometry
-	tiles := Geometry(g, 10)
+	tiles, _ := Geometry(g, 10)
 
 	b.ReportAllocs()
 	b.ResetTimer()
@@ -76,7 +76,7 @@ func BenchmarkMergeUpPartial4_z0z10(b *testing.B) {
 
 func BenchmarkMergeUpPartial4_z8z9(b *testing.B) {
 	g := loadFeature(b, "./testdata/russia.geojson").Geometry
-	tiles := Geometry(g, 9)
+	tiles, _ := Geometry(g, 9)
 
 	b.ReportAllocs()
 	b.ResetTimer()

maptile/tilecover/polygon.go

@@ -1,40 +1,52 @@
 package tilecover
 
 import (
+	"errors"
 	"sort"
 
 	"github.com/paulmach/orb"
 	"github.com/paulmach/orb/maptile"
 )
 
+// ErrUnevenIntersections can be returned when clipping polygons
+// and there are issues with the geometries, like the rings are not closed.
+var ErrUnevenIntersections = errors.New("tilecover: uneven intersections, ring not closed?")
+
 // Ring creates a tile cover for the ring.
-func Ring(r orb.Ring, z maptile.Zoom) maptile.Set {
+func Ring(r orb.Ring, z maptile.Zoom) (maptile.Set, error) {
 	if len(r) == 0 {
-		return make(maptile.Set)
+		return make(maptile.Set), nil
 	}
 
 	return Polygon(orb.Polygon{r}, z)
 }
 
 // Polygon creates a tile cover for the polygon.
-func Polygon(p orb.Polygon, z maptile.Zoom) maptile.Set {
+func Polygon(p orb.Polygon, z maptile.Zoom) (maptile.Set, error) {
 	set := make(maptile.Set)
-	polygon(set, p, z)
 
-	return set
+	err := polygon(set, p, z)
+	if err != nil {
+		return nil, err
+	}
+
+	return set, nil
 }
 
 // MultiPolygon creates a tile cover for the multi-polygon.
-func MultiPolygon(mp orb.MultiPolygon, z maptile.Zoom) maptile.Set {
+func MultiPolygon(mp orb.MultiPolygon, z maptile.Zoom) (maptile.Set, error) {
 	set := make(maptile.Set)
 	for _, p := range mp {
-		polygon(set, p, z)
+		err := polygon(set, p, z)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	return set
+	return set, nil
 }
 
-func polygon(set maptile.Set, p orb.Polygon, zoom maptile.Zoom) {
+func polygon(set maptile.Set, p orb.Polygon, zoom maptile.Zoom) error {
 	intersections := make([][2]uint32, 0)
 
 	for _, r := range p {
@@ -47,7 +59,7 @@ func polygon(set maptile.Set, p orb.Polygon, zoom maptile.Zoom) {
 			y := ring[i][1]
 
 			// add interesction if it's not local extremum or duplicate
-			if (y > ring[pi][1] || y > ring[ni][1]) && // not local minimum
+			if (ring[pi][1] < y || ring[ni][1] < y) && // not local minimum
 				(y < ring[pi][1] || y < ring[ni][1]) && // not local maximum
 				y != ring[ni][1] {
 
@@ -57,7 +69,7 @@ func polygon(set maptile.Set, p orb.Polygon, zoom maptile.Zoom) {
 	}
 
 	if len(intersections)%2 != 0 {
-		panic("tilecover: uneven intersections, ring not closed?")
+		return ErrUnevenIntersections
 	}
 
 	// sort by y, then x
@@ -79,4 +91,6 @@ func polygon(set maptile.Set, p orb.Polygon, zoom maptile.Zoom) {
 			set[maptile.New(x, y, zoom)] = true
 		}
 	}
+
+	return nil
 }

maptile/tilecover/polygon_test.go

@@ -0,0 +1,18 @@
+package tilecover
+
+import (
+	"testing"
+
+	"github.com/paulmach/orb"
+)
+
+func TestRing_error(t *testing.T) {
+	// not a closed ring
+	f := loadFeature(t, "./testdata/line.geojson")
+	l := f.Geometry.(orb.LineString)
+
+	_, err := Ring(orb.Ring(l), 25)
+	if err != ErrUnevenIntersections {
+		t.Errorf("incorrect error: %v", err)
+	}
+}