From 535d912742835d901e4bb73abac6515136a15984 Mon Sep 17 00:00:00 2001
From: karl <karl.bittner@hexaquo.at>
Date: Sat, 28 Nov 2020 23:58:17 +0100
Subject: [PATCH] Likely performance improvement by limiting points

Instead of just deleting points in the triangle, we only check specific ones which are relevant
---
 Line.h          | 12 +++++++++++
 Quickhull.h     | 55 +++++++++++++++++++++++++++++--------------------
 performance.cpp |  2 +-
 3 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/Line.h b/Line.h
index 6038a82..775fce6 100644
--- a/Line.h
+++ b/Line.h
@@ -12,6 +12,8 @@ private:
 public:
     Line(Point from, Point to) : m_from(from), m_to(to), m_to_from_origin(to - from) {}
 
+    Line() = default;
+
     Point from() const
     {
         return m_from;
@@ -22,6 +24,16 @@ public:
         return m_to;
     }
 
+    void set_from(Point from)
+    {
+        m_from = from;
+    }
+
+    void set_to(Point to)
+    {
+        m_to = to;
+    }
+
     // Return true if the given point is to the right of this line.
     // False is also returned if the point is directly on the line.
     bool is_point_right(Point other) const
diff --git a/Quickhull.h b/Quickhull.h
index 0e392cb..ef4b029 100644
--- a/Quickhull.h
+++ b/Quickhull.h
@@ -80,38 +80,49 @@ private:
 
         // Build a triangle with these 3 points
 
-        // The order with which we must pass the points depends on where the new furthest point is
-        // TODO: Is there a nicer way to do this?
-        Point a, b, c;
+        // We need to differentiate based on which side the furthest point is on
+        //  in order to keep the meaning of left/right consistent.
+        
+        Line new_line1, new_line2;
+
         if (line.is_point_right(furthest_point))
         {
-            a = line.from();
-            b = line.to();
-            c = furthest_point;
+            // TOOD: It's probably more efficient to set the fields?
+            new_line1 = Line(line.to(), furthest_point);
+            new_line2 = Line(furthest_point, line.from());
         }
         else
         {
-            a = line.from();
-            b = furthest_point;
-            c = line.to();
+            new_line1 = Line(line.from(), furthest_point);
+            new_line2 = Line(furthest_point, line.to());
         }
-        
-        Triangle triangle(a, b, c);
 
-        // Remove points inside this triangle
-        // TODO: I think we can actually skip this, and instead only
-        //  pass points to the left (?) of the individual line to the
-        //  new get_hull_with_line call. That way the ones inside are
-        //  implicitly ignored.
-        input.remove_if([triangle](Point point)
+        // TODO: Test if this improves performance
+        //Triangle triangle(a, b, c);
+        //input.remove_if([triangle](Point point)
+        //{
+        //    return triangle.is_point_inside(point);
+        //});
+
+        // Get points right of new_line1 and 2
+        std::list<Point> left_of_line1, left_of_line2;
+
+        for (const Point& point : input)
         {
-            return triangle.is_point_inside(point);
-        });
+            if (!new_line1.is_point_right(point))
+            {
+                left_of_line1.emplace_back(point);
+            }
+            // TODO: Can we do else if here, or could we then miss out on points?
+            if (!new_line2.is_point_right(point))
+            {
+                left_of_line2.emplace_back(point);
+            }
+        }
 
         // Recursively call get_hull_with_line for each side of the triangle
         // TODO: We can skip the original one
-        get_hull_with_line(input, output, triangle.l1());
-        get_hull_with_line(input, output, triangle.l2());
-        get_hull_with_line(input, output, triangle.l3());
+        get_hull_with_line(left_of_line1, output, new_line1);
+        get_hull_with_line(left_of_line2, output, new_line2);
     }
 };
\ No newline at end of file
diff --git a/performance.cpp b/performance.cpp
index 4a7cdeb..a813f5e 100644
--- a/performance.cpp
+++ b/performance.cpp
@@ -3,10 +3,10 @@
 int main()
 {
     std::list<Point> points = {
+        Point(1, 1),
         Point(-1, -1),
         Point(-1, 1),
         Point(1, -1),
-        Point(1, 1),
         Point(0.5, 0)  // Should not be in the hull
     };
     std::list<Point> hull;