Line.h

@@ -12,8 +12,6 @@ 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;
@@ -24,16 +22,6 @@ 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

Makefile

@@ -1,6 +1,5 @@
 CXX = g++
 CXXFLAGS = -Wall -O3 -g
-SFMLFLAGS = -lsfml-system -lsfml-window -lsfml-graphics
 
 # In case you installed SFML to a non-standard path, you'll need to tell the compiler where to find the SFML headers (.hpp files):
 # g++ -c main.cpp -I<sfml-install-path>/include
@@ -10,10 +9,10 @@ SFMLFLAGS = -lsfml-system -lsfml-window -lsfml-graphics
 
 quickhull: main.o Quickhull.h Point.h Line.h Triangle.h Timing.o Display.o
 	# link with sfml libs; -lsfml-network -lsfml-audio currently not needed
-	$(CXX) $(CXXFLAGS) -o quickhull main.o Quickhull.h Point.h Line.h Triangle.h Timing.o Display.o $(SFMLFLAGS)
+	$(CXX) $(CXXFLAGS) -o quickhull main.o Quickhull.h Point.h Line.h Triangle.h Timing.o Display.o -lsfml-system -lsfml-window -lsfml-graphics
 
-performance: performance.cpp Display.o
-	$(CXX) $(CXXFLAGS) -o performance Display.o performance.cpp $(SFMLFLAGS)
+performance: performance.cpp
+	$(CXX) $(CXXFLAGS) -o performance performance.cpp
 
 main.o: main.cpp Timing.h
 	$(CXX) $(CXXFLAGS) -c main.cpp

NumberGenerator.h

@@ -1,72 +0,0 @@
-#include "Point.h"
-
-#include <list>
-#include <cstdlib>  // for srand, rand
-
-class NumberGenerator
-{
-private:
-    static const int OFFSET = 0;
-    static const int WIDTH = 700;
-    static const int HEIGHT = 700;
-
-public:
-    static std::list<Point> get_random_numbers(int valCount)
-    {
-        std::list<Point> points;
-
-        //srand(static_cast <unsigned> (time(0)));
-        srand(static_cast <unsigned> (0)); // fixed seed for testing
-
-        float x = 0;
-        float y = 0;
-        for (int i = 0; i < valCount; ++i)
-        {
-            x = OFFSET + static_cast <float> (rand()) / (static_cast <float> (RAND_MAX / (WIDTH - 2*OFFSET)));
-            y = OFFSET + static_cast <float> (rand()) / (static_cast <float> (RAND_MAX / (HEIGHT - 2*OFFSET)));
-
-            points.push_back(Point(x, y));
-        }
-
-        return points;
-    }
-
-    static std::list<Point> get_rectangle_numbers(int valCount)
-    {
-        std::list<Point> points;
-
-        float diff = (2.f * HEIGHT / valCount);
-
-        float x = 0;
-        float y = 0;
-        for (int i = 0; i < valCount; ++i)
-        {
-            x = (i%2 == 0) ? OFFSET : WIDTH - OFFSET;
-            y = diff/2 + (i/2) * diff;
-
-            points.push_back(Point(x, y));
-        }
-
-        return points;
-    }
-
-    static std::list<Point> get_circle_numbers(int valCount)
-    {
-        std::list<Point> points;
-
-        float rad = HEIGHT / 2; //8.0f;
-        float deg = (float)(2 * 3.14159 / valCount);
-
-        float x = 0;
-        float y = 0;
-        for (int i = 0; i < valCount; ++i)
-        {
-            x = WIDTH/2 + rad * cosf(i * deg);
-            y = HEIGHT/2 + rad * sinf(i * deg);
-
-            points.push_back(Point(x, y));
-        }
-
-        return points;
-    }
-};

Quickhull.h

