From 5aca58727876bab658b8e9688d952024731c56b3 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 11 Apr 2024 22:33:22 -0400 Subject: [PATCH 01/16] Raise ZeroDivisionError immediately in Camera2D.from_raw_data when projection bounds are equal --- arcade/camera/camera_2d.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 143cec70ce..79f272bf3d 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -123,6 +123,18 @@ def from_raw_data( half_width = width / 2 half_height = height / 2 + # Unpack projection, but only validate when it's given directly + left, right, bottom, top = projection or (-half_width, half_width, -half_height, half_height) + if projection: + if left == right: + raise ZeroDivisionError(( + f"projection width is 0 due to equal {left=}" + f"and {right=} values")) + if bottom == top: + raise ZeroDivisionError(( + f"projection height is 0 due to equal {bottom=}" + f"and {top=}")) + _pos = position or (half_width, half_height) _data = CameraData( position=(_pos[0], _pos[1], 0.0), @@ -131,7 +143,6 @@ def from_raw_data( zoom=zoom ) - left, right, bottom, top = projection or (-half_width, half_width, -half_height, half_height) _projection: OrthographicProjectionData = OrthographicProjectionData( left=left, right=right, top=top, bottom=bottom, From b3302a5e35b3fffe30449508151e5b36af7663bf Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 11 Apr 2024 23:32:33 -0400 Subject: [PATCH 02/16] Add & use ZeroProjectionDimension exception type * Add ZeroProjectionDimension type to arcade.camera.data_types * Use it in the projection validation in from_raw_data --- arcade/camera/camera_2d.py | 11 ++++++++--- arcade/camera/data_types.py | 17 ++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 79f272bf3d..b7e40ebc0f 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -5,7 +5,12 @@ from typing_extensions import Self from arcade.camera.orthographic import OrthographicProjector -from arcade.camera.data_types import CameraData, OrthographicProjectionData, Projector +from arcade.camera.data_types import ( + CameraData, + OrthographicProjectionData, + Projector, + ZeroProjectionDimension +) from arcade.gl import Framebuffer from arcade.window_commands import get_window @@ -127,11 +132,11 @@ def from_raw_data( left, right, bottom, top = projection or (-half_width, half_width, -half_height, half_height) if projection: if left == right: - raise ZeroDivisionError(( + raise ZeroProjectionDimension(( f"projection width is 0 due to equal {left=}" f"and {right=} values")) if bottom == top: - raise ZeroDivisionError(( + raise ZeroProjectionDimension(( f"projection height is 0 due to equal {bottom=}" f"and {top=}")) diff --git a/arcade/camera/data_types.py b/arcade/camera/data_types.py index c4cc38e831..c6a8b785ef 100644 --- a/arcade/camera/data_types.py +++ b/arcade/camera/data_types.py @@ -16,10 +16,25 @@ 'PerspectiveProjectionData', 'Projection', 'Projector', - 'Camera' + 'Camera', + 'ZeroProjectionDimension', ] +class ZeroProjectionDimension(ZeroDivisionError): + """A projection's dimensions were zero along at least one axis. + + This usually happens because code tried to set one of the following: + + * ``left`` equal to ``right`` + * ``bottom`` equal to ``top`` + + You can handle this error as a :py:class:`ZeroDivisionError`. + """ + ... + + + class CameraData: """Stores position, orientation, and zoom for a camera. From 981faee9a3a0e9a6814d15e34b15f773c43f9cba Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 11 Apr 2024 23:35:24 -0400 Subject: [PATCH 03/16] Add & expand Camera2D inheritance and validation tests * Add new test for Camera2D.from_raw_data projection validation * Expand inheritance safety tests for Camera2D classmethods with parametrization --- tests/unit/camera/test_camera2d.py | 47 ++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/tests/unit/camera/test_camera2d.py b/tests/unit/camera/test_camera2d.py index 404c98055a..574ebccf93 100644 --- a/tests/unit/camera/test_camera2d.py +++ b/tests/unit/camera/test_camera2d.py @@ -1,15 +1,52 @@ +from typing import Tuple + import pytest as pytest from arcade import Window from arcade.camera import Camera2D +from arcade.camera.data_types import ZeroProjectionDimension + + +class Camera2DSub1(Camera2D): + ... + +class Camera2DSub2(Camera2DSub1): + ... -def test_camera2d_from_raw_data_inheritance_safety(window: Window): - class MyCamera2D(Camera2D): - ... +CAMERA2D_SUBS = [Camera2DSub1, Camera2DSub2] +ALL_CAMERA2D_TYPES = [Camera2D] + CAMERA2D_SUBS + + +AT_LEAST_ONE_EQUAL_VIEWPORT_DIMENSION = [ + (-100., -100., -1., 1.), + (100., 100., -1., 1.), + (0., 0., 1., 2.), + (0., 1., -100., -100.), + (-1., 1., 0., 0.), + (1., 2., 100., 100.), + (5., 5., 5., 5.) +] + - subclassed = MyCamera2D.from_raw_data(zoom=10.0) - assert isinstance(subclassed, MyCamera2D) +@pytest.mark.parametrize("bad_projection", AT_LEAST_ONE_EQUAL_VIEWPORT_DIMENSION) +@pytest.mark.parametrize("camera_class", ALL_CAMERA2D_TYPES) +def test_camera2d_from_raw_data_bound_validation( + window: Window, + bad_projection: Tuple[float, float, float, float], # Clarify type for PyCharm + camera_class +): + with pytest.raises(ZeroProjectionDimension): + camera_class.from_raw_data(projection=bad_projection) + + +@pytest.mark.parametrize("camera_class", CAMERA2D_SUBS) +def test_camera2d_from_raw_data_inheritance_safety( + window: Window, + camera_class +): + subclassed = camera_class.from_raw_data(zoom=10.0) + assert isinstance(subclassed, Camera2DSub1) RENDER_TARGET_SIZES = [ From 310c6def89a7401b9f06492e4eef706d8bd5bb2d Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 11 Apr 2024 23:51:10 -0400 Subject: [PATCH 04/16] Make ZeroProjectionDimension a ValueError since no math has been done yet --- arcade/camera/data_types.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arcade/camera/data_types.py b/arcade/camera/data_types.py index c6a8b785ef..d632ead067 100644 --- a/arcade/camera/data_types.py +++ b/arcade/camera/data_types.py @@ -21,7 +21,7 @@ ] -class ZeroProjectionDimension(ZeroDivisionError): +class ZeroProjectionDimension(ValueError): """A projection's dimensions were zero along at least one axis. This usually happens because code tried to set one of the following: @@ -29,7 +29,7 @@ class ZeroProjectionDimension(ZeroDivisionError): * ``left`` equal to ``right`` * ``bottom`` equal to ``top`` - You can handle this error as a :py:class:`ZeroDivisionError`. + You can handle this error as a :py:class:`ValueError`. """ ... From f7f2605079af3b18159f64c2f9d62fcfedcf8430 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 00:25:48 -0400 Subject: [PATCH 05/16] Add validation logic to Camera2D.projection + doc * Inline the validation logic to go fast * Update the docstring to be explicitly clear about this behavior --- arcade/camera/camera_2d.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index b7e40ebc0f..00b1924847 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -263,18 +263,44 @@ def top(self, _top: float) -> None: @property def projection(self) -> Tuple[float, float, float, float]: - """The camera's left, right, bottom, top projection values. + """Get/set the left, right, bottom, and top projection values. - These control how the camera projects the world onto the pixels - of the screen. + These are world space values which control how the camera + projects the world onto the pixel space of the current + :py:attr:`.viewport` area. + + .. warning:: The axis values cannot be equal! + + * ``left`` cannot equal ``right`` + * ``bottom`` cannot equal ``top`` + + This property raises a :py:class:`~arcade.camera.data_types.ZeroProjectionDimension` + exception if any axis pairs are equal. You can handle this + exception as a :py:class:`ValueError`. """ _p = self._projection return _p.left, _p.right, _p.bottom, _p.top @projection.setter def projection(self, value: Tuple[float, float, float, float]) -> None: + + # Unpack and validate + left, right, bottom, top = value + if left == right: + raise ZeroProjectionDimension(( + f"projection width is 0 due to equal {left=}" + f"and {right=} values")) + if bottom == top: + raise ZeroProjectionDimension(( + f"projection height is 0 due to equal {bottom=}" + f"and {top=}")) + + # Modify the projection data itself. _p = self._projection - _p.left, _p.right, _p.bottom, _p.top = value + _p.left = left + _p.right = right + _p.bottom = bottom + _p.top = top @property def projection_width(self) -> float: From 10ab2743854a94ec4bf15c30c2bd358647922aef Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 01:38:32 -0400 Subject: [PATCH 06/16] Rename Camera2D._projection to Camera2D._projection_data per Discord discussion --- arcade/camera/camera_2d.py | 112 ++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 00b1924847..a206dcbef2 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -77,7 +77,7 @@ def __init__(self, *, forward=(0.0, 0.0, -1.0), zoom=1.0 ) - self._projection: OrthographicProjectionData = projection_data or OrthographicProjectionData( + self._projection_data: OrthographicProjectionData = projection_data or OrthographicProjectionData( left=-half_width, right=half_width, bottom=-half_height, top=half_height, near=0.0, far=100.0, @@ -87,7 +87,7 @@ def __init__(self, *, self._ortho_projector: OrthographicProjector = OrthographicProjector( window=self._window, view=self._data, - projection=self._projection + projection=self._projection_data ) @classmethod @@ -194,7 +194,7 @@ def projection_data(self) -> OrthographicProjectionData: most use cases will only change the projection on screen resize. """ - return self._projection + return self._projection_data @property def position(self) -> Tuple[float, float]: @@ -211,11 +211,11 @@ def left(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[0] + self._projection.left/self._data.zoom + return self._data.position[0] + self._projection_data.left/self._data.zoom @left.setter def left(self, _left: float) -> None: - self._data.position = (_left - self._projection.left/self._data.zoom,) + self._data.position[1:] + self._data.position = (_left - self._projection_data.left / self._data.zoom,) + self._data.position[1:] @property def right(self) -> float: @@ -223,11 +223,11 @@ def right(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[0] + self._projection.right/self._data.zoom + return self._data.position[0] + self._projection_data.right/self._data.zoom @right.setter def right(self, _right: float) -> None: - self._data.position = (_right - self._projection.right/self._data.zoom,) + self._data.position[1:] + self._data.position = (_right - self._projection_data.right / self._data.zoom,) + self._data.position[1:] @property def bottom(self) -> float: @@ -235,13 +235,13 @@ def bottom(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[1] + self._projection.bottom/self._data.zoom + return self._data.position[1] + self._projection_data.bottom/self._data.zoom @bottom.setter def bottom(self, _bottom: float) -> None: self._data.position = ( self._data.position[0], - _bottom - self._projection.bottom/self._data.zoom, + _bottom - self._projection_data.bottom / self._data.zoom, self._data.position[2] ) @@ -251,13 +251,13 @@ def top(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[1] + self._projection.top/self._data.zoom + return self._data.position[1] + self._projection_data.top/self._data.zoom @top.setter def top(self, _top: float) -> None: self._data.position = ( self._data.position[0], - _top - self._projection.top/self._data.zoom, + _top - self._projection_data.top / self._data.zoom, self._data.position[2] ) @@ -278,7 +278,7 @@ def projection(self) -> Tuple[float, float, float, float]: exception if any axis pairs are equal. You can handle this exception as a :py:class:`ValueError`. """ - _p = self._projection + _p = self._projection_data return _p.left, _p.right, _p.bottom, _p.top @projection.setter @@ -296,7 +296,7 @@ def projection(self, value: Tuple[float, float, float, float]) -> None: f"and {top=}")) # Modify the projection data itself. - _p = self._projection + _p = self._projection_data _p.left = left _p.right = right _p.bottom = bottom @@ -312,7 +312,7 @@ def projection_width(self) -> float: If this isn't what you want, use projection_width_scaled instead. """ - return self._projection.right - self._projection.left + return self._projection_data.right - self._projection_data.left @projection_width.setter def projection_width(self, _width: float) -> None: @@ -333,7 +333,7 @@ def projection_width_scaled(self) -> float: If this isn't what you want, use projection_width instead. """ - return (self._projection.right - self._projection.left) / self._data.zoom + return (self._projection_data.right - self._projection_data.left) / self._data.zoom @projection_width_scaled.setter def projection_width_scaled(self, _width: float) -> None: @@ -354,7 +354,7 @@ def projection_height(self) -> float: If this isn't what you want, use projection_height_scaled instead. """ - return self._projection.top - self._projection.bottom + return self._projection_data.top - self._projection_data.bottom @projection_height.setter def projection_height(self, _height: float) -> None: @@ -375,7 +375,7 @@ def projection_height_scaled(self) -> float: If this isn't what you want, use projection_height instead. """ - return (self._projection.top - self._projection.bottom) / self._data.zoom + return (self._projection_data.top - self._projection_data.bottom) / self._data.zoom @projection_height_scaled.setter def projection_height_scaled(self, _height: float) -> None: @@ -396,11 +396,11 @@ def projection_left(self) -> float: If this isn't what you want, use projection_left_scaled instead. """ - return self._projection.left + return self._projection_data.left @projection_left.setter def projection_left(self, _left: float) -> None: - self._projection.left = _left + self._projection_data.left = _left @property def projection_left_scaled(self) -> float: @@ -412,11 +412,11 @@ def projection_left_scaled(self) -> float: If this isn't what you want, use projection_left instead. """ - return self._projection.left / self._data.zoom + return self._projection_data.left / self._data.zoom @projection_left_scaled.setter def projection_left_scaled(self, _left: float) -> None: - self._projection.left = _left * self._data.zoom + self._projection_data.left = _left * self._data.zoom @property def projection_right(self) -> float: @@ -428,11 +428,11 @@ def projection_right(self) -> float: If this isn't what you want, use projection_right_scaled instead. """ - return self._projection.right + return self._projection_data.right @projection_right.setter def projection_right(self, _right: float) -> None: - self._projection.right = _right + self._projection_data.right = _right @property def projection_right_scaled(self) -> float: @@ -444,11 +444,11 @@ def projection_right_scaled(self) -> float: If this isn't what you want, use projection_right instead. """ - return self._projection.right / self._data.zoom + return self._projection_data.right / self._data.zoom @projection_right_scaled.setter def projection_right_scaled(self, _right: float) -> None: - self._projection.right = _right * self._data.zoom + self._projection_data.right = _right * self._data.zoom @property def projection_bottom(self) -> float: @@ -460,11 +460,11 @@ def projection_bottom(self) -> float: If this isn't what you want, use projection_bottom_scaled instead. """ - return self._projection.bottom + return self._projection_data.bottom @projection_bottom.setter def projection_bottom(self, _bottom: float) -> None: - self._projection.bottom = _bottom + self._projection_data.bottom = _bottom @property def projection_bottom_scaled(self) -> float: @@ -476,11 +476,11 @@ def projection_bottom_scaled(self) -> float: If this isn't what you want, use projection_bottom instead. """ - return self._projection.bottom / self._data.zoom + return self._projection_data.bottom / self._data.zoom @projection_bottom_scaled.setter def projection_bottom_scaled(self, _bottom: float) -> None: - self._projection.bottom = _bottom * self._data.zoom + self._projection_data.bottom = _bottom * self._data.zoom @property def projection_top(self) -> float: @@ -492,11 +492,11 @@ def projection_top(self) -> float: If this isn't what you want, use projection_top_scaled instead. """ - return self._projection.top + return self._projection_data.top @projection_top.setter def projection_top(self, _top: float) -> None: - self._projection.top = _top + self._projection_data.top = _top @property def projection_top_scaled(self) -> float: @@ -508,11 +508,11 @@ def projection_top_scaled(self) -> float: If this isn't what you want, use projection_top instead. """ - return self._projection.top / self._data.zoom + return self._projection_data.top / self._data.zoom @projection_top_scaled.setter def projection_top_scaled(self, _top: float) -> None: - self._projection.top = _top * self._data.zoom + self._projection_data.top = _top * self._data.zoom @property def projection_near(self) -> float: @@ -522,11 +522,11 @@ def projection_near(self) -> float: NOTE this IS NOT scaled by zoom. """ - return self._projection.near + return self._projection_data.near @projection_near.setter def projection_near(self, _near: float) -> None: - self._projection.near = _near + self._projection_data.near = _near @property def projection_far(self) -> float: @@ -536,11 +536,11 @@ def projection_far(self) -> float: NOTE this IS NOT scaled by zoom. """ - return self._projection.far + return self._projection_data.far @projection_far.setter def projection_far(self, _far: float) -> None: - self._projection.far = _far + self._projection_data.far = _far @property def viewport(self) -> Tuple[int, int, int, int]: @@ -550,11 +550,11 @@ def viewport(self) -> Tuple[int, int, int, int]: from the bottom left of ``self.render_target``. They are ordered as ``(left, bottom, width, height)``. """ - return self._projection.viewport + return self._projection_data.viewport @viewport.setter def viewport(self, _viewport: Tuple[int, int, int, int]) -> None: - self._projection.viewport = _viewport + self._projection_data.viewport = _viewport @property def viewport_width(self) -> int: @@ -562,12 +562,12 @@ def viewport_width(self) -> int: The width of the viewport. Defines the number of pixels drawn too horizontally. """ - return self._projection.viewport[2] + return self._projection_data.viewport[2] @viewport_width.setter def viewport_width(self, _width: int) -> None: - self._projection.viewport = (self._projection.viewport[0], self._projection.viewport[1], - _width, self._projection.viewport[3]) + self._projection_data.viewport = (self._projection_data.viewport[0], self._projection_data.viewport[1], + _width, self._projection_data.viewport[3]) @property def viewport_height(self) -> int: @@ -575,30 +575,30 @@ def viewport_height(self) -> int: The height of the viewport. Defines the number of pixels drawn too vertically. """ - return self._projection.viewport[3] + return self._projection_data.viewport[3] @viewport_height.setter def viewport_height(self, _height: int) -> None: - self._projection.viewport = (self._projection.viewport[0], self._projection.viewport[1], - self._projection.viewport[2], _height) + self._projection_data.viewport = (self._projection_data.viewport[0], self._projection_data.viewport[1], + self._projection_data.viewport[2], _height) @property def viewport_left(self) -> int: """ The left most pixel drawn to on the X axis. """ - return self._projection.viewport[0] + return self._projection_data.viewport[0] @viewport_left.setter def viewport_left(self, _left: int) -> None: - self._projection.viewport = (_left,) + self._projection.viewport[1:] + self._projection_data.viewport = (_left,) + self._projection_data.viewport[1:] @property def viewport_right(self) -> int: """ The right most pixel drawn to on the X axis. """ - return self._projection.viewport[0] + self._projection.viewport[2] + return self._projection_data.viewport[0] + self._projection_data.viewport[2] @viewport_right.setter def viewport_right(self, _right: int) -> None: @@ -606,30 +606,30 @@ def viewport_right(self, _right: int) -> None: Set the right most pixel drawn to on the X axis. This moves the position of the viewport, not change the size. """ - self._projection.viewport = (_right - self._projection.viewport[2], self._projection.viewport[1], - self._projection.viewport[2], self._projection.viewport[3]) + self._projection_data.viewport = (_right - self._projection_data.viewport[2], self._projection_data.viewport[1], + self._projection_data.viewport[2], self._projection_data.viewport[3]) @property def viewport_bottom(self) -> int: """ The bottom most pixel drawn to on the Y axis. """ - return self._projection.viewport[1] + return self._projection_data.viewport[1] @viewport_bottom.setter def viewport_bottom(self, _bottom: int) -> None: """ Set the bottom most pixel drawn to on the Y axis. """ - self._projection.viewport = (self._projection.viewport[0], _bottom, - self._projection.viewport[2], self._projection.viewport[3]) + self._projection_data.viewport = (self._projection_data.viewport[0], _bottom, + self._projection_data.viewport[2], self._projection_data.viewport[3]) @property def viewport_top(self) -> int: """ The top most pixel drawn to on the Y axis. """ - return self._projection.viewport[1] + self._projection.viewport[3] + return self._projection_data.viewport[1] + self._projection_data.viewport[3] @viewport_top.setter def viewport_top(self, _top: int) -> None: @@ -637,8 +637,8 @@ def viewport_top(self, _top: int) -> None: Set the top most pixel drawn to on the Y axis. This moves the position of the viewport, not change the size. """ - self._projection.viewport = (self._projection.viewport[0], _top - self._projection.viewport[3], - self._projection.viewport[2], self._projection.viewport[3]) + self._projection_data.viewport = (self._projection_data.viewport[0], _top - self._projection_data.viewport[3], + self._projection_data.viewport[2], self._projection_data.viewport[3]) @property def up(self) -> Tuple[float, float]: From 093332285a402790c9bdf591c237c5ac914dd74e Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 01:41:19 -0400 Subject: [PATCH 07/16] Rename Camera2D._data to Camera2D._camera_data --- arcade/camera/camera_2d.py | 74 +++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index a206dcbef2..5770e85580 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -71,7 +71,7 @@ def __init__(self, *, half_width = width / 2 half_height = height / 2 - self._data = camera_data or CameraData( + self._camera_data = camera_data or CameraData( position=(half_width, half_height, 0.0), up=(0.0, 1.0, 0.0), forward=(0.0, 0.0, -1.0), @@ -86,7 +86,7 @@ def __init__(self, *, self._ortho_projector: OrthographicProjector = OrthographicProjector( window=self._window, - view=self._data, + view=self._camera_data, projection=self._projection_data ) @@ -176,7 +176,7 @@ def view_data(self) -> CameraData: Camera controllers use this property. You will need to access it if you use implement a custom one. """ - return self._data + return self._camera_data @property def projection_data(self) -> OrthographicProjectionData: @@ -199,11 +199,11 @@ def projection_data(self) -> OrthographicProjectionData: @property def position(self) -> Tuple[float, float]: """The 2D world position of the camera along the X and Y axes.""" - return self._data.position[:2] + return self._camera_data.position[:2] @position.setter def position(self, _pos: Tuple[float, float]) -> None: - self._data.position = (_pos[0], _pos[1], self._data.position[2]) + self._camera_data.position = (_pos[0], _pos[1], self._camera_data.position[2]) @property def left(self) -> float: @@ -211,11 +211,11 @@ def left(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[0] + self._projection_data.left/self._data.zoom + return self._camera_data.position[0] + self._projection_data.left/self._camera_data.zoom @left.setter def left(self, _left: float) -> None: - self._data.position = (_left - self._projection_data.left / self._data.zoom,) + self._data.position[1:] + self._camera_data.position = (_left - self._projection_data.left / self._camera_data.zoom,) + self._camera_data.position[1:] @property def right(self) -> float: @@ -223,11 +223,11 @@ def right(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[0] + self._projection_data.right/self._data.zoom + return self._camera_data.position[0] + self._projection_data.right/self._camera_data.zoom @right.setter def right(self, _right: float) -> None: - self._data.position = (_right - self._projection_data.right / self._data.zoom,) + self._data.position[1:] + self._camera_data.position = (_right - self._projection_data.right / self._camera_data.zoom,) + self._camera_data.position[1:] @property def bottom(self) -> float: @@ -235,14 +235,14 @@ def bottom(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[1] + self._projection_data.bottom/self._data.zoom + return self._camera_data.position[1] + self._projection_data.bottom/self._camera_data.zoom @bottom.setter def bottom(self, _bottom: float) -> None: - self._data.position = ( - self._data.position[0], - _bottom - self._projection_data.bottom / self._data.zoom, - self._data.position[2] + self._camera_data.position = ( + self._camera_data.position[0], + _bottom - self._projection_data.bottom / self._camera_data.zoom, + self._camera_data.position[2] ) @property @@ -251,14 +251,14 @@ def top(self) -> float: Useful for checking if a :py:class:`~arcade.Sprite` is on screen. """ - return self._data.position[1] + self._projection_data.top/self._data.zoom + return self._camera_data.position[1] + self._projection_data.top/self._camera_data.zoom @top.setter def top(self, _top: float) -> None: - self._data.position = ( - self._data.position[0], - _top - self._projection_data.top / self._data.zoom, - self._data.position[2] + self._camera_data.position = ( + self._camera_data.position[0], + _top - self._projection_data.top / self._camera_data.zoom, + self._camera_data.position[2] ) @property @@ -333,11 +333,11 @@ def projection_width_scaled(self) -> float: If this isn't what you want, use projection_width instead. """ - return (self._projection_data.right - self._projection_data.left) / self._data.zoom + return (self._projection_data.right - self._projection_data.left) / self._camera_data.zoom @projection_width_scaled.setter def projection_width_scaled(self, _width: float) -> None: - w = self.projection_width * self._data.zoom + w = self.projection_width * self._camera_data.zoom l = self.projection_left / w # Normalised Projection left r = self.projection_right / w # Normalised Projection Right @@ -375,11 +375,11 @@ def projection_height_scaled(self) -> float: If this isn't what you want, use projection_height instead. """ - return (self._projection_data.top - self._projection_data.bottom) / self._data.zoom + return (self._projection_data.top - self._projection_data.bottom) / self._camera_data.zoom @projection_height_scaled.setter def projection_height_scaled(self, _height: float) -> None: - h = self.projection_height * self._data.zoom + h = self.projection_height * self._camera_data.zoom b = self.projection_bottom / h # Normalised Projection Bottom t = self.projection_top / h # Normalised Projection Top @@ -412,11 +412,11 @@ def projection_left_scaled(self) -> float: If this isn't what you want, use projection_left instead. """ - return self._projection_data.left / self._data.zoom + return self._projection_data.left / self._camera_data.zoom @projection_left_scaled.setter def projection_left_scaled(self, _left: float) -> None: - self._projection_data.left = _left * self._data.zoom + self._projection_data.left = _left * self._camera_data.zoom @property def projection_right(self) -> float: @@ -444,11 +444,11 @@ def projection_right_scaled(self) -> float: If this isn't what you want, use projection_right instead. """ - return self._projection_data.right / self._data.zoom + return self._projection_data.right / self._camera_data.zoom @projection_right_scaled.setter def projection_right_scaled(self, _right: float) -> None: - self._projection_data.right = _right * self._data.zoom + self._projection_data.right = _right * self._camera_data.zoom @property def projection_bottom(self) -> float: @@ -476,11 +476,11 @@ def projection_bottom_scaled(self) -> float: If this isn't what you want, use projection_bottom instead. """ - return self._projection_data.bottom / self._data.zoom + return self._projection_data.bottom / self._camera_data.zoom @projection_bottom_scaled.setter def projection_bottom_scaled(self, _bottom: float) -> None: - self._projection_data.bottom = _bottom * self._data.zoom + self._projection_data.bottom = _bottom * self._camera_data.zoom @property def projection_top(self) -> float: @@ -508,11 +508,11 @@ def projection_top_scaled(self) -> float: If this isn't what you want, use projection_top instead. """ - return self._projection_data.top / self._data.zoom + return self._projection_data.top / self._camera_data.zoom @projection_top_scaled.setter def projection_top_scaled(self, _top: float) -> None: - self._projection_data.top = _top * self._data.zoom + self._projection_data.top = _top * self._camera_data.zoom @property def projection_near(self) -> float: @@ -649,7 +649,7 @@ def up(self) -> Tuple[float, float]: The base vector is 3D, but the simplified camera only provides a 2D view. """ - return self._data.up[:2] + return self._camera_data.up[:2] @up.setter def up(self, _up: Tuple[float, float]) -> None: @@ -662,7 +662,7 @@ def up(self, _up: Tuple[float, float]) -> None: NOTE that this is assumed to be normalised. """ - self._data.up = (_up[0], _up[1], 0.0) + self._camera_data.up = (_up[0], _up[1], 0.0) @property def angle(self) -> float: @@ -672,7 +672,7 @@ def angle(self) -> float: clock-wise. """ # Note that this is flipped as we want 0 degrees to be vert. Normally you have y first and then x. - return degrees(atan2(self._data.position[0], self._data.position[1])) + return degrees(atan2(self._camera_data.position[0], self._camera_data.position[1])) @angle.setter def angle(self, value: float) -> None: @@ -683,7 +683,7 @@ def angle(self, value: float) -> None: """ _r = radians(value) # Note that this is flipped as we want 0 degrees to be vert. - self._data.position = (sin(_r), cos(_r), 0.0) + self._camera_data.position = (sin(_r), cos(_r), 0.0) @property def zoom(self) -> float: @@ -696,7 +696,7 @@ def zoom(self) -> float: to be half its original size. This causes sprites to appear 2.0x larger. """ - return self._data.zoom + return self._camera_data.zoom @zoom.setter def zoom(self, _zoom: float) -> None: @@ -709,7 +709,7 @@ def zoom(self, _zoom: float) -> None: to be half its original size. This causes sprites to appear 2.0x larger. """ - self._data.zoom = _zoom + self._camera_data.zoom = _zoom def equalise(self) -> None: """ From cb00dbf4badea8a2df38cb705c59ee379624edb3 Mon Sep 17 00:00:00 2001 From: Aedan Andrews <86714785+DragonMoffon@users.noreply.github.com> Date: Fri, 12 Apr 2024 01:52:35 -0400 Subject: [PATCH 08/16] Bug fix for #2057 --- arcade/camera/camera_2d.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 5770e85580..ce0347474c 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -80,7 +80,7 @@ def __init__(self, *, self._projection_data: OrthographicProjectionData = projection_data or OrthographicProjectionData( left=-half_width, right=half_width, bottom=-half_height, top=half_height, - near=0.0, far=100.0, + near=-100.0, far=100.0, viewport=(0, 0, width, height) ) @@ -98,8 +98,8 @@ def from_raw_data( up: Tuple[float, float] = (0.0, 1.0), zoom: float = 1.0, projection: Optional[Tuple[float, float, float, float]] = None, - near: float = -100, - far: float = 100, + near: float = -100.0, + far: float = 100.0, *, render_target: Optional[Framebuffer] = None, window: Optional["Window"] = None From b001919fe8af81f253e5a7f117ff1cef4b29429e Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 01:55:46 -0400 Subject: [PATCH 09/16] Fix from_raw_data treating 0 near and far values as not set --- arcade/camera/camera_2d.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index ce0347474c..50632ba1b9 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -151,7 +151,7 @@ def from_raw_data( _projection: OrthographicProjectionData = OrthographicProjectionData( left=left, right=right, top=top, bottom=bottom, - near=near or 0.0, far=far or 100.0, + near=near, far=far, viewport=viewport or (0, 0, width, height) ) From b857fe3059203707bd4002714aa3e8e8bb248ea3 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 01:57:24 -0400 Subject: [PATCH 10/16] Add near/far validation to Camera2D.from_raw_data --- arcade/camera/camera_2d.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 50632ba1b9..27ca03c8e4 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -139,6 +139,11 @@ def from_raw_data( raise ZeroProjectionDimension(( f"projection height is 0 due to equal {bottom=}" f"and {top=}")) + if near == far: + raise ZeroProjectionDimension( + f"projection depth is 0 due to equal {near=}" + f"and {far=} values" + ) _pos = position or (half_width, half_height) _data = CameraData( From 9f25a68468e0f5321b8b5b5c2432eef2bd358b4e Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 02:05:27 -0400 Subject: [PATCH 11/16] Add tests for from_raw_data near/far validation * Turn camera_class into a fixture * Add simple inline test for from_raw_data's near/far validation --- tests/unit/camera/test_camera2d.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/unit/camera/test_camera2d.py b/tests/unit/camera/test_camera2d.py index 574ebccf93..667de610b5 100644 --- a/tests/unit/camera/test_camera2d.py +++ b/tests/unit/camera/test_camera2d.py @@ -18,6 +18,11 @@ class Camera2DSub2(Camera2DSub1): ALL_CAMERA2D_TYPES = [Camera2D] + CAMERA2D_SUBS +@pytest.fixture(params=ALL_CAMERA2D_TYPES) +def camera_class(request): + return request.param + + AT_LEAST_ONE_EQUAL_VIEWPORT_DIMENSION = [ (-100., -100., -1., 1.), (100., 100., -1., 1.), @@ -30,8 +35,7 @@ class Camera2DSub2(Camera2DSub1): @pytest.mark.parametrize("bad_projection", AT_LEAST_ONE_EQUAL_VIEWPORT_DIMENSION) -@pytest.mark.parametrize("camera_class", ALL_CAMERA2D_TYPES) -def test_camera2d_from_raw_data_bound_validation( +def test_camera2d_from_raw_data_projection_xy_pairs_equal_raises_zeroprojectiondimension( window: Window, bad_projection: Tuple[float, float, float, float], # Clarify type for PyCharm camera_class @@ -40,6 +44,20 @@ def test_camera2d_from_raw_data_bound_validation( camera_class.from_raw_data(projection=bad_projection) + +def test_camera2d_from_raw_data_equal_near_far_raises_zeroprojectiondimension( + window: Window, camera_class +): + with pytest.raises(ZeroProjectionDimension): + camera_class.from_raw_data(near=-100, far=-100) + + with pytest.raises(ZeroProjectionDimension): + camera_class.from_raw_data(near=0, far=0) + + with pytest.raises(ZeroProjectionDimension): + camera_class.from_raw_data(near=100, far=100) + + @pytest.mark.parametrize("camera_class", CAMERA2D_SUBS) def test_camera2d_from_raw_data_inheritance_safety( window: Window, From 52406ae2a32ccbbef239d97eaa565bba073ea4d5 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 02:18:56 -0400 Subject: [PATCH 12/16] Add projection validation to Camera2D.__init__ --- arcade/camera/camera_2d.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 27ca03c8e4..120e3e6175 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -65,6 +65,25 @@ def __init__(self, *, projection_data: Optional[OrthographicProjectionData] = None, render_target: Optional[Framebuffer] = None, window: Optional["Window"] = None): + + if projection_data: + left, right = projection_data.left, projection_data.right + if projection_data.left == projection_data.right: + raise ZeroProjectionDimension(( + f"projection width is 0 due to equal {left=}" + f"and {right=} values")) + bottom, top = projection_data.left, projection_data.right + if bottom == top: + raise ZeroProjectionDimension(( + f"projection height is 0 due to equal {bottom=}" + f"and {top=}")) + near, far = projection_data.near, projection_data.far + if near == far: + raise ZeroProjectionDimension( + f"projection depth is 0 due to equal {near=}" + f"and {far=} values" + ) + self._window: "Window" = window or get_window() self.render_target: Framebuffer = render_target or self._window.ctx.screen width, height = self.render_target.size From 0d49b22716ef09d34d8575cb686ec822956a1ca4 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 02:28:38 -0400 Subject: [PATCH 13/16] Fix init validation + add tests --- arcade/camera/camera_2d.py | 2 +- tests/unit/camera/test_camera2d.py | 39 +++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 120e3e6175..4fa91d441f 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -72,7 +72,7 @@ def __init__(self, *, raise ZeroProjectionDimension(( f"projection width is 0 due to equal {left=}" f"and {right=} values")) - bottom, top = projection_data.left, projection_data.right + bottom, top = projection_data.bottom, projection_data.top if bottom == top: raise ZeroProjectionDimension(( f"projection height is 0 due to equal {bottom=}" diff --git a/tests/unit/camera/test_camera2d.py b/tests/unit/camera/test_camera2d.py index 667de610b5..036eddb841 100644 --- a/tests/unit/camera/test_camera2d.py +++ b/tests/unit/camera/test_camera2d.py @@ -4,7 +4,7 @@ from arcade import Window from arcade.camera import Camera2D -from arcade.camera.data_types import ZeroProjectionDimension +from arcade.camera.data_types import ZeroProjectionDimension, OrthographicProjectionData class Camera2DSub1(Camera2D): @@ -33,8 +33,19 @@ def camera_class(request): (5., 5., 5., 5.) ] +NEAR_FAR_VALUES = [-50.,0.,50.] + + +@pytest.fixture(params=AT_LEAST_ONE_EQUAL_VIEWPORT_DIMENSION) +def bad_projection(request): + return request.param + + +@pytest.fixture(params=NEAR_FAR_VALUES) +def same_near_far(request): + return request.param + -@pytest.mark.parametrize("bad_projection", AT_LEAST_ONE_EQUAL_VIEWPORT_DIMENSION) def test_camera2d_from_raw_data_projection_xy_pairs_equal_raises_zeroprojectiondimension( window: Window, bad_projection: Tuple[float, float, float, float], # Clarify type for PyCharm @@ -44,18 +55,34 @@ def test_camera2d_from_raw_data_projection_xy_pairs_equal_raises_zeroprojectiond camera_class.from_raw_data(projection=bad_projection) +def test_camera2d_init_xy_pairs_equal_raises_zeroprojectiondimension( + window: Window, + bad_projection: Tuple[float, float, float, float], + camera_class +): + data = OrthographicProjectionData( + *bad_projection, -100.0, 100.0, + viewport=(0,0, 800, 600) + ) + + with pytest.raises(ZeroProjectionDimension): + _ = Camera2D(projection_data=data) + def test_camera2d_from_raw_data_equal_near_far_raises_zeroprojectiondimension( - window: Window, camera_class + window: Window, + same_near_far: float, + camera_class ): with pytest.raises(ZeroProjectionDimension): - camera_class.from_raw_data(near=-100, far=-100) + camera_class.from_raw_data(near=near_far, far=near_far) with pytest.raises(ZeroProjectionDimension): - camera_class.from_raw_data(near=0, far=0) + camera_class.from_raw_data(near=near_far, far=near_far) with pytest.raises(ZeroProjectionDimension): - camera_class.from_raw_data(near=100, far=100) + camera_class.from_raw_data(near=near_far, far=near_far) + @pytest.mark.parametrize("camera_class", CAMERA2D_SUBS) From 0594ce29099427d1a25c78cfb9e3e0c439d8f0e4 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 02:33:06 -0400 Subject: [PATCH 14/16] Fix typo --- tests/unit/camera/test_camera2d.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/camera/test_camera2d.py b/tests/unit/camera/test_camera2d.py index 036eddb841..4ef6bea933 100644 --- a/tests/unit/camera/test_camera2d.py +++ b/tests/unit/camera/test_camera2d.py @@ -74,6 +74,7 @@ def test_camera2d_from_raw_data_equal_near_far_raises_zeroprojectiondimension( same_near_far: float, camera_class ): + near_far = same_near_far with pytest.raises(ZeroProjectionDimension): camera_class.from_raw_data(near=near_far, far=near_far) From 38e4fb1c654028ebe4b7700990026f06f5b0fca5 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 02:43:10 -0400 Subject: [PATCH 15/16] Fixtures replace inline repeated tests --- tests/unit/camera/test_camera2d.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/unit/camera/test_camera2d.py b/tests/unit/camera/test_camera2d.py index 4ef6bea933..a6ea2ed5e8 100644 --- a/tests/unit/camera/test_camera2d.py +++ b/tests/unit/camera/test_camera2d.py @@ -78,13 +78,6 @@ def test_camera2d_from_raw_data_equal_near_far_raises_zeroprojectiondimension( with pytest.raises(ZeroProjectionDimension): camera_class.from_raw_data(near=near_far, far=near_far) - with pytest.raises(ZeroProjectionDimension): - camera_class.from_raw_data(near=near_far, far=near_far) - - with pytest.raises(ZeroProjectionDimension): - camera_class.from_raw_data(near=near_far, far=near_far) - - @pytest.mark.parametrize("camera_class", CAMERA2D_SUBS) def test_camera2d_from_raw_data_inheritance_safety( From 760ae097b6526b76017eae30692763dbe1022645 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 12 Apr 2024 02:47:21 -0400 Subject: [PATCH 16/16] Fix very long lines --- arcade/camera/camera_2d.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arcade/camera/camera_2d.py b/arcade/camera/camera_2d.py index 4fa91d441f..5cc89de6f1 100644 --- a/arcade/camera/camera_2d.py +++ b/arcade/camera/camera_2d.py @@ -239,7 +239,9 @@ def left(self) -> float: @left.setter def left(self, _left: float) -> None: - self._camera_data.position = (_left - self._projection_data.left / self._camera_data.zoom,) + self._camera_data.position[1:] + self._camera_data.position =\ + (_left - self._projection_data.left / self._camera_data.zoom,)\ + + self._camera_data.position[1:] @property def right(self) -> float: @@ -251,7 +253,9 @@ def right(self) -> float: @right.setter def right(self, _right: float) -> None: - self._camera_data.position = (_right - self._projection_data.right / self._camera_data.zoom,) + self._camera_data.position[1:] + self._camera_data.position =\ + (_right - self._projection_data.right / self._camera_data.zoom,)\ + + self._camera_data.position[1:] @property def bottom(self) -> float: