diff --git a/docs/changelog.rst b/docs/changelog.rst index 489907a6..a089d465 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,6 +45,11 @@ 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 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 b0894443..08b2cad1 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,24 @@ 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* or *up* is zero or has a + non-finite length, or a caller-supplied *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) + # 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"): + 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) 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..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,6 +218,48 @@ def test_returns_self_for_chaining(self): result = vs.look_along([1, 1, 1]) assert result is vs + @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(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 + 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."""