Skip to content

Commit a392f25

Browse files
pvcravenPaul V Cravenclaude
authored
Name the bad point when a draw function gets malformed points (#2972)
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. Before, a bad point raised errors like "'int' object is not iterable", and a point with 1 or 3 numbers silently shifted the rest of the data. The check is one length comparison per call; the slow scan only runs after something went wrong. Points are now flattened with tuple(chain.from_iterable(...)), which is about twice as fast as the generator it replaces, so drawing 1,000 lines or points got faster. Co-authored-by: Paul V Craven <paul.craven@optimizley.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8e34d91 commit a392f25

6 files changed

Lines changed: 150 additions & 40 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page.
7373
- 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.
7474

7575
### Misc Changes
76+
- `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.
7677
- 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.
7778
- 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)).
7879
- Docs fixes:

‎arcade/draw/helpers.py‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,52 @@
11
import array
22
import math
3+
from itertools import chain
34

45
from arcade import gl
56
from arcade.types import Color, Point2, Point2List, RGBOrA255
67
from arcade.window_commands import get_window
78

89

10+
def _bad_point_error(point_list: Point2List) -> ValueError | None:
11+
"""Return an error naming the first point that isn't 2 numbers, if any.
12+
13+
Only called after something went wrong, so it can be slow.
14+
"""
15+
for index, point in enumerate(point_list):
16+
try:
17+
x, y = point
18+
array.array("f", (x, y))
19+
except (TypeError, ValueError, OverflowError):
20+
return ValueError(
21+
f"point_list[{index}] is {point!r}, but each point must be 2 numbers, "
22+
"such as (x, y)"
23+
)
24+
return None
25+
26+
27+
def _flatten_points(point_list: Point2List) -> array.array:
28+
"""Flatten 2D points into an array of floats: x0, y0, x1, y1, ...
29+
30+
Raises a ValueError naming the first point that isn't 2 numbers.
31+
Checking costs one length comparison, so it doesn't slow down drawing.
32+
"""
33+
try:
34+
data = array.array("f", tuple(chain.from_iterable(point_list)))
35+
except (TypeError, ValueError, OverflowError) as error:
36+
raise (_bad_point_error(point_list) or error) from None
37+
# A point with 1 or 3 numbers would shift every number after it
38+
if len(data) != 2 * len(point_list):
39+
raise _bad_point_error(point_list) or ValueError("Each point must be 2 numbers")
40+
return data
41+
42+
43+
def _check_points_after(error: Exception, point_list: Point2List) -> None:
44+
"""Raise a clearer error than ``error`` if a point isn't 2 numbers."""
45+
clearer = _bad_point_error(point_list)
46+
if clearer is not None:
47+
raise clearer from None
48+
49+
950
def get_points_for_thick_line(
1051
start_x: float, start_y: float, end_x: float, end_y: float, line_width: float
1152
) -> tuple[Point2, Point2, Point2, Point2]:
@@ -72,7 +113,7 @@ def _generic_draw_line_strip(
72113

73114
# Translate Python objects into types Arcade's Buffer objects accept
74115
color_array = array.array("B", rgba * num_vertices)
75-
vertex_array = array.array("f", tuple(item for sublist in point_list for item in sublist))
116+
vertex_array = _flatten_points(point_list)
76117
geometry.num_vertices = num_vertices
77118

78119
# Double buffer sizes until they can hold all our data

‎arcade/draw/line.py‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44
from arcade.types import Color, Point2, Point2List, RGBOrA255
55
from arcade.window_commands import get_window
66

7-
from .helpers import _generic_draw_line_strip, get_points_for_thick_line
7+
from .helpers import (
8+
_check_points_after,
9+
_flatten_points,
10+
_generic_draw_line_strip,
11+
get_points_for_thick_line,
12+
)
813

914

1015
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
2631
triangle_point_list: list[Point2] = []
2732
# FIXME: This needs a lot of improvement
2833
last_point = None
29-
for point in point_list:
30-
if last_point is not None:
31-
points = get_points_for_thick_line(
32-
last_point[0], last_point[1], point[0], point[1], line_width
33-
)
34-
reordered_points = points[1], points[0], points[2], points[3]
35-
triangle_point_list.extend(reordered_points)
36-
last_point = point
34+
try:
35+
for point in point_list:
36+
if last_point is not None:
37+
points = get_points_for_thick_line(
38+
last_point[0], last_point[1], point[0], point[1], line_width
39+
)
40+
reordered_points = points[1], points[0], points[2], points[3]
41+
triangle_point_list.extend(reordered_points)
42+
last_point = point
43+
except (TypeError, ValueError, IndexError) as error:
44+
_check_points_after(error, point_list)
45+
raise
3746
_generic_draw_line_strip(triangle_point_list, color, gl.TRIANGLE_STRIP)
3847

3948

@@ -110,7 +119,7 @@ def draw_lines(point_list: Point2List, color: RGBOrA255, line_width: float = 1)
110119
# Validate & normalize to a pass the shader an RGBA float uniform
111120
color_normalized = Color.from_iterable(color).normalized
112121

113-
line_pos_array = array.array("f", (v for point in point_list for v in point))
122+
line_pos_array = _flatten_points(point_list)
114123
num_points = len(point_list)
115124
if num_points == 0:
116125
return

‎arcade/draw/point.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import array
2-
31
from arcade.types import Color, Point2List, RGBOrA255
42
from arcade.types.rect import XYWH
53
from arcade.window_commands import get_window
64

5+
from .helpers import _flatten_points
76
from .rect import draw_rect_filled
87

98

@@ -66,7 +65,7 @@ def draw_points(point_list: Point2List, color: RGBOrA255, size: float = 1.0) ->
6665
num_points = len(point_list)
6766
if num_points == 0:
6867
return
69-
point_array = array.array("f", (v for point in point_list for v in point))
68+
point_array = _flatten_points(point_list)
7069

7170
# Resize buffer
7271
data_size = num_points * 8

‎arcade/draw/polygon.py‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from arcade.earclip import earclip
33
from arcade.types import Point2, Point2List, RGBOrA255
44

5-
from .helpers import _generic_draw_line_strip, get_points_for_thick_line
5+
from .helpers import _check_points_after, _generic_draw_line_strip, get_points_for_thick_line
66

77

88
def draw_polygon_filled(point_list: Point2List, color: RGBOrA255) -> None:
@@ -16,7 +16,11 @@ def draw_polygon_filled(point_list: Point2List, color: RGBOrA255) -> None:
1616
color:
1717
The color, specified in RGB or RGBA format.
1818
"""
19-
triangle_points = earclip(point_list)
19+
try:
20+
triangle_points = earclip(point_list)
21+
except (TypeError, ValueError, IndexError) as error:
22+
_check_points_after(error, point_list)
23+
raise
2024
flattened_list = tuple(i for g in triangle_points for i in g)
2125
_generic_draw_line_strip(flattened_list, color, gl.TRIANGLES)
2226

@@ -35,29 +39,33 @@ def draw_polygon_outline(point_list: Point2List, color: RGBOrA255, line_width: f
3539
line_width:
3640
Width of the line in pixels.
3741
"""
38-
# Convert to modifiable list & close the loop
39-
new_point_list = list(point_list)
40-
new_point_list.append(point_list[0])
41-
42-
# Create a place to store the triangles we'll use to thicken the line
43-
triangle_point_list: list[Point2] = []
44-
45-
# This needs a lot of improvement
46-
last_point = None
47-
for point in new_point_list:
48-
if last_point is not None:
49-
# Calculate triangles, then re-order to link up the quad?
50-
points = get_points_for_thick_line(*last_point, *point, line_width)
51-
reordered_points = points[1], points[0], points[2], points[3]
52-
53-
triangle_point_list.extend(reordered_points)
54-
last_point = point
55-
56-
# Use first two points of new list to close the loop
57-
new_start, new_next = new_point_list[:2]
58-
s_x, s_y = new_start
59-
n_x, n_y = new_next
60-
points = get_points_for_thick_line(s_x, s_y, n_x, n_y, line_width)
61-
triangle_point_list.append(points[1])
42+
try:
43+
# Convert to modifiable list & close the loop
44+
new_point_list = list(point_list)
45+
new_point_list.append(point_list[0])
46+
47+
# Create a place to store the triangles we'll use to thicken the line
48+
triangle_point_list: list[Point2] = []
49+
50+
# This needs a lot of improvement
51+
last_point = None
52+
for point in new_point_list:
53+
if last_point is not None:
54+
# Calculate triangles, then re-order to link up the quad?
55+
points = get_points_for_thick_line(*last_point, *point, line_width)
56+
reordered_points = points[1], points[0], points[2], points[3]
57+
58+
triangle_point_list.extend(reordered_points)
59+
last_point = point
60+
61+
# Use first two points of new list to close the loop
62+
new_start, new_next = new_point_list[:2]
63+
s_x, s_y = new_start
64+
n_x, n_y = new_next
65+
points = get_points_for_thick_line(s_x, s_y, n_x, n_y, line_width)
66+
triangle_point_list.append(points[1])
67+
except (TypeError, ValueError, IndexError) as error:
68+
_check_points_after(error, point_list)
69+
raise
6270

6371
_generic_draw_line_strip(triangle_point_list, color, gl.TRIANGLE_STRIP)
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
"""Draw functions that take a list of points name the first bad point."""
2+
3+
import array
4+
5+
import pytest
6+
from pyglet.math import Vec2
7+
8+
import arcade
9+
from arcade.draw.helpers import _flatten_points
10+
11+
GOOD = [(0, 0), (10, 10), (20, 0), (30, 10)]
12+
13+
14+
def test_flatten_points():
15+
assert _flatten_points(GOOD) == array.array("f", [0, 0, 10, 10, 20, 0, 30, 10])
16+
assert _flatten_points([Vec2(1, 2), (3, 4)]) == array.array("f", [1, 2, 3, 4])
17+
assert _flatten_points([]) == array.array("f")
18+
19+
20+
@pytest.mark.parametrize(
21+
"points, bad_index",
22+
[
23+
# The example from the issue: used to raise "'int' object is not iterable"
24+
([(1, 1), 5, (2, 2)], 1),
25+
# Points with 1 or 3 numbers used to shift every number after them
26+
([(1,), (2, 2), (3, 3)], 0),
27+
([(1, 1), (2, 2, 2), (3, 3)], 1),
28+
([(1, 1), ("a", 2)], 1),
29+
],
30+
)
31+
def test_flatten_points_names_bad_point(points, bad_index):
32+
with pytest.raises(ValueError, match=rf"point_list\[{bad_index}\] is .*2 numbers"):
33+
_flatten_points(points)
34+
35+
36+
@pytest.mark.parametrize(
37+
"draw",
38+
[
39+
lambda points: arcade.draw_lines(points, arcade.color.RED),
40+
lambda points: arcade.draw_points(points, arcade.color.RED),
41+
lambda points: arcade.draw_line_strip(points, arcade.color.RED),
42+
lambda points: arcade.draw_line_strip(points, arcade.color.RED, line_width=3),
43+
lambda points: arcade.draw_polygon_filled(points, arcade.color.RED),
44+
lambda points: arcade.draw_polygon_outline(points, arcade.color.RED, line_width=3),
45+
],
46+
ids=["lines", "points", "line_strip", "thick_line_strip", "polygon_filled", "polygon_outline"],
47+
)
48+
def test_draw_functions_name_bad_point(window, draw):
49+
draw(GOOD)
50+
draw([Vec2(*point) for point in GOOD])
51+
with pytest.raises(ValueError, match=r"point_list\[2\] is 7"):
52+
draw([(0, 0), (10, 10), 7, (30, 10)])

0 commit comments

Comments
 (0)