diff --git a/CHANGELOG.md b/CHANGELOG.md index 00a2e1299..c3c87daa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page. - Added `arcade.sweep_line(start, end, sprite_list)`, which returns a `SweepInfo` for the first sprite a line hits (its `fraction`, `distance`, and surface `normal`), or `None`. It's `sweep_sprite` for a point: for lasers, hitscan weapons, or seeing what's in the way. Added the `sprite_laser_mirrors` example, where a laser bounces off rotating mirrors using the normal. ### Misc Changes +- `draw_lines`, `draw_points`, `draw_line_strip`, `draw_polygon_filled` and `draw_polygon_outline` raise a `ValueError` naming the first point that isn't 2 numbers, such as `point_list[2] is 7, but each point must be 2 numbers, such as (x, y)` ([#2215](https://github.com/pythonarcade/arcade/issues/2215)). Before, a bad point raised a confusing error like `'int' object is not iterable`, and in `draw_lines`, `draw_points` and `draw_line_strip` a point with 1 or 3 numbers silently shifted every number after it. The check costs one length comparison per call, and the points are now converted faster: drawing 1,000 lines or points went from about 137 to 90 µs per call. - The card game tutorial no longer passes `hit_box_algorithm="None"` to `Sprite`, an Arcade 2 argument that `Sprite` silently ignores. Its text no longer says hit box calculation is slow: loading all 52 cards with their default hit boxes takes about 40 ms. - Removed the docs build's workaround for Sphinx not copying changed CSS files (`util/sphinx_static_file_temp_fix.py` and its `.ENABLE_DEVMACHINE_SPHINX_STATIC_FIX` switch). Sphinx fixed it upstream, and the pinned Sphinx 9.1.0 copies changed CSS on incremental builds and with `make.py serve` ([#2266](https://github.com/pythonarcade/arcade/issues/2266)). - Docs fixes: diff --git a/arcade/draw/helpers.py b/arcade/draw/helpers.py index d3d333927..86fdcffbc 100644 --- a/arcade/draw/helpers.py +++ b/arcade/draw/helpers.py @@ -1,11 +1,52 @@ import array import math +from itertools import chain from arcade import gl from arcade.types import Color, Point2, Point2List, RGBOrA255 from arcade.window_commands import get_window +def _bad_point_error(point_list: Point2List) -> ValueError | None: + """Return an error naming the first point that isn't 2 numbers, if any. + + Only called after something went wrong, so it can be slow. + """ + for index, point in enumerate(point_list): + try: + x, y = point + array.array("f", (x, y)) + except (TypeError, ValueError, OverflowError): + return ValueError( + f"point_list[{index}] is {point!r}, but each point must be 2 numbers, " + "such as (x, y)" + ) + return None + + +def _flatten_points(point_list: Point2List) -> array.array: + """Flatten 2D points into an array of floats: x0, y0, x1, y1, ... + + Raises a ValueError naming the first point that isn't 2 numbers. + Checking costs one length comparison, so it doesn't slow down drawing. + """ + try: + data = array.array("f", tuple(chain.from_iterable(point_list))) + except (TypeError, ValueError, OverflowError) as error: + raise (_bad_point_error(point_list) or error) from None + # A point with 1 or 3 numbers would shift every number after it + if len(data) != 2 * len(point_list): + raise _bad_point_error(point_list) or ValueError("Each point must be 2 numbers") + return data + + +def _check_points_after(error: Exception, point_list: Point2List) -> None: + """Raise a clearer error than ``error`` if a point isn't 2 numbers.""" + clearer = _bad_point_error(point_list) + if clearer is not None: + raise clearer from None + + def get_points_for_thick_line( start_x: float, start_y: float, end_x: float, end_y: float, line_width: float ) -> tuple[Point2, Point2, Point2, Point2]: @@ -72,7 +113,7 @@ def _generic_draw_line_strip( # Translate Python objects into types Arcade's Buffer objects accept color_array = array.array("B", rgba * num_vertices) - vertex_array = array.array("f", tuple(item for sublist in point_list for item in sublist)) + vertex_array = _flatten_points(point_list) geometry.num_vertices = num_vertices # Double buffer sizes until they can hold all our data diff --git a/arcade/draw/line.py b/arcade/draw/line.py index 07e3ed545..b7d9647f9 100644 --- a/arcade/draw/line.py +++ b/arcade/draw/line.py @@ -4,7 +4,12 @@ from arcade.types import Color, Point2, Point2List, RGBOrA255 from arcade.window_commands import get_window -from .helpers import _generic_draw_line_strip, get_points_for_thick_line +from .helpers import ( + _check_points_after, + _flatten_points, + _generic_draw_line_strip, + get_points_for_thick_line, +) def draw_line_strip(point_list: Point2List, color: RGBOrA255, line_width: float = 1) -> None: @@ -26,14 +31,18 @@ def draw_line_strip(point_list: Point2List, color: RGBOrA255, line_width: float triangle_point_list: list[Point2] = [] # FIXME: This needs a lot of improvement last_point = None - for point in point_list: - if last_point is not None: - points = get_points_for_thick_line( - last_point[0], last_point[1], point[0], point[1], line_width - ) - reordered_points = points[1], points[0], points[2], points[3] - triangle_point_list.extend(reordered_points) - last_point = point + try: + for point in point_list: + if last_point is not None: + points = get_points_for_thick_line( + last_point[0], last_point[1], point[0], point[1], line_width + ) + reordered_points = points[1], points[0], points[2], points[3] + triangle_point_list.extend(reordered_points) + last_point = point + except (TypeError, ValueError, IndexError) as error: + _check_points_after(error, point_list) + raise _generic_draw_line_strip(triangle_point_list, color, gl.TRIANGLE_STRIP) @@ -110,7 +119,7 @@ def draw_lines(point_list: Point2List, color: RGBOrA255, line_width: float = 1) # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized - line_pos_array = array.array("f", (v for point in point_list for v in point)) + line_pos_array = _flatten_points(point_list) num_points = len(point_list) if num_points == 0: return diff --git a/arcade/draw/point.py b/arcade/draw/point.py index 4faf5e9d1..1c06b1276 100644 --- a/arcade/draw/point.py +++ b/arcade/draw/point.py @@ -1,9 +1,8 @@ -import array - from arcade.types import Color, Point2List, RGBOrA255 from arcade.types.rect import XYWH from arcade.window_commands import get_window +from .helpers import _flatten_points from .rect import draw_rect_filled @@ -66,7 +65,7 @@ def draw_points(point_list: Point2List, color: RGBOrA255, size: float = 1.0) -> num_points = len(point_list) if num_points == 0: return - point_array = array.array("f", (v for point in point_list for v in point)) + point_array = _flatten_points(point_list) # Resize buffer data_size = num_points * 8 diff --git a/arcade/draw/polygon.py b/arcade/draw/polygon.py index 7ea68aa6f..7705b199b 100644 --- a/arcade/draw/polygon.py +++ b/arcade/draw/polygon.py @@ -2,7 +2,7 @@ from arcade.earclip import earclip from arcade.types import Point2, Point2List, RGBOrA255 -from .helpers import _generic_draw_line_strip, get_points_for_thick_line +from .helpers import _check_points_after, _generic_draw_line_strip, get_points_for_thick_line def draw_polygon_filled(point_list: Point2List, color: RGBOrA255) -> None: @@ -16,7 +16,11 @@ def draw_polygon_filled(point_list: Point2List, color: RGBOrA255) -> None: color: The color, specified in RGB or RGBA format. """ - triangle_points = earclip(point_list) + try: + triangle_points = earclip(point_list) + except (TypeError, ValueError, IndexError) as error: + _check_points_after(error, point_list) + raise flattened_list = tuple(i for g in triangle_points for i in g) _generic_draw_line_strip(flattened_list, color, gl.TRIANGLES) @@ -35,29 +39,33 @@ def draw_polygon_outline(point_list: Point2List, color: RGBOrA255, line_width: f line_width: Width of the line in pixels. """ - # Convert to modifiable list & close the loop - new_point_list = list(point_list) - new_point_list.append(point_list[0]) - - # Create a place to store the triangles we'll use to thicken the line - triangle_point_list: list[Point2] = [] - - # This needs a lot of improvement - last_point = None - for point in new_point_list: - if last_point is not None: - # Calculate triangles, then re-order to link up the quad? - points = get_points_for_thick_line(*last_point, *point, line_width) - reordered_points = points[1], points[0], points[2], points[3] - - triangle_point_list.extend(reordered_points) - last_point = point - - # Use first two points of new list to close the loop - new_start, new_next = new_point_list[:2] - s_x, s_y = new_start - n_x, n_y = new_next - points = get_points_for_thick_line(s_x, s_y, n_x, n_y, line_width) - triangle_point_list.append(points[1]) + try: + # Convert to modifiable list & close the loop + new_point_list = list(point_list) + new_point_list.append(point_list[0]) + + # Create a place to store the triangles we'll use to thicken the line + triangle_point_list: list[Point2] = [] + + # This needs a lot of improvement + last_point = None + for point in new_point_list: + if last_point is not None: + # Calculate triangles, then re-order to link up the quad? + points = get_points_for_thick_line(*last_point, *point, line_width) + reordered_points = points[1], points[0], points[2], points[3] + + triangle_point_list.extend(reordered_points) + last_point = point + + # Use first two points of new list to close the loop + new_start, new_next = new_point_list[:2] + s_x, s_y = new_start + n_x, n_y = new_next + points = get_points_for_thick_line(s_x, s_y, n_x, n_y, line_width) + triangle_point_list.append(points[1]) + except (TypeError, ValueError, IndexError) as error: + _check_points_after(error, point_list) + raise _generic_draw_line_strip(triangle_point_list, color, gl.TRIANGLE_STRIP) diff --git a/tests/unit/draw/test_point_validation.py b/tests/unit/draw/test_point_validation.py new file mode 100644 index 000000000..93222a04b --- /dev/null +++ b/tests/unit/draw/test_point_validation.py @@ -0,0 +1,52 @@ +"""Draw functions that take a list of points name the first bad point.""" + +import array + +import pytest +from pyglet.math import Vec2 + +import arcade +from arcade.draw.helpers import _flatten_points + +GOOD = [(0, 0), (10, 10), (20, 0), (30, 10)] + + +def test_flatten_points(): + assert _flatten_points(GOOD) == array.array("f", [0, 0, 10, 10, 20, 0, 30, 10]) + assert _flatten_points([Vec2(1, 2), (3, 4)]) == array.array("f", [1, 2, 3, 4]) + assert _flatten_points([]) == array.array("f") + + +@pytest.mark.parametrize( + "points, bad_index", + [ + # The example from the issue: used to raise "'int' object is not iterable" + ([(1, 1), 5, (2, 2)], 1), + # Points with 1 or 3 numbers used to shift every number after them + ([(1,), (2, 2), (3, 3)], 0), + ([(1, 1), (2, 2, 2), (3, 3)], 1), + ([(1, 1), ("a", 2)], 1), + ], +) +def test_flatten_points_names_bad_point(points, bad_index): + with pytest.raises(ValueError, match=rf"point_list\[{bad_index}\] is .*2 numbers"): + _flatten_points(points) + + +@pytest.mark.parametrize( + "draw", + [ + lambda points: arcade.draw_lines(points, arcade.color.RED), + lambda points: arcade.draw_points(points, arcade.color.RED), + lambda points: arcade.draw_line_strip(points, arcade.color.RED), + lambda points: arcade.draw_line_strip(points, arcade.color.RED, line_width=3), + lambda points: arcade.draw_polygon_filled(points, arcade.color.RED), + lambda points: arcade.draw_polygon_outline(points, arcade.color.RED, line_width=3), + ], + ids=["lines", "points", "line_strip", "thick_line_strip", "polygon_filled", "polygon_outline"], +) +def test_draw_functions_name_bad_point(window, draw): + draw(GOOD) + draw([Vec2(*point) for point in GOOD]) + with pytest.raises(ValueError, match=r"point_list\[2\] is 7"): + draw([(0, 0), (10, 10), 7, (30, 10)])