From 69b9bfe3513b2c24c5bd21f78a46d21290f90b80 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 07:25:23 -0400 Subject: [PATCH 1/9] Replace old bloated color validation with Color's from_iterable + normalize --- arcade/draw_commands.py | 58 +++++++++-------------------------------- 1 file changed, 12 insertions(+), 46 deletions(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 5fe0a6c2a0..22739fe335 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -299,15 +299,9 @@ def draw_ellipse_filled(center_x: float, center_y: float, program = ctx.shape_ellipse_filled_unbuffered_program geometry = ctx.shape_ellipse_unbuffered_geometry buffer = ctx.shape_ellipse_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - if len(color) == 3: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, 1.0 - elif len(color) == 4: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, color[3] / 255 # type: ignore - else: - raise ValueError("Invalid color format. Use a 3 or 4 component tuple") - program['color'] = color_normalized + # We need to normalize the color because we are setting it as a float uniform + program['color'] = Color.from_iterable(color).normalized program['shape'] = width / 2, height / 2, tilt_angle program['segments'] = num_segments buffer.orphan() @@ -345,15 +339,9 @@ def draw_ellipse_outline(center_x: float, center_y: float, program = ctx.shape_ellipse_outline_unbuffered_program geometry = ctx.shape_ellipse_outline_unbuffered_geometry buffer = ctx.shape_ellipse_outline_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - if len(color) == 3: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, 1.0 - elif len(color) == 4: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, color[3] / 255 # type: ignore - else: - raise ValueError("Invalid color format. Use a 3 or 4 component tuple") - program['color'] = color_normalized + # We need to normalize the color because we are setting it as a float uniform + program['color'] = Color.from_iterable(color).normalized program['shape'] = width / 2, height / 2, tilt_angle, border_width program['segments'] = num_segments buffer.orphan() @@ -449,16 +437,10 @@ def draw_line(start_x: float, start_y: float, end_x: float, end_y: float, program = ctx.shape_line_program geometry = ctx.shape_line_geometry - # We need to normalize the color because we are setting it as a float uniform - if len(color) == 3: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, 1.0 - elif len(color) == 4: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, color[3] / 255 # type: ignore - else: - raise ValueError("Invalid color format. Use a 3 or 4 component tuple") + # We need to normalize the color because we are setting it as a float uniform + program['color'] = Color.from_iterable(color).normalized program['line_width'] = line_width - program['color'] = color_normalized ctx.shape_line_buffer_pos.orphan() # Allocate new buffer internally ctx.shape_line_buffer_pos.write( data=array.array('f', (start_x, start_y, end_x, end_y))) @@ -484,14 +466,9 @@ def draw_lines(point_list: PointList, program = ctx.shape_line_program geometry = ctx.shape_line_geometry - # We need to normalize the color because we are setting it as a float uniform - if len(color) == 3: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, 1.0 - elif len(color) == 4: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, color[3] / 255 # type: ignore - else: - raise ValueError("Invalid color format. Use a 3 or 4 component tuple") + # We need to normalize the color because we are setting it as a float uniform + color_normalized = Color.from_iterable(color).normalized while len(point_list) * 3 * 4 > ctx.shape_line_buffer_pos.size: ctx.shape_line_buffer_pos.orphan(ctx.shape_line_buffer_pos.size * 2) else: @@ -536,14 +513,9 @@ def draw_points(point_list: PointList, color: RGBA255, size: float = 1): program = ctx.shape_rectangle_filled_unbuffered_program geometry = ctx.shape_rectangle_filled_unbuffered_geometry buffer = ctx.shape_rectangle_filled_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - if len(color) == 3: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, 1.0 - elif len(color) == 4: - color_normalized = color[0] / 255, color[1] / 255, color[2] / 255, color[3] / 255 # type: ignore - else: - raise ValueError("Invalid color format. Use a 3 or 4 component tuple") + # We need to normalize the color because we are setting it as a float uniform + color_normalized = Color.from_iterable(color).normalized # Resize buffer data_size = len(point_list) * 8 # if data_size > buffer.size: @@ -881,15 +853,9 @@ def draw_rectangle_filled(center_x: float, center_y: float, width: float, program = ctx.shape_rectangle_filled_unbuffered_program geometry = ctx.shape_rectangle_filled_unbuffered_geometry buffer = ctx.shape_rectangle_filled_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - if len(color) == 3: - color_normalized = (color[0] / 255, color[1] / 255, color[2] / 255, 1.0) - elif len(color) == 4: - color_normalized = (color[0] / 255, color[1] / 255, color[2] / 255, color[3] / 255) # type: ignore - else: - raise ValueError("Invalid color format. Use a 3 or 4 component tuple") - program['color'] = color_normalized + # We need to normalize the color because we are setting it as a float uniform + program['color'] = Color.from_iterable(color).normalized program['shape'] = width, height, tilt_angle buffer.orphan() buffer.write(data=array.array('f', (center_x, center_y))) From 307f78d97dcde10d4c3a00ef43892c45a7c970c0 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 09:17:23 -0400 Subject: [PATCH 2/9] Improve readability & fail speed in draw_commands * Move validation & unpacking ctx members to locals to top of functions * Use Color's from_iterable + normalize * Use named variables for goal buffer sizes to make buffer expansion clearer * Add comments and attempt to make math more readable --- arcade/draw_commands.py | 99 ++++++++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 35 deletions(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 22739fe335..86e6d0eb31 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -293,15 +293,18 @@ def draw_ellipse_filled(center_x: float, center_y: float, The default value of -1 means arcade will try to calculate a reasonable amount of segments based on the size of the circle. """ + # Fail immediately if we have no window or context window = get_window() ctx = window.ctx - program = ctx.shape_ellipse_filled_unbuffered_program geometry = ctx.shape_ellipse_unbuffered_geometry buffer = ctx.shape_ellipse_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - program['color'] = Color.from_iterable(color).normalized + # Normalize the color because this shader takes a float uniform + color_normalized = Color.from_iterable(color).normalized + + # Pass data to the shader + program['color'] = color_normalized program['shape'] = width / 2, height / 2, tilt_angle program['segments'] = num_segments buffer.orphan() @@ -333,15 +336,18 @@ def draw_ellipse_outline(center_x: float, center_y: float, The default value of -1 means arcade will try to calculate a reasonable amount of segments based on the size of the circle. """ + # Fail immediately if we have no window or context window = get_window() ctx = window.ctx - program = ctx.shape_ellipse_outline_unbuffered_program geometry = ctx.shape_ellipse_outline_unbuffered_geometry buffer = ctx.shape_ellipse_outline_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - program['color'] = Color.from_iterable(color).normalized + # Normalize the color because this shader takes a float uniform + color_normalized = Color.from_iterable(color).normalized + + # Pass data to shader + program['color'] = color_normalized program['shape'] = width / 2, height / 2, tilt_angle, border_width program['segments'] = num_segments buffer.orphan() @@ -368,29 +374,36 @@ def _generic_draw_line_strip(point_list: PointList, :param color: A color, specified as an RGBA tuple or a :py:class:`~arcade.types.Color` instance. """ + # Fail if we don't have a window, context, or right GL abstractions window = get_window() ctx = window.ctx - - c4 = Color.from_iterable(color) - c4e = c4 * len(point_list) - a = array.array('B', c4e) - vertices = array.array('f', tuple(item for sublist in point_list for item in sublist)) - geometry = ctx.generic_draw_line_strip_geometry + vertex_buffer = ctx.generic_draw_line_strip_vbo + color_buffer = ctx.generic_draw_line_strip_color program = ctx.line_vertex_shader - geometry.num_vertices = len(point_list) - # Double buffer sizes if out of space - while len(vertices) * 4 > ctx.generic_draw_line_strip_vbo.size: - ctx.generic_draw_line_strip_vbo.orphan(ctx.generic_draw_line_strip_vbo.size * 2) - ctx.generic_draw_line_strip_color.orphan(ctx.generic_draw_line_strip_color.size * 2) + # Validate and alpha-pad color, then expand to multi-vertex form since + # this shader normalizes internally as if made to draw multicolor lines. + rgba = Color.from_iterable(color) + num_vertices = len(point_list) # Fail if it isn't a sized / sequence object + + # 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)) + geometry.num_vertices = num_vertices + + # Double buffer sizes until they can hold all our data + goal_vertex_buffer_size = len(vertex_array) * 4 + while goal_vertex_buffer_size > vertex_buffer.size: + vertex_buffer.orphan(color_buffer.size * 2) + color_buffer.orphan(color_buffer.size * 2) else: - ctx.generic_draw_line_strip_vbo.orphan() - ctx.generic_draw_line_strip_color.orphan() - - ctx.generic_draw_line_strip_vbo.write(vertices) - ctx.generic_draw_line_strip_color.write(a) + vertex_buffer.orphan() + color_buffer.orphan() + # Write data & render + vertex_buffer.write(vertex_array) + color_buffer.write(color_array) geometry.render(program, mode=mode) @@ -432,18 +445,23 @@ def draw_line(start_x: float, start_y: float, end_x: float, end_y: float, :py:class:`~arcade.types.Color` instance. :param line_width: Width of the line in pixels. """ + # Fail if we don't have a window, context, or right GL abstractions window = get_window() ctx = window.ctx - program = ctx.shape_line_program geometry = ctx.shape_line_geometry + line_pos_buffer = ctx.shape_line_buffer_pos + + # Validate & normalize to a pass the shader an RGBA float uniform + color_normalized = Color.from_iterable(color).normalized - # We need to normalize the color because we are setting it as a float uniform - program['color'] = Color.from_iterable(color).normalized + # Pass data to the shader + program['color'] = color_normalized program['line_width'] = line_width - ctx.shape_line_buffer_pos.orphan() # Allocate new buffer internally - ctx.shape_line_buffer_pos.write( + line_pos_buffer.orphan() # Allocate new buffer internally + line_pos_buffer.write( data=array.array('f', (start_x, start_y, end_x, end_y))) + geometry.render(program, mode=gl.GL_LINES, vertices=2) @@ -461,23 +479,29 @@ def draw_lines(point_list: PointList, :py:class:`~arcade.types.Color` instance. :param line_width: Width of the line in pixels. """ + # Fail if we don't have a window, context, or right GL abstractions window = get_window() ctx = window.ctx - program = ctx.shape_line_program geometry = ctx.shape_line_geometry + line_buffer_pos = ctx.shape_line_buffer_pos - # We need to normalize the color because we are setting it as a float uniform + # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized - while len(point_list) * 3 * 4 > ctx.shape_line_buffer_pos.size: + + # Grow buffer until large enough to hold all our data + goal_buffer_size = len(point_list) * 3 * 4 + while goal_buffer_size > line_buffer_pos.size: ctx.shape_line_buffer_pos.orphan(ctx.shape_line_buffer_pos.size * 2) else: ctx.shape_line_buffer_pos.orphan() + # Pass data to shader program['line_width'] = line_width program['color'] = color_normalized ctx.shape_line_buffer_pos.write( data=array.array('f', tuple(v for point in point_list for v in point))) + geometry.render(program, mode=gl.GL_LINES, vertices=len(point_list)) @@ -507,15 +531,16 @@ def draw_points(point_list: PointList, color: RGBA255, size: float = 1): :py:class:`~arcade.types.Color` instance. :param size: Size of the point in pixels. """ + # Fails immediately if we don't have a window or context window = get_window() ctx = window.ctx - program = ctx.shape_rectangle_filled_unbuffered_program geometry = ctx.shape_rectangle_filled_unbuffered_geometry buffer = ctx.shape_rectangle_filled_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform + # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized + # Resize buffer data_size = len(point_list) * 8 # if data_size > buffer.size: @@ -847,18 +872,22 @@ def draw_rectangle_filled(center_x: float, center_y: float, width: float, :py:class:`tuple` or :py:class`~arcade.types.Color` instance. :param tilt_angle: rotation of the rectangle (clockwise). Defaults to zero. """ + # Fail if we don't have a window, context, or right GL abstractions window = get_window() ctx = window.ctx - program = ctx.shape_rectangle_filled_unbuffered_program geometry = ctx.shape_rectangle_filled_unbuffered_geometry buffer = ctx.shape_rectangle_filled_unbuffered_buffer - # We need to normalize the color because we are setting it as a float uniform - program['color'] = Color.from_iterable(color).normalized + # Validate & normalize to a pass the shader an RGBA float uniform + color_normalized = Color.from_iterable(color).normalized + + # Pass data to the shader + program['color'] = color_normalized program['shape'] = width, height, tilt_angle buffer.orphan() buffer.write(data=array.array('f', (center_x, center_y))) + geometry.render(program, mode=ctx.POINTS, vertices=1) From df7d5a57523914a6ecc6f20a3a4d8592249044bf Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 09:29:13 -0400 Subject: [PATCH 3/9] Finish dedupe of logic / name shortening in draw_lines --- arcade/draw_commands.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 86e6d0eb31..133cbf4257 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -489,20 +489,22 @@ def draw_lines(point_list: PointList, # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized + line_pos_array = array.array('f', tuple(v for point in point_list for v in point)) + num_points = len(point_list) + # Grow buffer until large enough to hold all our data - goal_buffer_size = len(point_list) * 3 * 4 + goal_buffer_size = num_points * 3 * 4 while goal_buffer_size > line_buffer_pos.size: - ctx.shape_line_buffer_pos.orphan(ctx.shape_line_buffer_pos.size * 2) + ctx.shape_line_buffer_pos.orphan(line_buffer_pos.size * 2) else: ctx.shape_line_buffer_pos.orphan() # Pass data to shader program['line_width'] = line_width program['color'] = color_normalized - ctx.shape_line_buffer_pos.write( - data=array.array('f', tuple(v for point in point_list for v in point))) + line_buffer_pos.write(data=line_pos_array) - geometry.render(program, mode=gl.GL_LINES, vertices=len(point_list)) + geometry.render(program, mode=gl.GL_LINES, vertices=num_points) # --- BEGIN POINT FUNCTIONS # # # From 0645b6e3a16d0117d091059bc511fcb7529cdcf8 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 09:30:12 -0400 Subject: [PATCH 4/9] Skip tuple redundant tuple conversion since array.array accepts appropriate iterables --- arcade/draw_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 133cbf4257..97e4e94e0b 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -489,7 +489,7 @@ def draw_lines(point_list: PointList, # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized - line_pos_array = array.array('f', tuple(v for point in point_list for v in point)) + line_pos_array = array.array('f', (v for point in point_list for v in point)) num_points = len(point_list) # Grow buffer until large enough to hold all our data From e5563ed36dd645a611367524d70dc8f2f57c8576 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 09:34:44 -0400 Subject: [PATCH 5/9] Don't re-divide when we already know the size --- arcade/draw_commands.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 97e4e94e0b..6296481ac6 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -542,16 +542,17 @@ def draw_points(point_list: PointList, color: RGBA255, size: float = 1): # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized + num_points = len(point_list) # Resize buffer - data_size = len(point_list) * 8 + data_size = num_points * 8 # if data_size > buffer.size: buffer.orphan(size=data_size) program['color'] = color_normalized program['shape'] = size, size, 0 buffer.write(data=array.array('f', tuple(v for point in point_list for v in point))) - geometry.render(program, mode=ctx.POINTS, vertices=data_size // 8) + geometry.render(program, mode=ctx.POINTS, vertices=num_points) # --- END POINT FUNCTIONS # # # From 0bb4c7199d65eda0d2b7902a39d7389c2edb336c Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 09:37:33 -0400 Subject: [PATCH 6/9] Explain safety // 8 --- arcade/draw_commands.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 6296481ac6..e17fae7c7f 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -552,7 +552,9 @@ def draw_points(point_list: PointList, color: RGBA255, size: float = 1): program['color'] = color_normalized program['shape'] = size, size, 0 buffer.write(data=array.array('f', tuple(v for point in point_list for v in point))) - geometry.render(program, mode=ctx.POINTS, vertices=num_points) + + # Only render the # of points we have complete data for + geometry.render(program, mode=ctx.POINTS, vertices=data_size // 8) # --- END POINT FUNCTIONS # # # From 3adb4b0d47b4e971f421f0241220cb831a5bd7e6 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 09:47:00 -0400 Subject: [PATCH 7/9] Separate steps better in draw_points * Copy to ctypes array earlier * Skip tuple conversion for array.array since it takes generators --- arcade/draw_commands.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index e17fae7c7f..713a2ece8e 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -542,16 +542,20 @@ def draw_points(point_list: PointList, color: RGBA255, size: float = 1): # Validate & normalize to a pass the shader an RGBA float uniform color_normalized = Color.from_iterable(color).normalized + + # Get # of points and translate Python tuples to a C-style array num_points = len(point_list) + point_array = array.array('f', (v for point in point_list for v in point)) # Resize buffer data_size = num_points * 8 # if data_size > buffer.size: buffer.orphan(size=data_size) + # Pass data to shader program['color'] = color_normalized program['shape'] = size, size, 0 - buffer.write(data=array.array('f', tuple(v for point in point_list for v in point))) + buffer.write(data=point_array) # Only render the # of points we have complete data for geometry.render(program, mode=ctx.POINTS, vertices=data_size // 8) From cb18814fa113f0c588550cf5dbec9b95cf12eceb Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 10:01:21 -0400 Subject: [PATCH 8/9] Attempt to make draw_polygon_outline's current logic more readable --- arcade/draw_commands.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index 713a2ece8e..a227d204d0 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -591,22 +591,29 @@ def draw_polygon_outline(point_list: PointList, :py:class:`~arcade.types.Color` instance. :param 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 = [] + # This needs a lot of improvement last_point = None for point in new_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) + # 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 - points = get_points_for_thick_line(new_point_list[0][0], new_point_list[0][1], new_point_list[1][0], - new_point_list[1][1], line_width) + # Use first two points of new list to close the loop + new_start, new_next = new_point_list[:2] + points = get_points_for_thick_line(*new_start, *new_next, line_width) triangle_point_list.append(points[1]) + _generic_draw_line_strip(triangle_point_list, color, gl.GL_TRIANGLE_STRIP) From 6ba98d12d3ad76467ea3d774e3b8b25f6a5fa21b Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 20 Apr 2024 10:51:06 -0400 Subject: [PATCH 9/9] Sequence.extend doesn't exist, so annotate local variable as List[Point] --- arcade/draw_commands.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arcade/draw_commands.py b/arcade/draw_commands.py index a227d204d0..852676e86b 100644 --- a/arcade/draw_commands.py +++ b/arcade/draw_commands.py @@ -10,7 +10,7 @@ import array import math -from typing import Optional, Tuple +from typing import Optional, Tuple, List import PIL.Image import PIL.ImageOps @@ -18,7 +18,7 @@ import pyglet.gl as gl -from arcade.types import Color, RGBA255, PointList +from arcade.types import Color, RGBA255, PointList, Point from arcade.earclip import earclip from .math import rotate_point from arcade import ( @@ -420,7 +420,7 @@ def draw_line_strip(point_list: PointList, if line_width == 1: _generic_draw_line_strip(point_list, color, gl.GL_LINE_STRIP) else: - triangle_point_list: PointList = [] + triangle_point_list: List[Point] = [] # This needs a lot of improvement last_point = None for point in point_list: