EdgeQuery.initCovering: Fix bug causing skipped faces (#247)

Remove extra `break` causing intermediate faces to be skipped sometimes.

C++ does not have this `break`:
https://github.com/google/s2geometry/blob/98f435222f24cd3865619f2df9e02f630b8d5d53/src/s2/s2closest_edge_query_base.h#L814

Improve test coverage.

Author: benguild

s2/edge_query.go

@@ -697,7 +697,7 @@ func (e *EdgeQuery) initCovering() {
 	if next.CellID() != last.CellID() {
 		// The index has at least two cells. Choose a level such that the entire
 		// index can be spanned with at most 6 cells (if the index spans multiple
-		// faces) or 4 cells (it the index spans a single face).
+		// faces) or 4 cells (if the index spans a single face).
 		level, ok := next.CellID().CommonAncestorLevel(last.CellID())
 		if !ok {
 			level = 0
@@ -720,9 +720,7 @@ func (e *EdgeQuery) initCovering() {
 			cellLast := next.clone()
 			cellLast.Prev()
 			e.addInitialRange(cellFirst, cellLast)
-			break
 		}
-
 	}
 	e.addInitialRange(next, last)
 }

s2/edge_query_test.go

@@ -177,6 +177,102 @@ func TestEdgeQuerySortAndUnique(t *testing.T) {
 	}
 }
 
+func TestEdgeQueryOptimized(t *testing.T) {
+	tests := []struct {
+		name string
+		locs []struct{ latitude, longitude float64 }
+	}{
+		{
+			// 0 intermediate faces: first and last handled directly
+			"faces 0,4",
+			[]struct{ latitude, longitude float64 }{{20, 20}, {40, -100}},
+		},
+
+		{
+			"faces 0,4 (3+1 shapes)",
+			[]struct{ latitude, longitude float64 }{{20, 20}, {25, 25}, {15, 15}, {40, -100}},
+		},
+
+		{
+			// 1 intermediate face with 1 shape each
+			"faces 0,1,4",
+			[]struct{ latitude, longitude float64 }{{20, 20}, {40, 90}, {40, -100}},
+		},
+
+		{
+			// 1 intermediate face but multiple shapes on first face
+			"faces 0,1,4 (3+1+1 shapes)",
+			[]struct{ latitude, longitude float64 }{{20, 20}, {25, 25}, {15, 15}, {40, 90}, {40, -100}},
+		},
+
+		{
+			// 2 intermediate faces
+			"faces 0,1,3,4",
+			[]struct{ latitude, longitude float64 }{{20, 20}, {40, 90}, {0, 170}, {40, -100}},
+		},
+
+		{
+			// All 6 faces: extreme case with 4 intermediate faces
+			"all 6 faces",
+			[]struct{ latitude, longitude float64 }{
+				{20, 20}, {40, 90}, {90, 0}, {0, 170}, {40, -100}, {-90, 0},
+			},
+		},
+
+		{
+			// Non-zero starting face with 1 intermediate
+			"faces 1,3,4",
+			[]struct{ latitude, longitude float64 }{{40, 90}, {0, 170}, {40, -100}},
+		},
+
+		{
+			// Non-zero starting face with 2 intermediate faces
+			"faces 1,2,3,4",
+			[]struct{ latitude, longitude float64 }{{40, 90}, {90, 0}, {0, 170}, {40, -100}},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			index := NewShapeIndex()
+			for _, location := range test.locs {
+				center := PointFromLatLng(LatLngFromDegrees(location.latitude, location.longitude))
+				loop := RegularLoop(center, s1.Degree, 8)
+				index.Add(PolygonFromLoops([]*Loop{loop}))
+			}
+
+			queryPoint := PointFromLatLng(LatLngFromDegrees(0, 10))
+
+			optimized := NewClosestEdgeQueryOptions().IncludeInteriors(true)
+			got := NewClosestEdgeQuery(index, optimized).FindEdges(
+				NewMinDistanceToPointTarget(queryPoint),
+			)
+
+			bruteForce := NewClosestEdgeQueryOptions().IncludeInteriors(true).UseBruteForce(true)
+			want := NewClosestEdgeQuery(index, bruteForce).FindEdges(
+				NewMinDistanceToPointTarget(queryPoint),
+			)
+
+			shapesGot, shapesWant := make(map[int32]bool), make(map[int32]bool)
+			for _, r := range got {
+				shapesGot[r.ShapeID()] = true
+			}
+			for _, r := range want {
+				shapesWant[r.ShapeID()] = true
+			}
+
+			if len(shapesGot) != len(shapesWant) {
+				t.Errorf(
+					"%s: optimized found %d shapes, brute force found %d",
+					test.name,
+					len(shapesGot),
+					len(shapesWant),
+				)
+			}
+		})
+	}
+}
+
 // For various tests and benchmarks on the edge query code, there are a number of
 // ShapeIndex generators that can be used.
 type shapeIndexGeneratorFunc func(c Cap, numEdges int, index *ShapeIndex)