Change point state to use a singleton to smooth out some weird bugs

clusterview/mode_handlers.py

@@ -5,10 +5,11 @@ from PyQt5.QtCore import QEvent, Qt
 from .exceptions import ExceededWindowBoundsError
 from .mode import Mode
 from .opengl_widget import (get_bb_bottom_right, get_bb_top_left,
-                            set_current_points, set_drawing_event,
-                            set_move_bb_top_left, set_move_bb_bottom_right,
-                            reset_move_bbs, viewport_height, viewport_width)
+                            set_drawing_event, set_move_bb_top_left,
+                            set_move_bb_bottom_right, reset_move_bbs,
+                            viewport_height, viewport_width)
 from .points import PointSet
+from .point_manager import PointManager
 
 class __ClickFlag:
 
@@ -32,9 +33,10 @@ class __ClickFlag:
 # Size of point for drawing
 __POINT_SIZE = 8
 
-# There are a lot of module-global variables being used because of the
-# nature of state management in OpenGL.
-__point_set = PointSet(__POINT_SIZE, viewport_height(), viewport_width())
+# PointManager is a class that is filled with static methods
+# designed for managing state.
+PointManager.point_set = PointSet(__POINT_SIZE, viewport_height(),
+                                  viewport_width())
 
 # Module level flag for left click events (used to detect a left
 # click hold drag)
@@ -55,9 +57,11 @@ def __refresh_point_list(ctx):
     # it using the current __point_set.
     ctx.point_list_widget.clear()
 
-    for p in __point_set.points:
+    for p in PointManager.point_set.points:
         ctx.point_list_widget.addItem("({}, {})".format(p.x, p.y))
 
+    ctx.point_list_widget.update()
+
 def __handle_add_point(ctx, event):
     """
     Event handler for the add point mode.
@@ -69,23 +73,21 @@ def __handle_add_point(ctx, event):
     @param ctx A context handle to the main window.
     @param event The click event.
     """
-    global __point_set
-
     if (event.button() == Qt.LeftButton and
         event.type() == QEvent.MouseButtonPress):
 
         # At this point we can be sure resize_gl has been called
         # at least once, so set the viewport properties of the
         # point set so it knows the canvas bounds.
-        __point_set.viewport_width = viewport_width()
-        __point_set.viewport_height = viewport_height()
+        PointManager.point_set.viewport_width = viewport_width()
+        PointManager.point_set.viewport_height = viewport_height()
 
         # Clear any existing selections
-        __point_set.clear_selection()
+        PointManager.point_set.clear_selection()
 
         try:
             # No attribute at the moment.
-            __point_set.add_point(event.x(), event.y())
+            PointManager.point_set.add_point(event.x(), event.y())
         except ExceededWindowBoundsError:
             # The user tried to place a point whos edges would be
             # on the outside of the window. We will just ignore it.
@@ -95,8 +97,6 @@ def __handle_add_point(ctx, event):
 
         set_drawing_event(event)
 
-        set_current_points(__point_set)
-
         ctx.opengl_widget.update()
         ctx.point_list_widget.update()
 
@@ -112,9 +112,7 @@ def __handle_edit_point(ctx, event):
     #
     # Should move the associated point in the list to the new location if
     # applicable.
-    global __point_set
-
-    __point_set.clear_selection()
+    PointManager.point_set.clear_selection()
 
     # Store old x, y from event
     set_drawing_event(event)
@@ -143,7 +141,7 @@ def ogl_keypress_handler(ctx, event):
                 __mouse_start = None
 
                 __left_click_flag = __ClickFlag.NONE
-                __point_set.clear_selection()
+                PointManager.point_set.clear_selection()
                 reset_move_bbs()
 
         elif ctx.mode is not Mode.OFF:
@@ -162,7 +160,6 @@ def __handle_move_points(ctx, event):
 
     global __left_click_flag
     global __mouse_start
-    global __point_set
 
     set_drawing_event(event)
 
@@ -196,7 +193,7 @@ def __handle_move_points(ctx, event):
         dx = __mouse_start[0] - event.x()
         dy = __mouse_start[1] - event.y()
 
-        for p in __point_set.points:
+        for p in PointManager.point_set.points:
             if p.selected:
                 # Use the deltas to decide what direction to move.
                 # We only want to move in small unit increments.
@@ -239,7 +236,7 @@ def __handle_delete_point(ctx, event):
 
         set_drawing_event(event)
 
-        __point_set.remove_point(event.x(), event.y())
+        PointManager.point_set.remove_point(event.x(), event.y())
 
         __refresh_point_list(ctx)
 

clusterview/opengl_widget.py

@@ -21,7 +21,7 @@ from OpenGL.GL import (glBegin, glClearColor, glColor3f, glEnable,
 
 from .exceptions import handle_exceptions, InvalidModeError, InvalidStateError
 from .mode import Mode
-from .points import PointSet
+from .point_manager import PointManager
 
 class Color(Enum):
     BLUE = 0
@@ -55,10 +55,8 @@ __move_bb_bottom_right = None
 # function local.
 __current_mode = None
 __current_event = None
-__current_points = None
 __current_context = None
 
-
 def set_drawing_context(ctx):
     """
     Sets the drawing context so that drawing functions can properly
@@ -68,26 +66,6 @@ def set_drawing_context(ctx):
 
     __current_context = ctx
 
-def set_current_points(points):
-    """
-    Sets the point state variable that will be passed in to the paint_gl
-    function, and further to the point painter functions in order to
-    render the scene.
-
-    Each time a point is added, removed, or edited this point set must be
-    updated.
-
-    @param points The PointSet representing the current scene.
-    """
-    global __current_points
-
-    if not isinstance(points, PointSet):
-        raise ValueError("set_current_points must recieve a PointSet as its " +
-                         "argument.")
-
-    __current_points = points
-
-
 def set_drawing_mode(mode):
     """
     State management function. It is useful to look at the
@@ -128,7 +106,6 @@ def set_drawing_event(event):
     if event is not None:
         __current_event = event
 
-
 def set_move_bb_top_left(x, y):
     """
     Called to set the move bounding box's top left corner.
@@ -140,7 +117,6 @@ def set_move_bb_top_left(x, y):
 
     __move_bb_top_left = (x, y)
 
-
 def set_move_bb_bottom_right(x, y):
     """
     Called to set the move bounding box's bottom right corner.
@@ -152,15 +128,12 @@ def set_move_bb_bottom_right(x, y):
 
     __move_bb_bottom_right = (x, y)
 
-
 def get_bb_top_left():
     return __move_bb_top_left
 
-
 def get_bb_bottom_right():
     return __move_bb_bottom_right
 
-
 def reset_move_bbs():
     global __move_bb_top_left
     global __move_bb_bottom_right
@@ -185,7 +158,6 @@ def resize_gl(w, h):
     """
     global __WIDTH
     global __HEIGHT
-    global __current_points
 
     __WIDTH = __current_context.width()
     __HEIGHT = __current_context.height()
@@ -208,7 +180,7 @@ def paint_gl():
         return
 
     if (__current_mode in [Mode.ADD, Mode.EDIT, Mode.DELETE] and
-        __current_points is None):
+        PointManager.point_set.empty()):
         return
 
     if __current_mode is Mode.ADD or __current_mode is Mode.DELETE:
@@ -217,7 +189,7 @@ def paint_gl():
         # the point set, which will be redrawn here. This action
         # is the same as adding a point since we just draw what is in
         # the point set.
-        draw_points(__current_points, Color.GREY)
+        draw_points(PointManager.point_set, Color.GREY)
 
     elif __current_mode is Mode.EDIT:
         raise NotImplementedError("Drawing for EDIT not implemented.")
@@ -225,8 +197,8 @@ def paint_gl():
     elif __current_mode is Mode.MOVE:
         # We have to repeatedly draw the points while we are showing the
         # move box.
-        if __current_points is not None:
-            draw_points(__current_points, Color.GREY)
+        if not PointManager.point_set.empty():
+            draw_points(PointManager.point_set, Color.GREY)
 
             draw_selection_box(Color.BLACK)
 
@@ -236,8 +208,7 @@ def paint_gl():
             # Mark points that are selected in the bounding box
             # and draw them using the normal function
             highlight_selection()
-            draw_points(__current_points, Color.GREY)
-
+            draw_points(PointManager.point_set, Color.GREY)
 
 def __clamp_x(x):
     """
@@ -296,7 +267,7 @@ def highlight_selection():
     top_left = get_bb_top_left()
     bottom_right = get_bb_bottom_right()
 
-    for point in __current_points.points:
+    for point in PointManager.point_set.points:
         if box_hit(point.x, point.y, top_left[0], top_left[1],
                    bottom_right[0], bottom_right[1]):
 
@@ -358,7 +329,6 @@ def draw_selection_box(color):
 
     glEnd()
 
-
 def clear_selection():
     """
     A helper designed to be called from the main window
@@ -366,10 +336,8 @@ def clear_selection():
     and mode files. This way you dont have to do something
     before the selection clears.
     """
-    global __current_points
-
-    if __current_points is not None:
-        __current_points.clear_selection()
+    if not PointManager.point_set.empty():
+        PointManager.point_set.clear_selection()
 
 def draw_points(point_set, color):
     """
@@ -393,8 +361,7 @@ def draw_points(point_set, color):
 
 
     glViewport(0, 0, __WIDTH, __HEIGHT)
-    glPointSize(__current_points.point_size)
-
+    glPointSize(PointManager.point_set.point_size)
 
     glBegin(GL_POINTS)
     for point in point_set.points:

clusterview/point_list_widget.py

@@ -0,0 +1,52 @@
+"""
+Similar to the opengl_widget module, this module defines
+helper functions for the point_list_widget. It is named
+the same for convenience. The actual point_list_widget
+is defined in the clusterview_ui.py file.
+"""
+
+from .point_manager import PointManager
+
+
+def __string_point_to_point(str_point):
+    """
+    In the QListWidget points are stored as strings
+    because of the way Qt has list items defined.
+
+    @param str_point The string of the form (x, y) to convert.
+    """
+
+    # 1. Split
+    elems = str_point.split(",")
+
+    # 2. Take elements "(x" and "y)" and remove their first and
+    #    last characters, respectively. Note that for y this
+    #    function expects there to be a space after the comma.
+    x = elems[0][1:]
+    y = elems[1][1:-1]
+
+    return (x, y)
+
+def item_click_handler(ctx, item):
+    """
+    Handles an item becoming clicked in the list.
+
+    This function is designed to be partially applied with the
+    main_window context in order to be able to trigger an opengl_widget
+    refresh.
+
+    @param ctx The context.
+    @param item The clicked item.
+    """
+    point = __string_point_to_point(item.text())
+
+    # TODO: Super slow linear search, should write a find_point function
+    #       on the point_set in order to speed this up since PointSet
+    #       is backed by a set anyway.
+    for p in PointManager.point_set.points:
+        if p.x == point[0] and p.y == point[1]:
+            p.select()
+        else:
+            p.unselect()
+
+    ctx.opengl_widget.update()

clusterview/point_manager.py

@@ -0,0 +1,7 @@
+class PointManager():
+    """
+    A state class that represents the absolute state of the
+    world in regards to points.
+    """
+
+    point_set = None

clusterview/points.py

@@ -211,6 +211,9 @@ class PointSet:
     def viewport_width(self, width):
         self.__viewport_width = width
 
+    def empty(self):
+        return len(self.__points) == 0
+
     def clear_selection(self):
         """
         Handy helper function to clear all selected points.

main_window.py

@@ -12,6 +12,7 @@ from clusterview.mode_handlers import MODE_HANDLER_MAP, ogl_keypress_handler
 from clusterview.opengl_widget import (clear_selection, initialize_gl,
                                        paint_gl, resize_gl,
                                        set_drawing_mode, set_drawing_context)
+from clusterview.point_list_widget import item_click_handler
 from clusterview_ui import Ui_MainWindow
 
 class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
@@ -44,6 +45,11 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
         # into the function so that we can modify it as we please.
         self.opengl_widget.keyPressEvent = partial(ogl_keypress_handler, self)
 
+        # Same story here but this time with the itemClicked event
+        # so that when an element is clicked on in the point list it will
+        # highlight.
+        self.point_list_widget.itemClicked = partial(item_click_handler, self)
+
         #-----------------------------------------------
         # OpenGL Graphics Handlers are set
         # here and defined in clusterview.opengl_widget.

tests/test_point.py

@@ -13,25 +13,25 @@ def test_move_point():
     assert p.x == 5 and p.y == 5
 
 def test_move_point_outside_screen_x_positive():
-    p = Point(1, 2, 8, 100, 100)
+    p = Point(4, 4, 8, 100, 100)
 
-    with pytest.raises(ExceededWindowBoundsError):
+    with pytest.raises(ExceededWindowBoundsError) as exc_info:
         p.move(96, 0)
 
 def test_move_point_outside_screen_y_positive():
-    p = Point(1, 2, 8, 100, 100)
+    p = Point(4, 4, 8, 100, 100)
 
     with pytest.raises(ExceededWindowBoundsError):
         p.move(0, 95)
 
 def test_move_point_outside_screen_x_negative():
-    p = Point(1, 2, 8, 100, 100)
+    p = Point(4, 4, 8, 100, 100)
 
     with pytest.raises(ExceededWindowBoundsError):
-        p.move(-2, 0)
+        p.move(-5, 0)
 
 def test_move_point_outside_screen_y_negative():
-    p = Point(1, 2, 8, 100, 100)
+    p = Point(4, 4, 8, 100, 100)
 
     with pytest.raises(ExceededWindowBoundsError):
-        p.move(0, -3)
+        p.move(0, -5)

tests/test_point_set.py

@@ -3,6 +3,11 @@ import pytest
 from clusterview.points import Attribute, Point, PointSet
 
 
+def test_empty():
+    l = PointSet(3, 100, 100)
+
+    assert l.empty()
+
 def test_add_to_point_set():
     l = PointSet(3, 100, 100)
 
@@ -33,10 +38,10 @@ def test_remove_point_exact_click():
     attribute = Attribute("thing", 1)
 
     l = PointSet(8, 100, 100)
-    l.add_point(1, 2, attrs=[attribute])
+    l.add_point(4, 4, attrs=[attribute])
 
-    p = Point(1, 2, 8, 100, 100)
-    l.remove_point(1, 2)
+    p = Point(4, 4, 8, 100, 100)
+    l.remove_point(4, 4)
 
     points = list(l.points)
 
@@ -56,13 +61,13 @@ def test_remove_point_bounding_box():
     attribute = Attribute("thing", 1)
 
     l = PointSet(8, 100, 100)
-    l.add_point(1, 2, attrs=[attribute])
+    l.add_point(4, 4, attrs=[attribute])
 
-    p = Point(1, 2, 8, 100, 100)
+    p = Point(4, 4, 8, 100, 100)
 
     # The click-point (2, 1) will be inside of our point size 8
     # bounding box.
-    l.remove_point(2, 1)
+    l.remove_point(4, 4)
 
     points = list(l.points)
 
@@ -78,12 +83,12 @@ def test_remove_point_bounding_box():
 def test_attributes_must_be_array_of_attributes():
     with pytest.raises(ValueError):
         l = PointSet(8, 100, 100)
-        l.add_point(1, 2, attrs=[1,2,3,4,5])
+        l.add_point(4, 4, attrs=[1,2,3,4,5])
 
 def test_clear_all_selected_points():
     l = PointSet(8, 100, 100)
-    l.add_point(1, 2)
-    l.add_point(3, 4)
+    l.add_point(4, 4)
+    l.add_point(5, 5)
 
     for p in l.points:
         p.select()