diff --git a/clusterview/exceptions.py b/clusterview/exceptions.py index f37a1bb..8d01eae 100644 --- a/clusterview/exceptions.py +++ b/clusterview/exceptions.py @@ -2,11 +2,12 @@ from PyQt5.QtWidgets import QErrorMessage from .mode import Mode +class ExceededWindowBoundsError(Exception): + pass class InvalidStateError(Exception): pass - class InvalidModeError(Exception): """ An exception to specify an invalid mode has been provided. @@ -26,7 +27,6 @@ class InvalidModeError(Exception): if mode == Mode.OFF: super().__init__("You must select a mode before continuing.") - def handle_exceptions(func): """ A decorator designed to make exceptions thrown diff --git a/clusterview/mode_handlers.py b/clusterview/mode_handlers.py index 6842814..d10132a 100644 --- a/clusterview/mode_handlers.py +++ b/clusterview/mode_handlers.py @@ -6,7 +6,7 @@ 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) + reset_move_bbs, viewport_height, viewport_width) from .points import PointSet class __ClickFlag: @@ -33,7 +33,7 @@ __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) +__point_set = PointSet(__POINT_SIZE, viewport_height(), viewport_width()) # Module level flag for left click events (used to detect a left # click hold drag) @@ -57,7 +57,6 @@ def __refresh_point_list(ctx): for p in __point_set.points: ctx.point_list_widget.addItem("({}, {})".format(p.x, p.y)) - def __handle_add_point(ctx, event): """ Event handler for the add point mode. @@ -89,7 +88,6 @@ def __handle_add_point(ctx, event): ctx.opengl_widget.update() ctx.point_list_widget.update() - def __handle_edit_point(ctx, event): # TODO: This function and delete definitely need to make sure they are # on a point we have. @@ -111,7 +109,6 @@ def __handle_edit_point(ctx, event): ctx.update() # after this remove the point from the list - def ogl_keypress_handler(ctx, event): """ A keypress handler attached to the OpenGL widget. @@ -142,7 +139,6 @@ def ogl_keypress_handler(ctx, event): ctx.opengl_widget.update() - def __handle_move_points(ctx, event): """ A relatively complicated state machine that handles the process of @@ -195,21 +191,21 @@ def __handle_move_points(ctx, event): # If we used the deltas directly the points would # fly off screen quickly as we got farther from our # start. - - # TODO: THIS IS NUTS RIGHT NOW - # WE SHOULD NOT ALLOW ANY POINT TO EXCEED - # THE MAXIMUM SIZE OF THE SCREEN. - - if dx > 0 and dy > 0: - p.move(10, 10) - elif dx > 0 and dy == 0: - p.move(10, 0) - elif dx < 0 and dy == 0: - p.move(-10, 0) - elif dx == 0 and dy > 0: - p.move(0, 10) - elif dx == 0 and dy < 0: - p.move(0, -10) + try: + if dx > 0: + p.move(1, 0) + if dx < 0: + p.move(-1, 0) + if dy > 0: + p.move(0, 1) + if dy < 0: + p.move(0, -1) + + except ExceededWindowBoundsError: + # This point has indicated a move would exceed + # it's bounds, so we'll just go to the next + # point. + continue elif (__left_click_flag is not __ClickFlag.NONE and @@ -222,8 +218,6 @@ def __handle_move_points(ctx, event): # Satisfy the post condition by resetting the bounding box reset_move_bbs() - - ctx.point_list_widget.update() ctx.opengl_widget.update() diff --git a/clusterview/opengl_widget.py b/clusterview/opengl_widget.py index 90d2984..29646a8 100644 --- a/clusterview/opengl_widget.py +++ b/clusterview/opengl_widget.py @@ -191,6 +191,11 @@ def resize_gl(w, h): __WIDTH = __current_context.width() __HEIGHT = __current_context.height() +def viewport_width(): + return __WIDTH + +def viewport_height(): + return __HEIGHT @handle_exceptions def paint_gl(): diff --git a/clusterview/points.py b/clusterview/points.py index b0138f6..9de4675 100644 --- a/clusterview/points.py +++ b/clusterview/points.py @@ -1,5 +1,7 @@ from math import floor +from .exceptions import ExceededWindowBoundsError + class Point: """ A class representing a point. A point @@ -7,20 +9,27 @@ class Point: it. """ - def __init__(self, x, y, point_size): + def __init__(self, x, y, point_size, viewport_width, viewport_height): """ Initializes a new point with a point_size bounding box. - @param point_size The size of the point in pixels. + Initialized with additional viewport data to make sure the + move function refuses to move a point outside the screen. + @param x The x-coordinate. @param y The y-coordinate. + @param point_size The size of the point in pixels. + @param viewport_width The width of the viewport. + @param viewport_height The height of the viewport. """ self.__point_size = point_size self.__x = x self.__y = y self.__selected = False + self.__viewport_width = viewport_width + self.__viewport_height = viewport_height - half_point = floor(point_size / 2) + half_point = floor(point_size / 2.0) self.__top_left_corner = (self.__x - half_point, self.__y + half_point) @@ -51,6 +60,18 @@ class Point: @param dy The delta in the y direction. """ + half_point = floor(self.point_size / 2.0) + + # Screen size in pixels is always positive + # We need to include the half point here because + # the (x, y) for a point is the center of the square and we + # do not want the EDGES to exceed the viewport bounds. + if (self.__x + dx > self.__viewport_width - half_point or + self.__y + dy > self.__viewport_height - half_point or + self.__x + dx < half_point or self.__y + dy < half_point): + + raise ExceededWindowBoundsError + self.__x += dx self.__y += dy @@ -117,7 +138,6 @@ class Point: y <= self.__top_left_corner[1] and y >= self.__bottom_right_corner[1]) - class Attribute: def __init__(self, name, value): @@ -127,22 +147,27 @@ class Attribute: self.__name = name self.__value = value - class PointSet: """ Useful container for points backed by a set to insure point uniqueness. """ - def __init__(self, point_size): + def __init__(self, point_size, viewport_width, viewport_height): """ Initializes a point container with points of size point_size. @param point_size The size of the points. + @param viewport_width The width of the viewport for bounds + calculations. + @param viewport_height The height of the viewport for bounds + calculations. """ self.__points = set() self.__attributes = {} self.__point_size = point_size + self.__viewport_width = viewport_width + self.__viewport_height = viewport_height @property def points(self): @@ -178,7 +203,8 @@ class PointSet: raise ValueError("Attributes in add_point must be an " + "attribute array.") - point = Point(x, y, self.__point_size) + point = Point(x, y, self.__point_size, + self.__viewport_width, self.__viewport_height) self.__points.add(point) self.__attributes[point] = attrs diff --git a/tests/test_point.py b/tests/test_point.py index 351f606..077c226 100644 --- a/tests/test_point.py +++ b/tests/test_point.py @@ -1,7 +1,37 @@ +import pytest + +from clusterview.exceptions import ExceededWindowBoundsError from clusterview.points import Point def test_move_point(): - p = Point(1, 2, 8) + # The minimum starting position is 1/2 point away + # from the edges + p = Point(4, 4, 8, 100, 100) + p.move(1, 1) - assert p.x == 2 and p.y == 3 + assert p.x == 5 and p.y == 5 + +def test_move_point_outside_screen_x_positive(): + p = Point(1, 2, 8, 100, 100) + + with pytest.raises(ExceededWindowBoundsError): + p.move(96, 0) + +def test_move_point_outside_screen_y_positive(): + p = Point(1, 2, 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) + + with pytest.raises(ExceededWindowBoundsError): + p.move(-2, 0) + +def test_move_point_outside_screen_y_negative(): + p = Point(1, 2, 8, 100, 100) + + with pytest.raises(ExceededWindowBoundsError): + p.move(0, -3) diff --git a/tests/test_point_set.py b/tests/test_point_set.py index f337712..90aa2f8 100644 --- a/tests/test_point_set.py +++ b/tests/test_point_set.py @@ -4,13 +4,13 @@ from clusterview.points import Attribute, Point, PointSet def test_add_to_point_set(): - l = PointSet(3) + l = PointSet(3, 100, 100) l.add_point(1, 2) points = list(l.points) - p = Point(1, 2, 3) + p = Point(1, 2, 3, 100, 100) assert len(points) == 1 assert points[0] == p assert len(l.attributes(p)) == 0 @@ -18,11 +18,11 @@ def test_add_to_point_set(): def test_add_to_point_set_with_attributes(): attribute = Attribute("thing", 1) - l = PointSet(3) + l = PointSet(3, 100, 100) l.add_point(2, 3, attrs=[attribute]) points = list(l.points) - point = Point(2, 3, 3) + point = Point(2, 3, 3, 100, 100) attrs = l.attributes(point) assert len(points) == 1 @@ -32,10 +32,10 @@ def test_add_to_point_set_with_attributes(): def test_remove_point_exact_click(): attribute = Attribute("thing", 1) - l = PointSet(8) + l = PointSet(8, 100, 100) l.add_point(1, 2, attrs=[attribute]) - p = Point(1, 2, 8) + p = Point(1, 2, 8, 100, 100) l.remove_point(1, 2) points = list(l.points) @@ -55,10 +55,10 @@ def test_remove_point_bounding_box(): """ attribute = Attribute("thing", 1) - l = PointSet(8) + l = PointSet(8, 100, 100) l.add_point(1, 2, attrs=[attribute]) - p = Point(1, 2, 8) + p = Point(1, 2, 8, 100, 100) # The click-point (2, 1) will be inside of our point size 8 # bounding box. @@ -77,11 +77,11 @@ def test_remove_point_bounding_box(): def test_attributes_must_be_array_of_attributes(): with pytest.raises(ValueError): - l = PointSet(8) + l = PointSet(8, 100, 100) l.add_point(1, 2, attrs=[1,2,3,4,5]) def test_clear_all_selected_points(): - l = PointSet(8) + l = PointSet(8, 100, 100) l.add_point(1, 2) l.add_point(3, 4)