From 8115436b8ad944aa6e9bf875ae67683ccf87a372 Mon Sep 17 00:00:00 2001
From: Kenny Weiss <kenny@kennyweiss.com>
Date: Thu, 25 Nov 2021 12:16:09 -0800
Subject: [PATCH] Slight clean up of `primal::CurvedPolygon::isClosed()`

---
 src/axom/primal/geometry/CurvedPolygon.hpp    | 17 +++----
 .../primal/tests/primal_curved_polygon.cpp    | 48 ++++++++++++-------
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/src/axom/primal/geometry/CurvedPolygon.hpp b/src/axom/primal/geometry/CurvedPolygon.hpp
index 291392b85d..5e53278784 100644
--- a/src/axom/primal/geometry/CurvedPolygon.hpp
+++ b/src/axom/primal/geometry/CurvedPolygon.hpp
@@ -184,22 +184,19 @@ class CurvedPolygon
       return false;
     }
 
-    // foreach edge: check last vertex of current edge against first vertex of next edge
-    for(int i = 1; i < nEdges; ++i)
+    // foreach edge: check last vertex of previous edge against first vertex of current edge
+    for(int cur = 0, prev = nEdges - 1; cur < nEdges; prev = cur++)
     {
-      const auto ord = m_edges[i - 1].getOrder();
-      const auto& lastPrev = m_edges[i - 1][ord];
-      const auto& firstCur = m_edges[i][0];
+      const auto ord = m_edges[prev].getOrder();
+      const auto& lastPrev = m_edges[prev][ord];
+      const auto& firstCur = m_edges[cur][0];
       if(!isNearlyEqual(squared_distance(lastPrev, firstCur), 0., sq_tol))
       {
         return false;
       }
     }
-    // check last edge against first
-    const auto ord = m_edges[nEdges - 1].getOrder();
-    const auto& lastPrev = m_edges[nEdges - 1][ord];
-    const auto& firstCur = m_edges[0][0];
-    return isNearlyEqual(squared_distance(lastPrev, firstCur), 0., sq_tol);
+
+    return true;
   }
 
   /// \brief Reverses orientation of a CurvedPolygon
diff --git a/src/axom/primal/tests/primal_curved_polygon.cpp b/src/axom/primal/tests/primal_curved_polygon.cpp
index 859d05dbba..73af009c8f 100644
--- a/src/axom/primal/tests/primal_curved_polygon.cpp
+++ b/src/axom/primal/tests/primal_curved_polygon.cpp
@@ -158,9 +158,11 @@ TEST(primal_curvedpolygon, isClosed)
 
   SLIC_INFO("Test checking if CurvedPolygon is closed.");
 
-  CurvedPolygonType bPolygon;
-  EXPECT_EQ(0, bPolygon.numEdges());
-  EXPECT_EQ(false, bPolygon.isClosed());
+  {
+    CurvedPolygonType bPolygon;
+    EXPECT_EQ(0, bPolygon.numEdges());
+    EXPECT_FALSE(bPolygon.isClosed());
+  }
 
   std::vector<PointType> CP = {PointType {0.6, 1.2},
                                PointType {0.3, 2.0},
@@ -168,18 +170,28 @@ TEST(primal_curvedpolygon, isClosed)
                                PointType {0.6, 1.2}};
   std::vector<int> orders = {1, 1, 1};
 
-  std::vector<PointType> subCP = {PointType {0.6, 1.2}, PointType {0.3, 2.0}};
-  std::vector<int> suborders = {1};
-  CurvedPolygonType subPolygon = createPolygon(subCP, suborders);
-  EXPECT_EQ(false, subPolygon.isClosed());
+  {
+    std::vector<PointType> subCP = {PointType {0.6, 1.2}, PointType {0.3, 2.0}};
+    std::vector<int> suborders = {1};
+    CurvedPolygonType subPolygon = createPolygon(subCP, suborders);
+    EXPECT_FALSE(subPolygon.isClosed());
+  }
+
+  {
+    CurvedPolygonType bPolygon = createPolygon(CP, orders);
+    EXPECT_EQ(3, bPolygon.numEdges());
+    EXPECT_TRUE(bPolygon.isClosed());
 
-  bPolygon = createPolygon(CP, orders);
+    bPolygon[2][1][0] -= 2e-15;
+    EXPECT_FALSE(bPolygon.isClosed(1e-15));
+  }
 
-  EXPECT_EQ(3, bPolygon.numEdges());
-  EXPECT_EQ(true, bPolygon.isClosed());
+  {
+    CurvedPolygonType bPolygon = createPolygon(CP, orders);
 
-  bPolygon[2][1][0] -= 2e-15;
-  EXPECT_EQ(false, bPolygon.isClosed(1e-15));
+    bPolygon[1][0][0] = 5;
+    EXPECT_FALSE(bPolygon.isClosed(1e-15));
+  }
 }
 
 //----------------------------------------------------------------------------------
@@ -194,7 +206,7 @@ TEST(primal_curvedpolygon, isClosed_BiGon)
 
   CurvedPolygonType bPolygon;
   EXPECT_EQ(0, bPolygon.numEdges());
-  EXPECT_EQ(false, bPolygon.isClosed());
+  EXPECT_FALSE(bPolygon.isClosed());
 
   // Bi-gon defined by a quadratic edge and a straight line
   std::vector<PointType> CP = {PointType {0.8, .25},
@@ -554,11 +566,12 @@ TEST(primal_curvedpolygon, reverseOrientation)
   using SegmentType = primal::Segment<CoordType, DIM>;
   using BezierCurveType = primal::BezierCurve<CoordType, DIM>;
 
-  // Create a set of line segments on the unit circle
+  // Test several n-gons discretizing the unit circle
   const int MAX_SEG = 10;
   const PointType origin;
   for(int nseg = 3; nseg < MAX_SEG; ++nseg)
   {
+    // Create an n-gon with line segments going CCW along the unit circle
     CurvedPolygonType poly(nseg);
     axom::Array<PointType> pts(nseg + 1);
     for(int i = 0; i < nseg; ++i)
@@ -572,8 +585,9 @@ TEST(primal_curvedpolygon, reverseOrientation)
     {
       poly[i] = BezierCurveType(&pts[i], order);
     }
+    EXPECT_TRUE(poly.isClosed());
 
-    // Perform some checks
+    // Perform some checks on the polygon
     for(int i = 0; i < nseg; ++i)
     {
       // check that the end point of each segment is equal to the start of the next
@@ -586,7 +600,7 @@ TEST(primal_curvedpolygon, reverseOrientation)
       EXPECT_EQ(primal::ON_NEGATIVE_SIDE, primal::orientation(origin, seg));
     }
 
-    // Create a polygon of reversed segments;
+    // Create a polygon with reversed orientation
     CurvedPolygonType reversed = poly;
     reversed.reverseOrientation();
 
@@ -609,7 +623,7 @@ TEST(primal_curvedpolygon, reverseOrientation)
       EXPECT_EQ(primal::ON_POSITIVE_SIDE, primal::orientation(origin, seg));
     }
 
-    // Check that reversing twice yields the original;
+    // Check that reversing twice yields the original
     CurvedPolygonType reversedAgain = reversed;
     reversedAgain.reverseOrientation();
     EXPECT_EQ(poly, reversedAgain);