Implement Polygon.hasInconsistentLoopOrientations

s2/polygon.go

@@ -57,6 +57,15 @@ type Polygon struct {
 	// hasHoles tracks if this polygon has at least one hole.
 	hasHoles bool
 
+	// hasInconsistentLoopOrientation is true if PolygonFromOrientedLoops() was
+	// called and the given loops had inconsistent orientations (i.e., it is not
+	// possible to construct a polygon such that the interior is on the left-hand
+	// side of all loops).  We need to remember this error so that it can be
+	// returned later by Validate(), since it is not possible to detect this error
+	// once the polygon has been initialized. This field is not preserved by
+	// Encode/Decode.
+	hasInconsistentLoopOrientations bool
+
 	// numVertices keeps the running total of all of the vertices of the contained loops.
 	numVertices int
 
@@ -180,6 +189,19 @@ func PolygonFromOrientedLoops(loops []*Loop) *Polygon {
 		}
 	}
 
+	// Verify that the original loops had consistent shell/hole orientations.
+	// Each original loop L should have been inverted if and only if it now
+	// represents a hole.
+	for _, l := range p.Loops() {
+		if (containedOrigin[l] != l.ContainsOrigin()) != l.IsHole() {
+			// There is no point in saving the loop index because the error is a
+			// property of the entire set of loops. In general, there is no way to
+			// determine which ones are incorrect.
+			p.hasInconsistentLoopOrientations = true
+			break
+		}
+	}
+
 	return p
 }
 
@@ -468,9 +490,9 @@ func (p *Polygon) Validate() error {
 	// }
 
 	// Check whether initOriented detected inconsistent loop orientations.
-	// if p.hasInconsistentLoopOrientations {
-	// 	return fmt.Errorf("inconsistent loop orientations detected")
-	// }
+	if p.hasInconsistentLoopOrientations {
+		return fmt.Errorf("inconsistent loop orientations detected")
+	}
 
 	// Finally, verify the loop nesting hierarchy.
 	return p.findLoopNestingError()

s2/polygon_test.go

@@ -17,6 +17,7 @@ package s2
 import (
 	"math"
 	"math/rand"
+	"strings"
 	"testing"
 
 	"github.com/golang/geo/s1"
@@ -357,8 +358,11 @@ func checkPolygonInvalid(t *testing.T, label string, loops []*Loop, initOriented
 		f(polygon)
 	}
 
-	if err := polygon.Validate(); err == nil {
+	err := polygon.Validate()
+	if err == nil {
 		t.Errorf("%s: %v.Validate() = %v, want non-nil", label, polygon, err)
+	} else if !strings.Contains(err.Error(), label) {
+		t.Errorf("%s: %v.Validate() = %v, wrong error text", label, polygon, err)
 	}
 }
 
@@ -386,6 +390,15 @@ func TestPolygonIsValidLoopNestingInvalid(t *testing.T) {
 	}
 }
 
+func TestPolygonIsValidInconsistentOrientations(t *testing.T) {
+	const iters = 1000
+
+	for iter := 0; iter < iters; iter++ {
+		loops := generatePolygonConcentricTestLoops(2+randomUniformInt(5), 3)
+		checkPolygonInvalid(t, "inconsistent loop orientations", loops, true, nil)
+	}
+}
+
 // TODO(roberts): Implement remaining validity tests.
 // IsValidTests
 //   TestUnitLength
@@ -396,7 +409,6 @@ func TestPolygonIsValidLoopNestingInvalid(t *testing.T) {
 //   TestFullLoop
 //   TestLoopsCrossing
 //   TestDuplicateEdge
-//   TestInconsistentOrientations
 //   TestLoopDepthNegative
 //   TestLoopNestingInvalid
 //   TestFuzzTest