From 9c47eb9cd9214c28c1dc88d7131c5e79e23721a7 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Mon, 17 Aug 2026 11:43:47 +0100 Subject: [PATCH 1/4] Reject non-finite look_along directions and fix the camera-convention docs --- src/hofmann/model/view_state.py | 30 +++++++++++++++-------------- tests/test_model/test_view_state.py | 9 +++++++++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index b0894443..99f647df 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -192,22 +192,21 @@ def look_along( *, up: np.ndarray | list[float] | tuple[float, ...] = (0.0, 1.0, 0.0), ) -> ViewState: - """Set the rotation so the camera looks along *direction*. + """Set the rotation to view along the *direction* axis. - The view is oriented so that *direction* points into the screen - (along +z in camera space). The *up* vector determines which + The camera sits on the ``+direction`` side, looking back towards + the origin, so *direction* points out of the screen towards the + viewer (the camera's +z axis). The *up* vector determines which way is "up" on screen. - This is equivalent to placing the camera at a point along - *direction* looking back towards the origin. - Returns ``self`` so callers can chain, e.g.:: scene.view = ViewState(centre=centroid).look_along([1, 1, 1]) Args: - direction: 3D vector giving the viewing direction (from - the camera towards the scene). Need not be normalised. + direction: 3D vector giving the axis to view along; the + camera is placed on the ``+direction`` side, looking + back towards the origin. Need not be normalised. up: 3D vector indicating the upward direction in screen space. Defaults to ``[0, 1, 0]``. @@ -215,16 +214,19 @@ def look_along( ``self``, with the rotation updated in place. Raises: - ValueError: If *direction* is zero-length or *up* is - parallel to *direction*. + ValueError: If *direction* is zero or has a non-finite + length, or *up* is parallel to *direction*. """ d = np.asarray(direction, dtype=float) u = np.asarray(up, dtype=float) - d_len = np.linalg.norm(d) - if d_len < 1e-12: - raise ValueError("direction must be non-zero") - fwd = d / d_len # camera z-axis (into screen) + with np.errstate(over="ignore"): + # A huge direction overflows the norm to inf; the finiteness + # check below rejects it in place of a bare numpy warning. + d_len = np.linalg.norm(d) + if not np.isfinite(d_len) or d_len < 1e-12: + raise ValueError("direction must be finite and non-zero") + fwd = d / d_len # camera z-axis (out of screen) right = np.cross(u, fwd) right_len = np.linalg.norm(right) diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 4786ea66..4a080ad7 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -207,6 +207,15 @@ def test_returns_self_for_chaining(self): result = vs.look_along([1, 1, 1]) assert result is vs + def test_overflowing_direction_raises(self): + """A direction whose length overflows float64 is rejected, not + silently turned into a degenerate NaN rotation.""" + vs = ViewState() + before = vs.rotation.copy() + with pytest.raises(ValueError, match="finite"): + vs.look_along([1e308, 1e308, 1e308]) + np.testing.assert_array_equal(vs.rotation, before) + class TestViewStateSlab: """Tests for depth-slab clipping on ViewState.""" From 8ecf076d30c60fbffac7cdbb653e65cd6cdbbff9 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Mon, 17 Aug 2026 11:44:50 +0100 Subject: [PATCH 2/4] Record the look_along finiteness guard in the changelog --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 489907a6..637c0348 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,6 +45,10 @@ Unreleased - A view opened with a zoom above one now starts zoomed in at that level, rather than fitting the window regardless of the zoom set. +- ``ViewState.look_along`` now raises a clear error for a viewing + direction that is not finite — for example one whose length overflows + — instead of silently producing a degenerate, all-blank view. + 0.20.0 ------ From 398b71fba1fd0a091b6851135fb5ab2051fa627f Mon Sep 17 00:00:00 2001 From: Benjamin Date: Mon, 17 Aug 2026 11:58:44 +0100 Subject: [PATCH 3/4] Guard the look_along up vector against zero and non-finite values The direction guard left the up vector unchecked: a non-finite up ([nan, ...] or [inf, 0, 0]) produced a silent all-NaN rotation and an overflowing up ([1e308, ...]) a silent rank-deficient one, the same failure the direction guard removes. A zero up raised, but with a misleading 'parallel to the viewing direction' message. Guard up symmetrically (finite and non-zero), leaving the genuine parallel case to the existing check. --- docs/changelog.rst | 7 ++++--- src/hofmann/model/view_state.py | 12 ++++++++---- tests/test_model/test_view_state.py | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 637c0348..a089d465 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,9 +45,10 @@ Unreleased - A view opened with a zoom above one now starts zoomed in at that level, rather than fitting the window regardless of the zoom set. -- ``ViewState.look_along`` now raises a clear error for a viewing - direction that is not finite — for example one whose length overflows - — instead of silently producing a degenerate, all-blank view. +- ``ViewState.look_along`` now rejects a viewing direction or up vector + that is zero or not finite — for example one whose length overflows — + raising a clear error instead of silently producing a degenerate, + all-blank view. 0.20.0 ------ diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 99f647df..8eb7cea0 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -214,18 +214,22 @@ def look_along( ``self``, with the rotation updated in place. Raises: - ValueError: If *direction* is zero or has a non-finite - length, or *up* is parallel to *direction*. + ValueError: If *direction* or *up* is zero or has a + non-finite length, or *up* is parallel to *direction*. """ d = np.asarray(direction, dtype=float) u = np.asarray(up, dtype=float) + # Norms overflow to inf for a huge vector; the finiteness checks + # below reject that (and NaN, and zero) in place of a bare numpy + # warning or a silent degenerate rotation. with np.errstate(over="ignore"): - # A huge direction overflows the norm to inf; the finiteness - # check below rejects it in place of a bare numpy warning. d_len = np.linalg.norm(d) + u_len = np.linalg.norm(u) if not np.isfinite(d_len) or d_len < 1e-12: raise ValueError("direction must be finite and non-zero") + if not np.isfinite(u_len) or u_len < 1e-12: + raise ValueError("up must be finite and non-zero") fwd = d / d_len # camera z-axis (out of screen) right = np.cross(u, fwd) diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 4a080ad7..03308c5b 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -216,6 +216,28 @@ def test_overflowing_direction_raises(self): vs.look_along([1e308, 1e308, 1e308]) np.testing.assert_array_equal(vs.rotation, before) + def test_non_finite_up_raises(self): + """A non-finite or overflowing up vector is rejected, not + silently turned into a degenerate rotation (symmetric with + direction).""" + vs = ViewState() + before = vs.rotation.copy() + for up in ( + [np.inf, 0.0, 0.0], + [np.nan, np.nan, np.nan], + [1e308, 1e308, 1e308], + ): + with pytest.raises(ValueError, match="up must be finite"): + vs.look_along([1, 0, 0], up=up) + np.testing.assert_array_equal(vs.rotation, before) + + def test_zero_up_raises_clear_error(self): + """A zero-length up raises a clear error, not the misleading + 'parallel to the viewing direction' message.""" + vs = ViewState() + with pytest.raises(ValueError, match="up must be finite and non-zero"): + vs.look_along([1, 0, 0], up=[0, 0, 0]) + class TestViewStateSlab: """Tests for depth-slab clipping on ViewState.""" From 93fdabba94dc54a6a22473818352a6f02335d42e Mon Sep 17 00:00:00 2001 From: Benjamin Date: Mon, 17 Aug 2026 12:03:45 +0100 Subject: [PATCH 4/4] Pin the look_along convention and broaden the input-guard tests - Add a depth-sign test: a point along +direction is nearer the viewer than one along -direction, pinning the documented out-of-the-screen convention that the existing direction-maps-to-z test (which discards depth) leaves unpinned. - Parametrise the non-finite direction test over NaN and inf components, not only the overflowing-norm case. - Add a zero-direction test for the finite-and-non-zero message. - Tighten the Raises doc: only a caller-supplied up parallel to the direction raises; a default parallel up falls back. --- src/hofmann/model/view_state.py | 3 ++- tests/test_model/test_view_state.py | 28 +++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 8eb7cea0..08b2cad1 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -215,7 +215,8 @@ def look_along( Raises: ValueError: If *direction* or *up* is zero or has a - non-finite length, or *up* is parallel to *direction*. + non-finite length, or a caller-supplied *up* is parallel + to *direction*. """ d = np.asarray(direction, dtype=float) u = np.asarray(up, dtype=float) diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 03308c5b..1307ff70 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -149,6 +149,17 @@ def test_direction_maps_to_z(self): xy, _, _ = vs.project(coords) np.testing.assert_allclose(xy[0], [0.0, 0.0], atol=1e-12) + def test_positive_direction_is_nearest_the_viewer(self): + """A point along +direction has larger depth (is nearer the + viewer) than one along -direction, pinning the documented + convention that +direction points out of the screen.""" + vs = ViewState() + vs.look_along([1, 1, 1]) + _, depth, _ = vs.project( + np.array([[1.0, 1.0, 1.0], [-1.0, -1.0, -1.0]]) + ) + assert depth[0] > depth[1] + def test_x_axis_view(self): """Looking along [1, 0, 0] should show the yz plane.""" vs = ViewState() @@ -207,15 +218,26 @@ def test_returns_self_for_chaining(self): result = vs.look_along([1, 1, 1]) assert result is vs - def test_overflowing_direction_raises(self): - """A direction whose length overflows float64 is rejected, not + @pytest.mark.parametrize("direction", [ + [1e308, 1e308, 1e308], # length overflows to inf + [np.nan, 0.0, 0.0], + [np.inf, 0.0, 0.0], + ]) + def test_non_finite_direction_raises(self, direction): + """A non-finite or overflowing direction is rejected, not silently turned into a degenerate NaN rotation.""" vs = ViewState() before = vs.rotation.copy() with pytest.raises(ValueError, match="finite"): - vs.look_along([1e308, 1e308, 1e308]) + vs.look_along(direction) np.testing.assert_array_equal(vs.rotation, before) + def test_zero_direction_raises(self): + """A zero-length direction is rejected.""" + vs = ViewState() + with pytest.raises(ValueError, match="non-zero"): + vs.look_along([0.0, 0.0, 0.0]) + def test_non_finite_up_raises(self): """A non-finite or overflowing up vector is rejected, not silently turned into a degenerate rotation (symmetric with