diff --git a/s2/polygon.go b/s2/polygon.go index c691ec0..52fef7e 100644 --- a/s2/polygon.go +++ b/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() diff --git a/s2/polygon_test.go b/s2/polygon_test.go index cbd85c1..4a1a538 100644 --- a/s2/polygon_test.go +++ b/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