@@ -10,7 +10,7 @@
 class Quickhull
 {
 public:
-    static void get_hull(std::list<Point> input, std::list<Point> &output) 
+    static void get_hull(std::list<Point> &input, std::list<Point> &output) 
     {
         // Get leftmost and rightmost point
         Point leftmost(INFINITY, 0.0), rightmost(-INFINITY, 0.0);
@@ -63,7 +63,7 @@ private:
 
         // Find the point which is furthest away from the line, add it to the output
         Point furthest_point;
-        float furthest_distance = -1.0;
+        float furthest_distance = 0.0;
 
         for (const Point &point : input)
         {
@@ -75,78 +75,43 @@ private:
             }
         }
 
-        // TODO: e.g. in the case of a rectangle, it's possible for there to be
-        //  multiple closest points (sometimes all at distance 0). How do we handle
-        //  these properly? We definitely need to remove them all from input later;
-        //  do we also need to handle them all further? This hotfix works, but seems
-        //  like an unnecessarily big performance hit for that edge case.
-        // FIXME: This workaround also causes problems with extremely large numbers
-        //  of randomly generated numbers, causing random lines within the data!
-        //  Points inside the hull are added because of random lines.
-        for (const Point &point : input)
-        {
-            float this_distance = line.distance_to(point);
-            // TODO: Both are required, otherwise only one side of the rectangle is
-            //  taken -- why?
-            if (this_distance == furthest_distance || line.distance_to(point) == 0)
-            {
-                output.emplace_back(point);
-            }
-        }
-        input.remove_if([furthest_distance, line](Point point)
-        {
-            return furthest_distance == line.distance_to(point);
-        });
-        // Hotfix end
-
         output.emplace_back(furthest_point);
         input.remove(furthest_point);
 
         // Build a triangle with these 3 points
 
-        // 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;
-
+        // 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;
         if (line.is_point_right(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());
+            a = line.from();
+            b = line.to();
+            c = furthest_point;
         }
         else
         {
-            new_line1 = Line(line.from(), furthest_point);
-            new_line2 = Line(furthest_point, line.to());
+            a = line.from();
+            b = furthest_point;
+            c = line.to();
         }
         
-        // TODO: Test if this improves performance
-        //Triangle triangle(a, b, c);
-        //input.remove_if([triangle](Point point)
-        //{
-        //    return triangle.is_point_inside(point);
-        //});
+        Triangle triangle(a, b, c);
 
-        // Get points right of new_line1 and 2
-        std::list<Point> left_of_line1, left_of_line2;
-
-        for (const Point& point : input)
+        // 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)
         {
-            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);
-            }
-        }
+            return triangle.is_point_inside(point);
+        });
 
         // Recursively call get_hull_with_line for each side of the triangle
         // TODO: We can skip the original one
-        get_hull_with_line(left_of_line1, output, new_line1);
-        get_hull_with_line(left_of_line2, output, new_line2);
+        get_hull_with_line(input, output, triangle.l1());
+        get_hull_with_line(input, output, triangle.l2());
+        get_hull_with_line(input, output, triangle.l3());
     }
 };

performance.cpp

@@ -1,56 +1,22 @@
 #include "Quickhull.h"
-#include "NumberGenerator.h"
-#include "Display.h"
 
 int main()
 {
-    std::list<Point> points = NumberGenerator::get_rectangle_numbers(100);
+    std::list<Point> points = {
+        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;
 
     Quickhull::get_hull(points, hull);
 
-    // create the window
-    sf::RenderWindow window(sf::VideoMode(800, 800), "k-d-tree");
-
-    sf::CircleShape normal_p(2);
-    normal_p.setFillColor(sf::Color(250, 250, 250));
-
-    sf::CircleShape hull_p(2);
-    hull_p.setFillColor(sf::Color(250, 100, 50));
-
-    // run the program as long as the window is open
-    while (window.isOpen())
-    {
-        // check all the window's events that were triggered since the last iteration of the loop
-        sf::Event event;
-        while (window.pollEvent(event))
-        {
-            // "close requested" event: we close the window
-            if (event.type == sf::Event::Closed)
-                window.close();
-        }
-
-        // clear the window with black color
-        window.clear(sf::Color::Black);
-
-        // Draw all points
-        for (const Point &point : points)
-        {
-            normal_p.setPosition(point.x() + 50, point.y() + 50);
-            window.draw(normal_p);
-        }
-
-        // Draw hull points
     for (const Point &point : hull)
     {
-            hull_p.setPosition(point.x() + 50, point.y() + 50);
-            window.draw(hull_p);
+        std::cout << point.x() << ", " << point.y() << std::endl;
     }
 
-        // end the current frame
-        window.display();
-    }
-
-
     return 0;
 }