diff --git a/clusterview.py b/clusterview.py index 3b2d5c0..25f218f 100644 --- a/clusterview.py +++ b/clusterview.py @@ -9,7 +9,6 @@ def main(): app = QApplication(sys.argv) window = MainWindow() - window.show() sys.exit(app.exec_()) diff --git a/clusterview/mode_handlers.py b/clusterview/mode_handlers.py index 8c327b9..6bdb224 100644 --- a/clusterview/mode_handlers.py +++ b/clusterview/mode_handlers.py @@ -2,9 +2,26 @@ from .mode import Mode from .opengl_widget import set_current_points, set_drawing_event from .points import PointSet +# 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_set = PointSet(__POINT_SIZE) + +def __refresh_point_list(ctx): + """ + Refreshes the point list display. + + @param ctx A handle to the window context. + """ + # In order to make some guarantees and avoid duplicate + # data we will clear the point list widget and re-populate + # it using the current __point_set. + ctx.point_list_widget.clear() + + for p in __point_set.points: + ctx.point_list_widget.addItem("({}, {})".format(p.x, p.y)) def __handle_add_point(ctx, event): @@ -23,17 +40,11 @@ def __handle_add_point(ctx, event): # No attribute at the moment. __point_set.add_point(event.x(), event.y()) + __refresh_point_list(ctx) + set_drawing_event(event) set_current_points(__point_set) - # In order to make some guarantees and avoid duplicate - # data we will clear the point list widget and re-populate - # it using the current __point_set. - ctx.point_list_widget.clear() - - for p in __point_set.points: - ctx.point_list_widget.addItem("({}, {})".format(p[0], p[1])) - ctx.opengl_widget.update() ctx.point_list_widget.update() @@ -66,8 +77,13 @@ def __handle_move_points(ctx, event): def __handle_delete_point(ctx, event): set_drawing_event(event) - ctx.update() - # Find the point from event and remove it from the list + + __point_set.remove_point(event.x(), event.y()) + + __refresh_point_list(ctx) + + ctx.opengl_widget.update() + ctx.point_list_widget.update() # Simple dispatcher to make it easy to dispatch the right mode # function when the OpenGL window is clicked. diff --git a/clusterview/opengl_widget.py b/clusterview/opengl_widget.py index 3457860..6ca6e08 100644 --- a/clusterview/opengl_widget.py +++ b/clusterview/opengl_widget.py @@ -30,9 +30,6 @@ COLOR_TO_RGBA = { Color.BLUE: (0, 0.5, 1.0, 0.0) } -# Size of point for drawing -__POINT_SIZE = 8 - # Constants set based on the size of the window. __BOTTOM_LEFT = (0, 0) __WIDTH = None @@ -163,7 +160,12 @@ def paint_gl(): raise InvalidStateError("Points must exist for ADD, EDIT, MOVE, " + "and DELETE") - if __current_mode is Mode.ADD: + if __current_mode is Mode.ADD or __current_mode is Mode.DELETE: + # Note that drawing the points doesn't require a bounding box or + # any special context, so delete just removes the element from + # 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.BLUE) elif __current_mode is Mode.EDIT: raise NotImplementedError("Drawing for EDIT not implemented.") @@ -220,25 +222,15 @@ def draw_points(point_set, color): ct = COLOR_TO_RGBA[color] glViewport(0, 0, __WIDTH, __HEIGHT) - glPointSize(__POINT_SIZE) - glColor4f(ct[0], ct[1], ct[2], ct[3]) + glPointSize(__current_points.point_size) + glBegin(GL_POINTS) + glColor4f(ct[0], ct[1], ct[2], ct[3]) for point in point_set.points: - glVertex3f(__clamp_x(point[0]), - __clamp_y(point[1]), + glVertex3f(__clamp_x(point.x), + __clamp_y(point.y), 0.0) # Z is currently fixed to 0 glEnd() -def delete_point(x, y): - """ - Deletes a point. - - The list deletion happens in the clusterview.mode module. This - function just overwrites the point color with the background. - - @param x The x-coordinate. - @param y The y-coordinate. - """ - raise NotImplementedError("delete_point not implemented.") diff --git a/clusterview/points.py b/clusterview/points.py index 1850ca4..08b92fa 100644 --- a/clusterview/points.py +++ b/clusterview/points.py @@ -1,6 +1,101 @@ +from math import floor + +class Point: + """ + A class representing a point. A point + has a point_size bounding box around + it. + """ + + def __init__(self, x, y, point_size): + """ + Initializes a new point with a point_size bounding box. + + @param point_size The size of the point in pixels. + @param x The x-coordinate. + @param y The y-coordinate. + """ + self.__point_size = point_size + self.__x = x + self.__y = y + + half_point = floor(point_size / 2) + + self.__top_left_corner = (self.__x - half_point, + self.__y + half_point) + + self.__bottom_right_corner = (self.__x + half_point, + self.__y - half_point) + + @property + def x(self): + return self.__x + + + @property + def y(self): + return self.__y + + + @property + def point_size(self): + return self.__point_size + + + def __eq__(self, other): + """ + Override for class equality. + + @param other The other object. + """ + return (self.__x == other.x and + self.__y == other.y and + self.__point_size == other.point_size) + + + def __hash__(self): + """ + Overridden hashing function so it can be used as a dictionary key + for attributes. + """ + return hash((self.__x, self.__y, self.__point_size)) + + + def __repr__(self): + return "POINT".format(self.__x, + self.__y, + self.__point_size) + + + def hit(self, x, y): + """ + Determines if the point was hit inside of it's bounding box. + + The condition for hit is simple - consider the following + bounding box: + ------------- + | | + | (x,y) | + | | + ------------- + + Where the clicked location is in the center. Then the top + left corner is defined as (x - half_point_size, y + half_point_size) + and the bottom corner is (x + half_point_size, y - half_point_size) + + So long as x and y are greater than the top left and less than the + top right it is considered a hit. + + This function is necessary for properly deleting and selecting points. + """ + + return (x >= self.__top_left_corner[0] and + x <= self.__bottom_right_corner[0] and + y <= self.__top_left_corner[1] and + y >= self.__bottom_right_corner[1]) + + class Attribute: - __name = None - __value = None def __init__(self, name, value): """ @@ -12,11 +107,35 @@ class Attribute: class PointSet: """ - Useful point set for storing coordinates and attributes. It is - backed by a set to provide nice convenience functions. + Useful container for points backed by a set to insure point + uniqueness. """ - __points = set() - __attributes = {} + + def __init__(self, point_size): + """ + Initializes a point container with points of size point_size. + + @param point_size The size of the points. + """ + self.__points = set() + self.__attributes = {} + self.__point_size = point_size + + + @property + def points(self): + """ + Getter for points. Returns a generator for + looping. + """ + for point in self.__points: + yield point + + + @property + def point_size(self): + return self.__point_size + def add_point(self, x, y, attrs=[]): """ @@ -32,34 +151,43 @@ class PointSet: raise ValueError("Attributes in add_point must be an " + "attribute array.") - point = (x, y) + point = Point(x, y, self.__point_size) self.__points.add(point) self.__attributes[point] = attrs def remove_point(self, x, y): """ - Removes a point and it's attributes from the point set. - """ - point = (x, y) - self.__points.discard(point) - self.__attributes.pop(point) + Removes a point and it's attributes from the point set based + on a bounding box calculation. + Removing a point is an exercise is determining which points + have been hit, and then pulling them out of the list. - def attributes(self, x, y): - """ - Returns the attribute array for a given point. + If two points have a section overlapping, and the user clicks + the overlapped section, both points will be removed. + + Currently O(n). - @param x The x-coordinate of the point. - @param y The y-coordinate of the point. + @param x The x-coordinate. + @param y The y-coordinate. """ - return self.__attributes[(x, y)] - @property - def points(self): + # Find points that match + matched = set(p for p in self.__points if p.hit(x, y)) + + # In place set difference + self.__points = self.__points - matched + + # Remove associated attributes + for point in matched: + self.__attributes.pop(point) + + + def attributes(self, point): """ - Getter for points. Returns a generator for - looping. + Returns the attribute array for a given point. + + @param point The point to get attributes for.. """ - for point in self.__points: - yield point + return self.__attributes[point] diff --git a/tests/test_point_set.py b/tests/test_point_set.py index ace40f7..a5c31c4 100644 --- a/tests/test_point_set.py +++ b/tests/test_point_set.py @@ -1,38 +1,43 @@ import pytest -from clusterview.points import Attribute, PointSet +from clusterview.points import Attribute, Point, PointSet def test_add_to_point_set(): - l = PointSet() + l = PointSet(3) l.add_point(1, 2) points = list(l.points) + p = Point(1, 2, 3) assert len(points) == 1 - assert points[0] == (1, 2) - assert len(l.attributes(1, 2)) == 0 + assert points[0] == p + assert len(l.attributes(p)) == 0 + def test_add_to_point_set_with_attributes(): attribute = Attribute("thing", 1) - l = PointSet() - l.add_point(1, 2, attrs=[attribute]) + l = PointSet(3) + l.add_point(2, 3, attrs=[attribute]) points = list(l.points) - attrs = l.attributes(1, 2) + point = Point(2, 3, 3) + attrs = l.attributes(point) assert len(points) == 1 - assert points[0] == (1, 2) - assert len(l.attributes(1, 2)) == 1 + assert points[0] == point + assert len(l.attributes(point)) == 1 + -def test_remove_point(): +def test_remove_point_exact_click(): attribute = Attribute("thing", 1) - l = PointSet() + l = PointSet(8) l.add_point(1, 2, attrs=[attribute]) + p = Point(1, 2, 8) l.remove_point(1, 2) points = list(l.points) @@ -44,9 +49,37 @@ def test_remove_point(): # point to raise a KeyError because it no # longer exists in the point -> attribute_list # dictionary. - l.attributes(1, 2) + l.attributes(p) + + +def test_remove_point_bounding_box(): + """ + This test checks the bounding box hit heuristic. + """ + attribute = Attribute("thing", 1) + + l = PointSet(8) + l.add_point(1, 2, attrs=[attribute]) + + p = Point(1, 2, 8) + + # The click-point (2, 1) will be inside of our point size 8 + # bounding box. + l.remove_point(2, 1) + + points = list(l.points) + + assert len(points) == 0 + + with pytest.raises(KeyError): + # We expect a call to attributes on a removed + # point to raise a KeyError because it no + # longer exists in the point -> attribute_list + # dictionary. + l.attributes(p) + def test_attributes_must_be_array_of_attributes(): with pytest.raises(ValueError): - l = PointSet() + l = PointSet(8) l.add_point(1, 2, attrs=[1,2,3,4,5])