From f494c9ce22b92e2c0dc9d93d543c46431461e425 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 07:15:33 +0100 Subject: [PATCH 01/13] Add Orthographic and Perspective projection mode types Introduce frozen dataclasses for the two projection modes, validated at construction. Nothing consumes them yet. Perspective takes a positive strength, so a parallel projection is Orthographic rather than a zero strength: the sentinel encoding it replaces made view_distance meaningless whenever perspective was off, and left no answer to whether 0.0 meant "disabled" or "very weak". The screen scale D / (D - z * s) places the eye at view_distance / strength, so strength 1.0 is a true pinhole at view_distance. --- src/hofmann/__init__.py | 4 +++ src/hofmann/model/__init__.py | 4 ++- src/hofmann/model/view_state.py | 39 +++++++++++++++++++++++++ tests/test_model/test_view_state.py | 44 ++++++++++++++++++++++++++++- 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/hofmann/__init__.py b/src/hofmann/__init__.py index c9a068bd..f42fe82c 100644 --- a/src/hofmann/__init__.py +++ b/src/hofmann/__init__.py @@ -39,6 +39,8 @@ RenderStyle, SlabClipMode, StructureScene, + Orthographic, + Perspective, ViewState, WidgetCorner, normalise_colour, @@ -69,6 +71,8 @@ "SlabClipMode", "StructureScene", "StyleSet", + "Orthographic", + "Perspective", "ViewState", "WidgetCorner", "compute_bonds", diff --git a/src/hofmann/model/__init__.py b/src/hofmann/model/__init__.py index b46b403d..776c33be 100644 --- a/src/hofmann/model/__init__.py +++ b/src/hofmann/model/__init__.py @@ -30,7 +30,7 @@ _DEFAULT_SPACING, ) from hofmann.model.structure_scene import StructureScene -from hofmann.model.view_state import ViewState +from hofmann.model.view_state import Orthographic, Perspective, ViewState __all__ = [ "AtomLegendItem", @@ -52,6 +52,8 @@ "RenderStyle", "SlabClipMode", "StructureScene", + "Orthographic", + "Perspective", "ViewState", "WidgetCorner", "normalise_colour", diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 9d7797df..4058bc75 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -1,10 +1,49 @@ from __future__ import annotations +import math from dataclasses import dataclass, field import numpy as np +@dataclass(frozen=True, slots=True) +class Orthographic: + """Parallel projection: depth is not foreshortened.""" + + +@dataclass(frozen=True, slots=True) +class Perspective: + """Perspective projection with the eye on the camera's +z axis. + + Screen positions are scaled by ``D / (D - z * s)`` for an atom at + camera depth *z*, which places the eye at ``view_distance / + strength``. A *strength* of ``1.0`` is therefore a true pinhole + camera at :attr:`view_distance`; smaller values move the eye + further out, weakening the foreshortening. + + Attributes: + strength: Perspective strength. Must be positive; use + :class:`Orthographic` for a parallel projection. + view_distance: Reference distance from the scene centre, + equal to the eye distance at ``strength = 1``. + """ + + strength: float = 0.5 + view_distance: float = 10.0 + + def __post_init__(self) -> None: + if not math.isfinite(self.strength) or self.strength <= 0: + raise ValueError( + "strength must be finite and positive (use Orthographic " + f"for a parallel projection), got {self.strength}" + ) + if not math.isfinite(self.view_distance) or self.view_distance <= 0: + raise ValueError( + "view_distance must be finite and positive, got " + f"{self.view_distance}" + ) + + @dataclass class ViewState: """Camera state for 3D-to-2D projection. diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 26a2c400..8ae96b04 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -1,9 +1,11 @@ """Tests for ViewState projection, look_along, slab clipping, and validation.""" +import dataclasses + import numpy as np import pytest -from hofmann.model.view_state import ViewState +from hofmann.model.view_state import Orthographic, Perspective, ViewState class TestViewStateProject: @@ -293,3 +295,43 @@ def test_negative_view_distance_raises(self): def test_valid_view_state_accepted(self): vs = ViewState(zoom=2.0, view_distance=15.0) assert vs.zoom == 2.0 + + +class TestProjectionTypes: + def test_perspective_defaults(self): + p = Perspective() + assert p.strength == 0.5 + assert p.view_distance == 10.0 + + @pytest.mark.parametrize( + "strength", [0.0, -0.5, float("nan"), float("inf")], + ) + def test_invalid_strength_rejected(self, strength): + with pytest.raises(ValueError, match="strength"): + Perspective(strength=strength) + + @pytest.mark.parametrize( + "view_distance", [0.0, -1.0, float("nan"), float("inf")], + ) + def test_invalid_view_distance_rejected(self, view_distance): + with pytest.raises(ValueError, match="view_distance"): + Perspective(view_distance=view_distance) + + def test_zero_strength_directs_the_caller_to_orthographic(self): + """A parallel projection is a different type, not a zero strength.""" + with pytest.raises(ValueError, match="Orthographic"): + Perspective(strength=0.0) + + def test_modes_are_frozen(self): + with pytest.raises(dataclasses.FrozenInstanceError): + Perspective().strength = 0.9 + + def test_modes_reject_unknown_attributes(self): + """slots keeps a typo loud rather than setting an inert attribute.""" + with pytest.raises((AttributeError, TypeError)): + Orthographic().anything = 1 + + def test_modes_compare_by_value(self): + assert Perspective(0.5, 10.0) == Perspective(0.5, 10.0) + assert Perspective(0.5, 10.0) != Perspective(0.6, 10.0) + assert Orthographic() == Orthographic() From 13bcc4f5b211223f0f63cbc30be4a45dfbff5abf Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 07:24:08 +0100 Subject: [PATCH 02/13] Select the projection mode with a single sum-typed field Replace ViewState.perspective and ViewState.view_distance with a single projection field holding Orthographic or Perspective, and translate every consumer. The arithmetic is carried over unchanged: rendered output is bit-for-bit identical to the two-field implementation for both modes, verified across a spread of scenes and camera configurations. Two interactive keys change behaviour, because the state they used to mutate no longer exists in the orthographic case: d and D now act only in perspective mode, where previously they adjusted a view_distance that had no effect until perspective was switched on. P now steps out to Orthographic once the strength falls below a floor, rather than approaching zero and stranding on a float residue. --- src/hofmann/model/view_state.py | 57 ++++++++++---------- src/hofmann/rendering/bond_geometry.py | 14 +++-- src/hofmann/rendering/cell_edges.py | 20 ++++--- src/hofmann/rendering/interactive.py | 50 ++++++++++++++---- src/hofmann/rendering/projection.py | 20 +++---- tests/test_model/test_view_state.py | 25 ++++----- tests/test_rendering/test_interactive.py | 66 +++++++++++++++--------- tests/test_rendering/test_projection.py | 15 ++++-- 8 files changed, 166 insertions(+), 101 deletions(-) diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 4058bc75..e731af62 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -63,8 +63,8 @@ class ViewState: rotation: 3x3 rotation matrix. zoom: Magnification factor. centre: 3D point about which to centre the view. - perspective: Perspective strength (0 = orthographic). - view_distance: Distance from camera to scene centre. + projection: Projection mode, :class:`Orthographic` (the + default) or :class:`Perspective`. slab_origin: 3D point defining the slab reference depth, or ``None`` to use *centre*. slab_near: Near offset from the slab origin depth (negative = @@ -80,8 +80,9 @@ class ViewState: centre: np.ndarray = field( default_factory=lambda: np.zeros(3, dtype=float) ) - perspective: float = 0.0 - view_distance: float = 10.0 + projection: Orthographic | Perspective = field( + default_factory=Orthographic + ) slab_origin: np.ndarray | None = None slab_near: float | None = None slab_far: float | None = None @@ -89,10 +90,6 @@ class ViewState: def __post_init__(self) -> None: if self.zoom <= 0: raise ValueError(f"zoom must be positive, got {self.zoom}") - if self.view_distance <= 0: - raise ValueError( - f"view_distance must be positive, got {self.view_distance}" - ) def project( self, coords: np.ndarray, radii: np.ndarray | None = None, @@ -120,25 +117,31 @@ def project( rotated = centred @ self.rotation.T depth = rotated[:, 2] - if self.perspective > 0: - # Eye-to-atom distance along z. - d = self.view_distance - depth * self.perspective - scale = self.view_distance / d - xy = rotated[:, :2] * scale[:, np.newaxis] * self.zoom - - if radii is not None: - radii = np.asarray(radii, dtype=float) - # Silhouette radius: r * D / sqrt(d^2 - r^2). - denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) - projected_radii = radii * self.view_distance / denom * self.zoom - else: - projected_radii = np.zeros(len(depth)) - else: - xy = rotated[:, :2] * self.zoom - if radii is not None: - projected_radii = np.asarray(radii, dtype=float) * self.zoom - else: - projected_radii = np.zeros(len(depth)) + match self.projection: + case Perspective() as p: + # Denominator of the projection scale, proportional to + # the eye-to-atom distance along z. + d = p.view_distance - depth * p.strength + scale = p.view_distance / d + xy = rotated[:, :2] * scale[:, np.newaxis] * self.zoom + + if radii is not None: + radii = np.asarray(radii, dtype=float) + # Silhouette radius: r * D / sqrt(d^2 - r^2). + denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) + projected_radii = ( + radii * p.view_distance / denom * self.zoom + ) + else: + projected_radii = np.zeros(len(depth)) + case Orthographic(): + xy = rotated[:, :2] * self.zoom + if radii is not None: + projected_radii = ( + np.asarray(radii, dtype=float) * self.zoom + ) + else: + projected_radii = np.zeros(len(depth)) return xy, depth, projected_radii diff --git a/src/hofmann/rendering/bond_geometry.py b/src/hofmann/rendering/bond_geometry.py index 94b22912..ebcb311c 100644 --- a/src/hofmann/rendering/bond_geometry.py +++ b/src/hofmann/rendering/bond_geometry.py @@ -4,7 +4,7 @@ import numpy as np -from hofmann.model import ViewState +from hofmann.model import Perspective, ViewState from hofmann.rendering.projection import _project_point @@ -188,7 +188,11 @@ def _bond_polygon( # perpendicular to the view has cth~0). # For orthographic projection, push the eye to effective infinity # so all view rays are parallel (matching XBS pmode==0 behaviour). - eye_dist = view.view_distance if view.perspective > 0 else 1e6 + eye_dist = ( + view.projection.view_distance + if isinstance(view.projection, Perspective) + else 1e6 + ) eye = np.array([0.0, 0.0, eye_dist]) q_a = eye - p_a q_b = eye - p_b @@ -331,7 +335,11 @@ def _bond_polygons_batch( valid &= (bond_len_safe - w_a - w_b) > 0 # Foreshortening angles. - eye_dist = view.view_distance if view.perspective > 0 else 1e6 + eye_dist = ( + view.projection.view_distance + if isinstance(view.projection, Perspective) + else 1e6 + ) eye = np.array([0.0, 0.0, eye_dist]) q_a = eye - p_a # (n_bonds, 3) q_b = eye - p_b # (n_bonds, 3) diff --git a/src/hofmann/rendering/cell_edges.py b/src/hofmann/rendering/cell_edges.py index c3fc8df1..a893b01d 100644 --- a/src/hofmann/rendering/cell_edges.py +++ b/src/hofmann/rendering/cell_edges.py @@ -6,7 +6,12 @@ import numpy as np -from hofmann.model import CellEdgeStyle, ViewState, normalise_colour +from hofmann.model import ( + CellEdgeStyle, + Perspective, + ViewState, + normalise_colour, +) # The 12 edges of a unit cube in fractional coordinates. _CUBE_EDGES: list[tuple[tuple[int, int, int], tuple[int, int, int]]] = [ @@ -305,14 +310,13 @@ def _collect_cell_edges( sub_c_e = c_s + (c_e - c_s) * t1 sub_d = (sub_c_s[2] + sub_c_e[2]) / 2.0 - if view.perspective > 0: - s_s = view.view_distance / ( - view.view_distance - - sub_c_s[2] * view.perspective + if isinstance(view.projection, Perspective): + p = view.projection + s_s = p.view_distance / ( + p.view_distance - sub_c_s[2] * p.strength ) - s_e = view.view_distance / ( - view.view_distance - - sub_c_e[2] * view.perspective + s_e = p.view_distance / ( + p.view_distance - sub_c_e[2] * p.strength ) xy_s_i = sub_c_s[:2] * s_s * view.zoom xy_e_i = sub_c_e[:2] * s_e * view.zoom diff --git a/src/hofmann/rendering/interactive.py b/src/hofmann/rendering/interactive.py index cc7d8efb..f78069f5 100644 --- a/src/hofmann/rendering/interactive.py +++ b/src/hofmann/rendering/interactive.py @@ -3,6 +3,7 @@ from __future__ import annotations import time +from dataclasses import replace from typing import Any import matplotlib.pyplot as plt @@ -11,6 +12,8 @@ from hofmann.model import ( CmapSpec, Colour, + Orthographic, + Perspective, RenderStyle, StructureScene, ViewState, @@ -64,7 +67,9 @@ def _rotation_z(angle: float) -> np.ndarray: _KEY_ZOOM_FACTOR = 1.1 # multiplicative zoom per key press / scroll step _KEY_PAN_FRACTION = 0.05 # fraction of scene extent per key press _PERSPECTIVE_STEP = 0.1 # perspective increment per key press +_PERSPECTIVE_FLOOR = 1e-9 # below this, the descent lands on Orthographic _DISTANCE_FACTOR = 1.05 # viewing distance multiplier per key press +_MIN_VIEW_DISTANCE = 0.1 # closest the eye may be brought to the centre _HELP_TEXT = """\ Arrows Rotate Shift+Arrows Pan @@ -167,13 +172,41 @@ def _apply_key_action( # -- Perspective / distance -- elif key == "p": - view.perspective = min(1.0, view.perspective + _PERSPECTIVE_STEP) + match view.projection: + case Perspective() as proj: + view.projection = replace( + proj, + strength=min(1.0, proj.strength + _PERSPECTIVE_STEP), + ) + case _: + view.projection = Perspective(strength=_PERSPECTIVE_STEP) elif key == "P": - view.perspective = max(0.0, view.perspective - _PERSPECTIVE_STEP) + match view.projection: + case Perspective() as proj: + strength = proj.strength - _PERSPECTIVE_STEP + # The floor absorbs the float residue left by repeated + # addition and subtraction, so the descent lands on + # Orthographic rather than stranding on ~1e-17. + if strength > _PERSPECTIVE_FLOOR: + view.projection = replace(proj, strength=strength) + else: + view.projection = Orthographic() elif key == "d": - view.view_distance *= _DISTANCE_FACTOR + match view.projection: + case Perspective() as proj: + view.projection = replace( + proj, view_distance=proj.view_distance * _DISTANCE_FACTOR, + ) elif key == "D": - view.view_distance = max(0.1, view.view_distance / _DISTANCE_FACTOR) + match view.projection: + case Perspective() as proj: + view.projection = replace( + proj, + view_distance=max( + _MIN_VIEW_DISTANCE, + proj.view_distance / _DISTANCE_FACTOR, + ), + ) # -- Style toggles (no recomputation needed) -- elif key == "b": @@ -230,8 +263,7 @@ def _apply_key_action( view.rotation = initial_view["rotation"].copy() view.zoom = initial_view["zoom"] view.centre = initial_view["centre"].copy() - view.perspective = initial_view["perspective"] - view.view_distance = initial_view["view_distance"] + view.projection = initial_view["projection"] # -- Help overlay -- elif key == "h": @@ -338,8 +370,7 @@ def render_mpl_interactive( rotation=scene.view.rotation.copy(), zoom=scene.view.zoom, centre=scene.view.centre.copy(), - perspective=scene.view.perspective, - view_distance=scene.view.view_distance, + projection=scene.view.projection, slab_origin=( scene.view.slab_origin.copy() if scene.view.slab_origin is not None else None @@ -395,8 +426,7 @@ def render_mpl_interactive( "rotation": view.rotation.copy(), "zoom": view.zoom, "centre": view.centre.copy(), - "perspective": view.perspective, - "view_distance": view.view_distance, + "projection": view.projection, } _DRAG_SENSITIVITY = 0.01 # radians per pixel diff --git a/src/hofmann/rendering/projection.py b/src/hofmann/rendering/projection.py index 879c7fbf..2c33d5de 100644 --- a/src/hofmann/rendering/projection.py +++ b/src/hofmann/rendering/projection.py @@ -4,7 +4,7 @@ import numpy as np -from hofmann.model import StructureScene, ViewState +from hofmann.model import Perspective, StructureScene, ViewState from hofmann.model.composition import Composition, _OCCUPANCY_TOLERANCE from hofmann.rendering.precompute import _compute_atom_radii @@ -41,10 +41,11 @@ def _project_point( is the perspective scale factor at this depth. """ z = pt[2] - if view.perspective > 0: - s = view.view_distance / (view.view_distance - z * view.perspective) - else: - s = 1.0 + match view.projection: + case Perspective() as p: + s = p.view_distance / (p.view_distance - z * p.strength) + case _: + s = 1.0 xy = pt[:2] * s * view.zoom return xy, s @@ -99,13 +100,14 @@ def _scene_extent( # worst-case magnification for an atom at distance *d* from the # view centre is when it is rotated to depth z = +d (closest to # the camera). - if view.perspective > 0 and len(dists) > 0: + if isinstance(view.projection, Perspective) and len(dists) > 0: + p = view.projection worst_depth = float(np.max(dists)) - denom = view.view_distance - worst_depth * view.perspective + denom = p.view_distance - worst_depth * p.strength if denom > 0: - persp_scale = view.view_distance / denom + persp_scale = p.view_distance / denom else: - persp_scale = view.view_distance / 1e-6 + persp_scale = p.view_distance / 1e-6 max_extent *= persp_scale return float(max_extent * view.zoom) diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 8ae96b04..0b1f5250 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -44,7 +44,7 @@ def test_90_degree_z_rotation(self): np.testing.assert_allclose(xy, [[0.0, 1.0]], atol=1e-10) def test_perspective_scaling(self): - vs = ViewState(perspective=1.0, view_distance=10.0) + vs = ViewState(projection=Perspective(1.0, 10.0)) coords = np.array([ [1.0, 0.0, 0.0], [1.0, 0.0, -5.0], @@ -71,7 +71,7 @@ def test_projected_radii_orthographic(self): np.testing.assert_allclose(proj_r, [3.0]) # r * zoom def test_projected_radii_perspective(self): - vs = ViewState(perspective=1.0, view_distance=10.0) + vs = ViewState(projection=Perspective(1.0, 10.0)) coords = np.array([[0.0, 0.0, 0.0]]) radii = np.array([1.0]) _, _, proj_r = vs.project(coords, radii) @@ -81,7 +81,7 @@ def test_projected_radii_perspective(self): def test_projected_radii_larger_than_point_scale(self): """Silhouette radii should exceed naive r * scale under perspective.""" - vs = ViewState(perspective=1.0, view_distance=10.0) + vs = ViewState(projection=Perspective(1.0, 10.0)) coords = np.array([[0.0, 0.0, 2.0]]) # closer to eye radii = np.array([1.0]) _, _, proj_r = vs.project(coords, radii) @@ -150,11 +150,10 @@ def test_custom_up_vector(self): def test_preserves_other_state(self): """look_along should only change the rotation.""" - vs = ViewState(zoom=2.5, perspective=0.8, view_distance=15.0) + vs = ViewState(zoom=2.5, projection=Perspective(0.8, 15.0)) vs.look_along([1, 1, 0]) assert vs.zoom == 2.5 - assert vs.perspective == 0.8 - assert vs.view_distance == 15.0 + assert vs.projection == Perspective(0.8, 15.0) def test_up_parallel_to_direction_raises(self): """An explicit up vector parallel to the view direction should raise.""" @@ -284,17 +283,13 @@ def test_negative_zoom_raises(self): with pytest.raises(ValueError, match="zoom"): ViewState(zoom=-1.0) - def test_zero_view_distance_raises(self): - with pytest.raises(ValueError, match="view_distance"): - ViewState(view_distance=0.0) - - def test_negative_view_distance_raises(self): - with pytest.raises(ValueError, match="view_distance"): - ViewState(view_distance=-1.0) - def test_valid_view_state_accepted(self): - vs = ViewState(zoom=2.0, view_distance=15.0) + vs = ViewState(zoom=2.0, projection=Perspective(0.5, 15.0)) assert vs.zoom == 2.0 + assert vs.projection.view_distance == 15.0 + + def test_projection_defaults_to_orthographic(self): + assert ViewState().projection == Orthographic() class TestProjectionTypes: diff --git a/tests/test_rendering/test_interactive.py b/tests/test_rendering/test_interactive.py index 7ffcc5ad..c6597575 100644 --- a/tests/test_rendering/test_interactive.py +++ b/tests/test_rendering/test_interactive.py @@ -3,7 +3,7 @@ import numpy as np import pytest -from hofmann.model import RenderStyle, ViewState +from hofmann.model import Orthographic, Perspective, RenderStyle, ViewState from hofmann.rendering.interactive import ( _apply_key_action, _HELP_TEXT, @@ -83,8 +83,7 @@ def _key_action_fixtures(): "rotation": view.rotation.copy(), "zoom": view.zoom, "centre": view.centre.copy(), - "perspective": view.perspective, - "view_distance": view.view_distance, + "projection": view.projection, } return view, style, state, initial_view @@ -205,50 +204,69 @@ def test_pan_down(self): def test_perspective_increase(self): view, style, state, iv = _key_action_fixtures() kind = _do_key("p", view, style, state, iv) - assert view.perspective == pytest.approx(_PERSPECTIVE_STEP) + assert view.projection.strength == pytest.approx(_PERSPECTIVE_STEP) assert kind == "view" def test_perspective_decrease(self): view, style, state, iv = _key_action_fixtures() - view.perspective = 0.5 + view.projection = Perspective(0.5) _do_key("P", view, style, state, iv) - assert view.perspective == pytest.approx(0.5 - _PERSPECTIVE_STEP) + assert view.projection.strength == pytest.approx( + 0.5 - _PERSPECTIVE_STEP + ) def test_perspective_clamped_max(self): view, style, state, iv = _key_action_fixtures() - view.perspective = 0.95 + view.projection = Perspective(0.95) _do_key("p", view, style, state, iv) - assert view.perspective == 1.0 + assert view.projection.strength == 1.0 - def test_perspective_clamped_min(self): + def test_perspective_steps_out_to_orthographic(self): + """The last step down lands on Orthographic, not a tiny strength.""" view, style, state, iv = _key_action_fixtures() - view.perspective = 0.05 + view.projection = Perspective(0.05) _do_key("P", view, style, state, iv) - assert view.perspective == 0.0 + assert view.projection == Orthographic() + + def test_perspective_descent_reaches_orthographic_exactly(self): + """Float residue must not strand the descent just above zero.""" + view, style, state, iv = _key_action_fixtures() + view.projection = Perspective(0.3) + for _ in range(3): + _do_key("P", view, style, state, iv) + assert view.projection == Orthographic() # -- Distance -- def test_distance_increase(self): view, style, state, iv = _key_action_fixtures() - old = view.view_distance + view.projection = Perspective(0.5) + old = view.projection.view_distance kind = _do_key("d", view, style, state, iv) - assert view.view_distance == pytest.approx(old * 1.05) + assert view.projection.view_distance == pytest.approx(old * 1.05) assert kind == "view" def test_distance_decrease(self): view, style, state, iv = _key_action_fixtures() - old = view.view_distance + view.projection = Perspective(0.5) + old = view.projection.view_distance _do_key("D", view, style, state, iv) - assert view.view_distance == pytest.approx(old / 1.05) + assert view.projection.view_distance == pytest.approx(old / 1.05) def test_distance_clamped_min(self): view, style, state, iv = _key_action_fixtures() - view.view_distance = 0.11 - _do_key("D", view, style, state, iv) - # 0.11 / 1.05 ~ 0.1048, still above 0.1 - _do_key("D", view, style, state, iv) - _do_key("D", view, style, state, iv) - assert view.view_distance >= 0.1 + view.projection = Perspective(0.5, 0.11) + # 0.11 / 1.05 ~ 0.1048, still above the floor + for _ in range(3): + _do_key("D", view, style, state, iv) + assert view.projection.view_distance >= 0.1 + + @pytest.mark.parametrize("key", ["P", "d", "D"]) + def test_perspective_keys_are_inert_without_perspective(self, key): + """Only p enters perspective mode; the rest need it already set.""" + view, style, state, iv = _key_action_fixtures() + _do_key(key, view, style, state, iv) + assert view.projection == Orthographic() # -- Style toggles -- @@ -398,14 +416,12 @@ def test_reset_restores_initial_view(self): view.rotation = _rotation_y(1.0) @ view.rotation view.zoom = 3.5 view.centre = np.array([1.0, 2.0, 3.0]) - view.perspective = 0.7 - view.view_distance = 20.0 + view.projection = Perspective(0.7, 20.0) kind = _do_key("r", view, style, state, iv) np.testing.assert_allclose(view.rotation, np.eye(3)) assert view.zoom == 1.0 np.testing.assert_allclose(view.centre, [0.0, 0.0, 0.0]) - assert view.perspective == 0.0 - assert view.view_distance == 10.0 + assert view.projection == Orthographic() assert kind == "view" # -- Help overlay -- diff --git a/tests/test_rendering/test_projection.py b/tests/test_rendering/test_projection.py index e2c1e556..6b8e1eb2 100644 --- a/tests/test_rendering/test_projection.py +++ b/tests/test_rendering/test_projection.py @@ -4,7 +4,14 @@ import numpy as np -from hofmann.model import AtomStyle, Frame, StructureScene, ViewState +from hofmann.model import ( + AtomStyle, + Frame, + Orthographic, + Perspective, + StructureScene, + ViewState, +) from hofmann.model.composition import Composition from hofmann.rendering.projection import ( _make_vacancy_wedge, @@ -23,7 +30,7 @@ def test_orthographic(self): assert s == 1.0 def test_perspective(self): - view = ViewState(perspective=1.0, view_distance=10.0) + view = ViewState(projection=Perspective(1.0, 10.0)) pt = np.array([1.0, 0.0, 0.0]) # depth 0 -> scale = 1 xy, s = _project_point(pt, view) np.testing.assert_allclose(s, 1.0) @@ -44,8 +51,8 @@ def test_perspective_increases_extent(self): ]))], atom_styles={"C": AtomStyle(1.0, (0.5, 0.5, 0.5))}, ) - view_no_persp = ViewState(perspective=0.0) - view_persp = ViewState(perspective=0.5) + view_no_persp = ViewState(projection=Orthographic()) + view_persp = ViewState(projection=Perspective(0.5)) e_no = _scene_extent(scene, view_no_persp, 0, atom_scale=0.5) e_yes = _scene_extent(scene, view_persp, 0, atom_scale=0.5) assert e_yes > e_no From 58127c28ffe8f0d8c0fe41edd366e5d93a36f72b Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 07:27:58 +0100 Subject: [PATCH 03/13] Route every renderer through one camera-to-screen mapping The perspective scale was open-coded in three places: ViewState.project, _project_point, and the cell-edge sub-segment loop. Three copies of one formula can drift apart silently, leaving cell edges or bonds projected differently from the atoms they attach to. Add ViewState.project_camera as the single mapping and route all three through it. Output is unchanged, bit-for-bit. project recomputes the perspective denominator directly for the silhouette radii rather than recovering it from the returned scale: that division round trip is not bit-exact. --- src/hofmann/model/view_state.py | 38 +++++++++++++++++++++++++---- src/hofmann/rendering/cell_edges.py | 24 ++++-------------- src/hofmann/rendering/projection.py | 10 ++------ 3 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index e731af62..6182de1a 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -116,14 +116,13 @@ def project( centred = coords - self.centre rotated = centred @ self.rotation.T depth = rotated[:, 2] + xy, _ = self.project_camera(rotated) match self.projection: case Perspective() as p: - # Denominator of the projection scale, proportional to - # the eye-to-atom distance along z. + # Recomputed rather than recovered as view_distance / + # scale: that division round trip is not bit-exact. d = p.view_distance - depth * p.strength - scale = p.view_distance / d - xy = rotated[:, :2] * scale[:, np.newaxis] * self.zoom if radii is not None: radii = np.asarray(radii, dtype=float) @@ -135,7 +134,6 @@ def project( else: projected_radii = np.zeros(len(depth)) case Orthographic(): - xy = rotated[:, :2] * self.zoom if radii is not None: projected_radii = ( np.asarray(radii, dtype=float) * self.zoom @@ -145,6 +143,36 @@ def project( return xy, depth, projected_radii + def project_camera( + self, camera: np.ndarray, + ) -> tuple[np.ndarray, np.ndarray]: + """Map camera-space positions to screen positions. + + This is the single camera-to-screen mapping: every renderer + obtains screen positions through it, so all drawn geometry + stays consistent with the atoms. + + Args: + camera: Array of shape ``(n, 3)``, already centred and + rotated into camera space. + + Returns: + Tuple of ``(xy, scale)`` where *xy* has shape ``(n, 2)`` + and *scale* has shape ``(n,)``, the perspective scale + applied at each point (all ones under a parallel + projection). + """ + camera = np.asarray(camera, dtype=float) + match self.projection: + case Perspective() as p: + scale = p.view_distance / ( + p.view_distance - camera[:, 2] * p.strength + ) + case Orthographic(): + scale = np.ones(len(camera)) + xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom + return xy, scale + def slab_mask(self, coords: np.ndarray) -> np.ndarray: """Return a boolean mask selecting atoms within the depth slab. diff --git a/src/hofmann/rendering/cell_edges.py b/src/hofmann/rendering/cell_edges.py index a893b01d..d35c19a7 100644 --- a/src/hofmann/rendering/cell_edges.py +++ b/src/hofmann/rendering/cell_edges.py @@ -6,12 +6,7 @@ import numpy as np -from hofmann.model import ( - CellEdgeStyle, - Perspective, - ViewState, - normalise_colour, -) +from hofmann.model import CellEdgeStyle, ViewState, normalise_colour # The 12 edges of a unit cube in fractional coordinates. _CUBE_EDGES: list[tuple[tuple[int, int, int], tuple[int, int, int]]] = [ @@ -310,19 +305,10 @@ def _collect_cell_edges( sub_c_e = c_s + (c_e - c_s) * t1 sub_d = (sub_c_s[2] + sub_c_e[2]) / 2.0 - if isinstance(view.projection, Perspective): - p = view.projection - s_s = p.view_distance / ( - p.view_distance - sub_c_s[2] * p.strength - ) - s_e = p.view_distance / ( - p.view_distance - sub_c_e[2] * p.strength - ) - xy_s_i = sub_c_s[:2] * s_s * view.zoom - xy_e_i = sub_c_e[:2] * s_e * view.zoom - else: - xy_s_i = sub_c_s[:2] * view.zoom - xy_e_i = sub_c_e[:2] * view.zoom + sub_xy, _ = view.project_camera( + np.array([sub_c_s, sub_c_e]) + ) + xy_s_i, xy_e_i = sub_xy[0], sub_xy[1] if dash_pattern is not None: dash_segs = _split_dashes( diff --git a/src/hofmann/rendering/projection.py b/src/hofmann/rendering/projection.py index 2c33d5de..45fe777c 100644 --- a/src/hofmann/rendering/projection.py +++ b/src/hofmann/rendering/projection.py @@ -40,14 +40,8 @@ def _project_point( Tuple of (xy, scale) where *xy* is the 2D position and *scale* is the perspective scale factor at this depth. """ - z = pt[2] - match view.projection: - case Perspective() as p: - s = p.view_distance / (p.view_distance - z * p.strength) - case _: - s = 1.0 - xy = pt[:2] * s * view.zoom - return xy, s + xy, scale = view.project_camera(np.asarray(pt, dtype=float)[np.newaxis]) + return xy[0], float(scale[0]) # Fractional coordinates of the 8 unit cube corners. From 366c5a727d2a6000c8407e247b08dd6edce694de Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 07:30:22 +0100 Subject: [PATCH 04/13] Add projection setters and migrate the docs set_orthographic and set_perspective build the mode from scalars, so selecting a projection needs no import and chains with look_along the way the rest of the camera API does. Assigning a mode value to projection directly stays available for callers that already hold one. --- README.md | 2 +- docs/api.rst | 6 ++++++ docs/changelog.rst | 21 +++++++++++++++++++++ docs/interactive.rst | 2 +- docs/rendering.rst | 8 ++++---- src/hofmann/model/view_state.py | 28 ++++++++++++++++++++++++++++ tests/test_model/test_view_state.py | 18 ++++++++++++++++++ 7 files changed, 79 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 31dd6e1f..bfba4ecc 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ scene.render_mpl("si.pdf") ```python scene.view.look_along([1, 1, 0]) # View along [110] scene.view.zoom = 1.5 # Zoom in -scene.view.perspective = 0.3 # Mild perspective +scene.view.set_perspective(0.3) # Mild perspective scene.render_mpl("rotated.svg") ``` diff --git a/docs/api.rst b/docs/api.rst index 1882dbd7..54cc2751 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -43,6 +43,12 @@ Data model .. autoclass:: ViewState :members: +.. autoclass:: Orthographic + :members: + +.. autoclass:: Perspective + :members: + .. autoclass:: hofmann.model.atom_data.AtomData :members: n_atoms, ranges, labels diff --git a/docs/changelog.rst b/docs/changelog.rst index a7e46718..8d6816e8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,27 @@ Changelog ========= +Unreleased +---------- + +- **Breaking:** Projection mode is now selected with a single + :attr:`~hofmann.ViewState.projection` field taking + :class:`~hofmann.Orthographic` (the default) or + :class:`~hofmann.Perspective`, set with + :meth:`~hofmann.ViewState.set_orthographic` or + :meth:`~hofmann.ViewState.set_perspective`. The ``perspective`` and + ``view_distance`` attributes are removed; ``perspective = 0.3`` + becomes ``set_perspective(0.3)`` and ``perspective = 0.0`` becomes + ``set_orthographic()``. A perspective strength no longer doubles as + an on/off switch, so a viewing distance can no longer be set on a + view that ignores it. + +- In interactive sessions, the ``d`` and ``D`` viewing-distance keys + now act only in perspective mode, where previously they adjusted a + distance that had no effect until perspective was switched on. + ``P`` steps out to an orthographic projection once the strength + reaches the bottom of the ladder. + 0.20.0 ------ diff --git a/docs/interactive.rst b/docs/interactive.rst index e777fe4d..fdea5b3e 100644 --- a/docs/interactive.rst +++ b/docs/interactive.rst @@ -77,7 +77,7 @@ Pan and perspective * - ``p`` / ``P`` - Increase / decrease perspective strength * - ``d`` / ``D`` - - Increase / decrease viewing distance + - Increase / decrease viewing distance (perspective only) Display toggles ^^^^^^^^^^^^^^^ diff --git a/docs/rendering.rst b/docs/rendering.rst index 7084ac7a..5423ff9f 100644 --- a/docs/rendering.rst +++ b/docs/rendering.rst @@ -42,19 +42,19 @@ Perspective .. code-block:: python - scene.view.perspective = 0.3 # Mild perspective - scene.view.perspective = 0.0 # Orthographic (default) + scene.view.set_perspective(0.3) # Mild perspective + scene.view.set_orthographic() # Parallel projection (default) .. list-table:: :widths: 50 50 * - .. figure:: _static/perovskite_ortho.svg - Orthographic (``perspective=0.0``) + Orthographic (the default) - .. figure:: _static/perovskite_perspective.svg - Perspective (``perspective=0.5``) + Perspective (``set_perspective(0.5)``) Render styles diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 6182de1a..2a68b4c2 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -173,6 +173,34 @@ def project_camera( xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom return xy, scale + def set_orthographic(self) -> ViewState: + """Draw without perspective foreshortening. + + Returns: + ``self``, so the call can be chained with + :meth:`look_along`. + """ + self.projection = Orthographic() + return self + + def set_perspective( + self, strength: float = 0.5, view_distance: float = 10.0, + ) -> ViewState: + """Draw with perspective foreshortening. + + Args: + strength: Perspective strength. ``1.0`` is a true pinhole + camera at *view_distance*; smaller values move the eye + further out and weaken the effect. + view_distance: Reference distance from the scene centre. + + Returns: + ``self``, so the call can be chained with + :meth:`look_along`. + """ + self.projection = Perspective(strength, view_distance) + return self + def slab_mask(self, coords: np.ndarray) -> np.ndarray: """Return a boolean mask selecting atoms within the depth slab. diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 0b1f5250..48a6bdbd 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -326,6 +326,24 @@ def test_modes_reject_unknown_attributes(self): with pytest.raises((AttributeError, TypeError)): Orthographic().anything = 1 + def test_setters_return_self_for_chaining(self): + vs = ViewState() + assert vs.set_perspective() is vs + assert vs.set_orthographic() is vs + + def test_setter_defaults_match_the_type(self): + """The two spellings of "default perspective" cannot drift.""" + assert ViewState().set_perspective().projection == Perspective() + + def test_setters_select_the_mode(self): + vs = ViewState().set_perspective(0.8, 15.0) + assert vs.projection == Perspective(0.8, 15.0) + assert vs.set_orthographic().projection == Orthographic() + + def test_setter_validates_through_the_type(self): + with pytest.raises(ValueError, match="strength"): + ViewState().set_perspective(strength=-1.0) + def test_modes_compare_by_value(self): assert Perspective(0.5, 10.0) == Perspective(0.5, 10.0) assert Perspective(0.5, 10.0) != Perspective(0.6, 10.0) From fc10f0728e1a814179563ce05c5a54f9a6f689f9 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 07:33:34 +0100 Subject: [PATCH 05/13] Pin drawn geometry against the shared projection The consolidation is only worth having if it stays consolidated, so assert the property rather than the call sites: every drawn cell-edge endpoint must lie on a cell edge as ViewState projects it, under both projection modes, and _project_point must agree with project_camera. Verified to bite: perturbing the cell-edge projection by 0.01% fails both parametrised cases. --- tests/test_rendering/test_projection.py | 114 ++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/test_rendering/test_projection.py b/tests/test_rendering/test_projection.py index 6b8e1eb2..d3d0460c 100644 --- a/tests/test_rendering/test_projection.py +++ b/tests/test_rendering/test_projection.py @@ -2,7 +2,9 @@ import math +import matplotlib.pyplot as plt import numpy as np +import pytest from hofmann.model import ( AtomStyle, @@ -13,6 +15,7 @@ ViewState, ) from hofmann.model.composition import Composition +from hofmann.rendering.static import render_mpl from hofmann.rendering.projection import ( _make_vacancy_wedge, _make_wedges, @@ -183,3 +186,114 @@ def test_mixed_partial_composition_returns_polygon(self): comp, n_segments_total=24, start_angle=math.pi / 2, ) assert result is not None + + +def _point_to_segment_distance( + point: np.ndarray, start: np.ndarray, end: np.ndarray, +) -> float: + """Shortest distance from *point* to the segment *start*-*end*.""" + seg = end - start + length_sq = float(seg @ seg) + if length_sq == 0.0: + return float(np.linalg.norm(point - start)) + t = float(np.clip((point - start) @ seg / length_sq, 0.0, 1.0)) + return float(np.linalg.norm(point - (start + t * seg))) + + +class TestDrawnGeometryMatchesProjection: + """Every renderer must obtain screen positions from ViewState. + + The perspective scale was once open-coded in the cell-edge loop + and in _project_point as well as in ViewState.project. These + tests pin the agreement, so a copy reintroduced in either place + fails rather than drifting silently. + """ + + @staticmethod + def _scene() -> StructureScene: + """Two tiny atoms in a cell, so edge clipping at spheres is inert.""" + lattice = np.array([ + [4.0, 0.0, 0.0], [0.5, 3.6, 0.0], [0.3, 0.4, 5.2], + ]) + return StructureScene( + species=["A", "B"], + frames=[Frame( + coords=np.array([[1.0, 1.0, 1.0], [2.4, 1.8, 3.0]]), + lattice=lattice, + )], + atom_styles={ + "A": AtomStyle(0.01, (0.5, 0.5, 0.5)), + "B": AtomStyle(0.01, (0.8, 0.2, 0.2)), + }, + ) + + @staticmethod + def _drawn_edge_endpoints(fig) -> np.ndarray: + """Recover segment endpoints from drawn cell-edge rectangles. + + Edges are drawn as [start+offset, end+offset, end-offset, + start-offset], so the midpoints of the short sides recover the + endpoints exactly -- the half-width offsets cancel. With atom + radii this small, 4-vertex polygons are only ever cell edges. + """ + endpoints = [] + for collection in fig.axes[0].collections: + for path in collection.get_paths(): + v = np.asarray(path.vertices) + if len(v) in (4, 5): # 5 = closed polygon repeats vertex 0 + q = v[:4] + endpoints.append((q[0] + q[3]) / 2) + endpoints.append((q[1] + q[2]) / 2) + return np.array(endpoints) + + @pytest.mark.parametrize( + "projection", + [Orthographic(), Perspective(0.6, 12.0)], + ids=["orthographic", "perspective"], + ) + def test_cell_edges_agree_with_view_project(self, projection): + scene = self._scene() + scene.view = ViewState(projection=projection) + scene.view.look_along([1.0, 0.6, 0.4]) + + fig = render_mpl(scene, show=False) + drawn = self._drawn_edge_endpoints(fig) + plt.close(fig) + + fracs = np.array([ + [0, 0, 0], [1, 0, 0], [0, 1, 0], [1, 1, 0], + [0, 0, 1], [1, 0, 1], [0, 1, 1], [1, 1, 1], + ], dtype=float) + corners = fracs @ scene.frames[0].lattice + projected, _, _ = scene.view.project(corners) + # Cell edges join corners differing in one fractional + # coordinate. Edges are subdivided for depth sorting, so a + # drawn endpoint is generally interior to an edge -- but a + # projection maps straight lines to straight lines, so it must + # still lie on the projected edge. + edges = [ + (i, j) + for i in range(8) for j in range(i + 1, 8) + if np.sum(fracs[i] != fracs[j]) == 1 + ] + assert len(edges) == 12 + + assert len(drawn) > 0 + for point in drawn: + gap = min( + _point_to_segment_distance( + point, projected[i], projected[j], + ) + for i, j in edges + ) + assert gap < 1e-9, f"drawn endpoint {point} lies off every edge" + + def test_project_point_agrees_with_project_camera(self): + """_project_point is a scalar view of the same mapping.""" + view = ViewState(zoom=1.4, projection=Perspective(0.7, 9.0)) + camera = np.array([[1.0, -2.0, 3.0], [0.5, 0.25, -4.0]]) + batch_xy, batch_scale = view.project_camera(camera) + for i, point in enumerate(camera): + xy, scale = _project_point(point, view) + np.testing.assert_array_equal(xy, batch_xy[i]) + assert scale == batch_scale[i] From b05c64b826f2c7c9ec8177136232c3bfb09815be Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 08:11:37 +0100 Subject: [PATCH 06/13] Apply cleanup review findings Extract _eye_distance in bond_geometry, replacing a five-line conditional duplicated between the scalar and batch paths and naming the 1e6 parallel stand-in that the sum type finally makes expressible. Hoist the radii check out of project's match, so the match dispatches on the projection alone rather than on a 2x2 of mode and radii. Forward set_perspective's defaults to Perspective's own, and drop the test that existed only to police drift between the two copies. Guard both total matches with assert_never: adding a third variant now fails mypy at every unhandled site rather than raising UnboundLocalError at runtime. Reuse the renderer's _cell_edges_3d in the drawn-geometry test instead of rebuilding the corner and edge tables, migrate the last four un-migrated assignments in the docs image generator, and alphabetise the new exports. --- .gitignore | 3 ++ docs/_static/generate_images.py | 7 ++-- src/hofmann/__init__.py | 6 +-- src/hofmann/model/__init__.py | 2 +- src/hofmann/model/view_state.py | 51 ++++++++++++++----------- src/hofmann/rendering/bond_geometry.py | 36 +++++++++-------- src/hofmann/rendering/projection.py | 6 +-- tests/test_model/test_view_state.py | 6 +-- tests/test_rendering/test_projection.py | 34 ++++++----------- 9 files changed, 75 insertions(+), 76 deletions(-) diff --git a/.gitignore b/.gitignore index 0058ab52..139e0885 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,9 @@ htmlcov/ # Planning docs RESTRUCTURE.md PLAN-*.md +OBLIQUE_PROJECTION.md +.superpowers/ +docs/superpowers/ # OS .DS_Store diff --git a/docs/_static/generate_images.py b/docs/_static/generate_images.py index 479d196e..ba6e5392 100644 --- a/docs/_static/generate_images.py +++ b/docs/_static/generate_images.py @@ -411,8 +411,7 @@ def logo_scene() -> StructureScene: scene.set_atom_data("gradient", by_index=data) scene.view.look_along(look_dir) - scene.view.perspective = 0.12 - scene.view.view_distance = 5.0 + scene.view.set_perspective(0.12, 5.0) return scene @@ -652,13 +651,13 @@ def generate_docs_images() -> None: figsize=(3, 3), dpi=150, ) print(f" wrote {OUT / 'perovskite_ortho.svg'}") - perov_plain.view.perspective = 0.5 + perov_plain.view.set_perspective(0.5) perov_plain.render_mpl( OUT / "perovskite_perspective.svg", figsize=(3, 3), dpi=150, ) print(f" wrote {OUT / 'perovskite_perspective.svg'}") - perov_plain.view.perspective = 0.0 # Reset + perov_plain.view.set_orthographic() # Reset # 6–9. Per-atom colouring examples: ring of atoms. n = 16 diff --git a/src/hofmann/__init__.py b/src/hofmann/__init__.py index f42fe82c..082d06f7 100644 --- a/src/hofmann/__init__.py +++ b/src/hofmann/__init__.py @@ -38,9 +38,9 @@ PolyhedronSpec, RenderStyle, SlabClipMode, - StructureScene, Orthographic, Perspective, + StructureScene, ViewState, WidgetCorner, normalise_colour, @@ -65,14 +65,14 @@ "LegendStyle", "Polyhedron", "PolygonLegendItem", + "Orthographic", + "Perspective", "PolyhedronLegendItem", "PolyhedronSpec", "RenderStyle", "SlabClipMode", "StructureScene", "StyleSet", - "Orthographic", - "Perspective", "ViewState", "WidgetCorner", "compute_bonds", diff --git a/src/hofmann/model/__init__.py b/src/hofmann/model/__init__.py index 776c33be..3114907b 100644 --- a/src/hofmann/model/__init__.py +++ b/src/hofmann/model/__init__.py @@ -51,9 +51,9 @@ "PolyhedronSpec", "RenderStyle", "SlabClipMode", - "StructureScene", "Orthographic", "Perspective", + "StructureScene", "ViewState", "WidgetCorner", "normalise_colour", diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 2a68b4c2..e3137065 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -2,6 +2,7 @@ import math from dataclasses import dataclass, field +from typing import assert_never import numpy as np @@ -44,6 +45,10 @@ def __post_init__(self) -> None: ) +#: Default perspective, so the setter and the type cannot drift apart. +_DEFAULT_PERSPECTIVE = Perspective() + + @dataclass class ViewState: """Camera state for 3D-to-2D projection. @@ -96,8 +101,9 @@ def project( ) -> tuple[np.ndarray, np.ndarray, np.ndarray]: """Project 3D coordinates to 2D with depth information. - The eye sits at ``[0, 0, view_distance]`` and each sphere's - visible silhouette is projected onto the z=0 plane. + Under :class:`Perspective` the eye sits on the camera's +z + axis and each sphere's visible silhouette is projected onto + the z=0 plane. Args: coords: Array of shape ``(n, 3)``. @@ -118,28 +124,22 @@ def project( depth = rotated[:, 2] xy, _ = self.project_camera(rotated) + if radii is None: + return xy, depth, np.zeros(len(depth)) + + radii = np.asarray(radii, dtype=float) match self.projection: case Perspective() as p: # Recomputed rather than recovered as view_distance / # scale: that division round trip is not bit-exact. d = p.view_distance - depth * p.strength - - if radii is not None: - radii = np.asarray(radii, dtype=float) - # Silhouette radius: r * D / sqrt(d^2 - r^2). - denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) - projected_radii = ( - radii * p.view_distance / denom * self.zoom - ) - else: - projected_radii = np.zeros(len(depth)) + # Silhouette radius: r * D / sqrt(d^2 - r^2). + denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) + projected_radii = radii * p.view_distance / denom * self.zoom case Orthographic(): - if radii is not None: - projected_radii = ( - np.asarray(radii, dtype=float) * self.zoom - ) - else: - projected_radii = np.zeros(len(depth)) + projected_radii = radii * self.zoom + case _: + assert_never(self.projection) return xy, depth, projected_radii @@ -148,9 +148,12 @@ def project_camera( ) -> tuple[np.ndarray, np.ndarray]: """Map camera-space positions to screen positions. - This is the single camera-to-screen mapping: every renderer - obtains screen positions through it, so all drawn geometry - stays consistent with the atoms. + This is the single camera-to-screen mapping for scene + geometry -- atoms, bonds, and cell edges all obtain screen + positions through it, so they stay consistent with each other. + Fixed-size screen furniture that deliberately ignores + :attr:`zoom`, such as the axes orientation widget, maps + directions itself and does not come through here. Args: camera: Array of shape ``(n, 3)``, already centred and @@ -170,6 +173,8 @@ def project_camera( ) case Orthographic(): scale = np.ones(len(camera)) + case _: + assert_never(self.projection) xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom return xy, scale @@ -184,7 +189,9 @@ def set_orthographic(self) -> ViewState: return self def set_perspective( - self, strength: float = 0.5, view_distance: float = 10.0, + self, + strength: float = _DEFAULT_PERSPECTIVE.strength, + view_distance: float = _DEFAULT_PERSPECTIVE.view_distance, ) -> ViewState: """Draw with perspective foreshortening. diff --git a/src/hofmann/rendering/bond_geometry.py b/src/hofmann/rendering/bond_geometry.py index ebcb311c..dad28a8e 100644 --- a/src/hofmann/rendering/bond_geometry.py +++ b/src/hofmann/rendering/bond_geometry.py @@ -4,9 +4,27 @@ import numpy as np -from hofmann.model import Perspective, ViewState +from hofmann.model import Orthographic, Perspective, ViewState from hofmann.rendering.projection import _project_point +#: Stand-in eye distance for parallel projections: far enough that all +#: view rays are effectively parallel (matching XBS pmode == 0). +_PARALLEL_EYE_DISTANCE = 1e6 + + +def _eye_distance(view: ViewState) -> float: + """Distance along +z from the scene centre to the eye. + + A parallel projection has no eye, so it takes a stand-in far + enough away that the view rays it produces are parallel to + within rounding. + """ + match view.projection: + case Perspective() as p: + return p.view_distance + case Orthographic(): + return _PARALLEL_EYE_DISTANCE + def _clip_bond_3d( p_a: np.ndarray, @@ -186,14 +204,7 @@ def _bond_polygon( # vector. This determines how much the arc squashes along the bond # direction (a bond pointing at the viewer has cth~1, one # perpendicular to the view has cth~0). - # For orthographic projection, push the eye to effective infinity - # so all view rays are parallel (matching XBS pmode==0 behaviour). - eye_dist = ( - view.projection.view_distance - if isinstance(view.projection, Perspective) - else 1e6 - ) - eye = np.array([0.0, 0.0, eye_dist]) + eye = np.array([0.0, 0.0, _eye_distance(view)]) q_a = eye - p_a q_b = eye - p_b denom_a = np.linalg.norm(q_a) * bond_len @@ -335,12 +346,7 @@ def _bond_polygons_batch( valid &= (bond_len_safe - w_a - w_b) > 0 # Foreshortening angles. - eye_dist = ( - view.projection.view_distance - if isinstance(view.projection, Perspective) - else 1e6 - ) - eye = np.array([0.0, 0.0, eye_dist]) + eye = np.array([0.0, 0.0, _eye_distance(view)]) q_a = eye - p_a # (n_bonds, 3) q_b = eye - p_b # (n_bonds, 3) q_a_len = np.linalg.norm(q_a, axis=1) # (n_bonds,) diff --git a/src/hofmann/rendering/projection.py b/src/hofmann/rendering/projection.py index 45fe777c..39f8044c 100644 --- a/src/hofmann/rendering/projection.py +++ b/src/hofmann/rendering/projection.py @@ -98,10 +98,8 @@ def _scene_extent( p = view.projection worst_depth = float(np.max(dists)) denom = p.view_distance - worst_depth * p.strength - if denom > 0: - persp_scale = p.view_distance / denom - else: - persp_scale = p.view_distance / 1e-6 + # Not max(denom, 1e-6): that would alter 0 < denom < 1e-6. + persp_scale = p.view_distance / (denom if denom > 0 else 1e-6) max_extent *= persp_scale return float(max_extent * view.zoom) diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 48a6bdbd..52acc005 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -299,7 +299,7 @@ def test_perspective_defaults(self): assert p.view_distance == 10.0 @pytest.mark.parametrize( - "strength", [0.0, -0.5, float("nan"), float("inf")], + "strength", [-0.5, float("nan"), float("inf")], ) def test_invalid_strength_rejected(self, strength): with pytest.raises(ValueError, match="strength"): @@ -331,10 +331,6 @@ def test_setters_return_self_for_chaining(self): assert vs.set_perspective() is vs assert vs.set_orthographic() is vs - def test_setter_defaults_match_the_type(self): - """The two spellings of "default perspective" cannot drift.""" - assert ViewState().set_perspective().projection == Perspective() - def test_setters_select_the_mode(self): vs = ViewState().set_perspective(0.8, 15.0) assert vs.projection == Perspective(0.8, 15.0) diff --git a/tests/test_rendering/test_projection.py b/tests/test_rendering/test_projection.py index d3d0460c..5bcc5914 100644 --- a/tests/test_rendering/test_projection.py +++ b/tests/test_rendering/test_projection.py @@ -15,13 +15,14 @@ ViewState, ) from hofmann.model.composition import Composition -from hofmann.rendering.static import render_mpl +from hofmann.rendering.cell_edges import _cell_edges_3d from hofmann.rendering.projection import ( _make_vacancy_wedge, _make_wedges, _project_point, _scene_extent, ) +from hofmann.rendering.static import render_mpl class TestProjectPoint: @@ -260,31 +261,20 @@ def test_cell_edges_agree_with_view_project(self, projection): drawn = self._drawn_edge_endpoints(fig) plt.close(fig) - fracs = np.array([ - [0, 0, 0], [1, 0, 0], [0, 1, 0], [1, 1, 0], - [0, 0, 1], [1, 0, 1], [0, 1, 1], [1, 1, 1], - ], dtype=float) - corners = fracs @ scene.frames[0].lattice - projected, _, _ = scene.view.project(corners) - # Cell edges join corners differing in one fractional - # coordinate. Edges are subdivided for depth sorting, so a - # drawn endpoint is generally interior to an edge -- but a - # projection maps straight lines to straight lines, so it must - # still lie on the projected edge. - edges = [ - (i, j) - for i in range(8) for j in range(i + 1, 8) - if np.sum(fracs[i] != fracs[j]) == 1 - ] - assert len(edges) == 12 + # The renderer's own edge set, so the test cannot drift from + # the geometry it checks. Edges are subdivided for depth + # sorting, so a drawn endpoint is generally interior to an + # edge -- but a projection maps straight lines to straight + # lines, so it must still lie on the projected edge. + starts, ends = _cell_edges_3d(scene.frames[0].lattice) + projected, _, _ = scene.view.project(np.vstack([starts, ends])) + edges = list(zip(projected[: len(starts)], projected[len(starts):])) assert len(drawn) > 0 for point in drawn: gap = min( - _point_to_segment_distance( - point, projected[i], projected[j], - ) - for i, j in edges + _point_to_segment_distance(point, start, end) + for start, end in edges ) assert gap < 1e-9, f"drawn endpoint {point} lies off every edge" From 2fde91fc4b36be74a28bb50fa2bde46207ebd530 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 08:15:53 +0100 Subject: [PATCH 07/13] Project cell-edge sub-segments in one call The sub-segment loop projected each endpoint pair separately, which made the consolidation cost 0.35 ms per cell-edge pass on a 216-atom scene and left a Python loop doing work numpy can do in one call. Interpolating and projecting all of an edge's sub-segments together measures 2.52 ms against 2.99 ms on main and 3.33 ms per-pair, and the drawn output is unchanged bit-for-bit. Also skip the multiply by a column of ones under a parallel projection, which is 11% of ViewState.project at ten thousand atoms. --- src/hofmann/model/view_state.py | 5 ++++- src/hofmann/rendering/cell_edges.py | 18 +++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index e3137065..2aa1ff19 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -171,11 +171,14 @@ def project_camera( scale = p.view_distance / ( p.view_distance - camera[:, 2] * p.strength ) + xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom case Orthographic(): + # Scale is reported as ones, but not multiplied + # through: a parallel projection does not scale. scale = np.ones(len(camera)) + xy = camera[:, :2] * self.zoom case _: assert_never(self.projection) - xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom return xy, scale def set_orthographic(self) -> ViewState: diff --git a/src/hofmann/rendering/cell_edges.py b/src/hofmann/rendering/cell_edges.py index d35c19a7..a60c1945 100644 --- a/src/hofmann/rendering/cell_edges.py +++ b/src/hofmann/rendering/cell_edges.py @@ -300,15 +300,15 @@ def _collect_cell_edges( if boundaries[i + 1] - boundaries[i] > 1e-12 ] - for t0, t1 in sub_fracs: - sub_c_s = c_s + (c_e - c_s) * t0 - sub_c_e = c_s + (c_e - c_s) * t1 - sub_d = (sub_c_s[2] + sub_c_e[2]) / 2.0 - - sub_xy, _ = view.project_camera( - np.array([sub_c_s, sub_c_e]) - ) - xy_s_i, xy_e_i = sub_xy[0], sub_xy[1] + # Project every sub-segment endpoint of this edge in one + # call: one (2k, 3) array rather than k two-row ones. + fracs = np.asarray(sub_fracs, dtype=float).ravel() + sub_c = c_s + (c_e - c_s) * fracs[:, np.newaxis] + sub_xy = view.project_camera(sub_c)[0].reshape(-1, 2, 2) + sub_depths = sub_c[:, 2].reshape(-1, 2) + + for (xy_s_i, xy_e_i), (z_s, z_e) in zip(sub_xy, sub_depths): + sub_d = (z_s + z_e) / 2.0 if dash_pattern is not None: dash_segs = _split_dashes( From 6b7af25f17a8e8fffd911e1becef7120762b6980 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 09:02:08 +0100 Subject: [PATCH 08/13] Address review findings on the projection sum type Preserve the viewing distance across an orthographic excursion. The sum type correctly makes view_distance unrepresentable while parallel, but nothing then remembered it, so P-to-orthographic followed by p silently reset a dialled-in distance to the default. The session state now carries the outgoing Perspective, which is where per-session UI memory already lives. P, d and D return "none" under a parallel projection rather than forcing a redraw for a guaranteed no-op, matching the file's own idiom. Cover the gaps behind three green-but-empty tests, each confirmed by mutation: zoom was never exercised under perspective, so deleting it from project_camera passed; the perspective floor was pinned from a start value whose residue was negative, so a bare > 0.0 passed; and sub-segment depths could be paired with the wrong screen positions, which the point-on-segment check cannot see, being invariant under permutation. Rename _eye_distance to _foreshortening_distance: it returns view_distance, but the eye sits at view_distance / strength, so the old name asserted something this branch's own Perspective docstring contradicts. The silhouette radius carries the same approximation and now says so at the line. Close the projection sum at _scene_extent, export a Projection alias for the union, document the view_distance migration, and correct the comment nits. --- docs/changelog.rst | 7 ++- src/hofmann/__init__.py | 2 + src/hofmann/model/__init__.py | 8 ++- src/hofmann/model/view_state.py | 16 ++++-- src/hofmann/rendering/bond_geometry.py | 22 +++++--- src/hofmann/rendering/interactive.py | 31 +++++++++-- src/hofmann/rendering/projection.py | 27 +++++++--- tests/test_model/test_view_state.py | 20 ++++++- tests/test_rendering/test_interactive.py | 26 +++++++-- tests/test_rendering/test_projection.py | 69 ++++++++++++++++++++++++ 10 files changed, 195 insertions(+), 33 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8d6816e8..27f80774 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,7 +12,9 @@ Unreleased :meth:`~hofmann.ViewState.set_perspective`. The ``perspective`` and ``view_distance`` attributes are removed; ``perspective = 0.3`` becomes ``set_perspective(0.3)`` and ``perspective = 0.0`` becomes - ``set_orthographic()``. A perspective strength no longer doubles as + ``set_orthographic()``. A ``view_distance`` set alongside a + perspective strength moves into the same call: + ``set_perspective(0.3, 5.0)``. A perspective strength no longer doubles as an on/off switch, so a viewing distance can no longer be set on a view that ignores it. @@ -20,7 +22,8 @@ Unreleased now act only in perspective mode, where previously they adjusted a distance that had no effect until perspective was switched on. ``P`` steps out to an orthographic projection once the strength - reaches the bottom of the ladder. + reaches the bottom of the ladder, and ``p`` restores the viewing + distance the session was last using rather than resetting it. 0.20.0 ------ diff --git a/src/hofmann/__init__.py b/src/hofmann/__init__.py index 082d06f7..ce5dafad 100644 --- a/src/hofmann/__init__.py +++ b/src/hofmann/__init__.py @@ -40,6 +40,7 @@ SlabClipMode, Orthographic, Perspective, + Projection, StructureScene, ViewState, WidgetCorner, @@ -69,6 +70,7 @@ "Perspective", "PolyhedronLegendItem", "PolyhedronSpec", + "Projection", "RenderStyle", "SlabClipMode", "StructureScene", diff --git a/src/hofmann/model/__init__.py b/src/hofmann/model/__init__.py index 3114907b..dbc8b0eb 100644 --- a/src/hofmann/model/__init__.py +++ b/src/hofmann/model/__init__.py @@ -30,7 +30,12 @@ _DEFAULT_SPACING, ) from hofmann.model.structure_scene import StructureScene -from hofmann.model.view_state import Orthographic, Perspective, ViewState +from hofmann.model.view_state import ( + Orthographic, + Perspective, + Projection, + ViewState, +) __all__ = [ "AtomLegendItem", @@ -53,6 +58,7 @@ "SlabClipMode", "Orthographic", "Perspective", + "Projection", "StructureScene", "ViewState", "WidgetCorner", diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 2aa1ff19..013a06e9 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -17,8 +17,8 @@ class Perspective: """Perspective projection with the eye on the camera's +z axis. Screen positions are scaled by ``D / (D - z * s)`` for an atom at - camera depth *z*, which places the eye at ``view_distance / - strength``. A *strength* of ``1.0`` is therefore a true pinhole + camera depth *z*, writing *D* for :attr:`view_distance` and *s* + for :attr:`strength`. That places the eye at ``D / s``. A *strength* of ``1.0`` is therefore a true pinhole camera at :attr:`view_distance`; smaller values move the eye further out, weakening the foreshortening. @@ -45,6 +45,9 @@ def __post_init__(self) -> None: ) +#: The projection modes, as a single name for annotations. +Projection = Orthographic | Perspective + #: Default perspective, so the setter and the type cannot drift apart. _DEFAULT_PERSPECTIVE = Perspective() @@ -85,7 +88,7 @@ class ViewState: centre: np.ndarray = field( default_factory=lambda: np.zeros(3, dtype=float) ) - projection: Orthographic | Perspective = field( + projection: Projection = field( default_factory=Orthographic ) slab_origin: np.ndarray | None = None @@ -134,6 +137,9 @@ def project( # scale: that division round trip is not bit-exact. d = p.view_distance - depth * p.strength # Silhouette radius: r * D / sqrt(d^2 - r^2). + # Exact for an eye at D; the eye is at D / strength, + # for which the exact form carries (r * strength)^2. + # The two agree at full strength. denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) projected_radii = radii * p.view_distance / denom * self.zoom case Orthographic(): @@ -173,8 +179,8 @@ def project_camera( ) xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom case Orthographic(): - # Scale is reported as ones, but not multiplied - # through: a parallel projection does not scale. + # Reported as ones for the caller, but not applied: + # a parallel projection does not scale with depth. scale = np.ones(len(camera)) xy = camera[:, :2] * self.zoom case _: diff --git a/src/hofmann/rendering/bond_geometry.py b/src/hofmann/rendering/bond_geometry.py index dad28a8e..c129f514 100644 --- a/src/hofmann/rendering/bond_geometry.py +++ b/src/hofmann/rendering/bond_geometry.py @@ -12,12 +12,18 @@ _PARALLEL_EYE_DISTANCE = 1e6 -def _eye_distance(view: ViewState) -> float: - """Distance along +z from the scene centre to the eye. - - A parallel projection has no eye, so it takes a stand-in far - enough away that the view rays it produces are parallel to - within rounding. +def _foreshortening_distance(view: ViewState) -> float: + """Reference distance used to foreshorten bond end caps. + + Under :class:`Perspective` this is ``view_distance``, which is the + true eye position only at full strength: the eye actually sits at + ``view_distance / strength``, so the caps are foreshortened + towards a nearer point than the atoms are projected from at lower + strengths. + + A parallel projection has no eye at all, so it takes a stand-in + far enough away that the resulting view rays are parallel to + within a few parts in a million over a typical structure. """ match view.projection: case Perspective() as p: @@ -204,7 +210,7 @@ def _bond_polygon( # vector. This determines how much the arc squashes along the bond # direction (a bond pointing at the viewer has cth~1, one # perpendicular to the view has cth~0). - eye = np.array([0.0, 0.0, _eye_distance(view)]) + eye = np.array([0.0, 0.0, _foreshortening_distance(view)]) q_a = eye - p_a q_b = eye - p_b denom_a = np.linalg.norm(q_a) * bond_len @@ -346,7 +352,7 @@ def _bond_polygons_batch( valid &= (bond_len_safe - w_a - w_b) > 0 # Foreshortening angles. - eye = np.array([0.0, 0.0, _eye_distance(view)]) + eye = np.array([0.0, 0.0, _foreshortening_distance(view)]) q_a = eye - p_a # (n_bonds, 3) q_b = eye - p_b # (n_bonds, 3) q_a_len = np.linalg.norm(q_a, axis=1) # (n_bonds,) diff --git a/src/hofmann/rendering/interactive.py b/src/hofmann/rendering/interactive.py index f78069f5..3d8049fc 100644 --- a/src/hofmann/rendering/interactive.py +++ b/src/hofmann/rendering/interactive.py @@ -69,12 +69,15 @@ def _rotation_z(angle: float) -> np.ndarray: _PERSPECTIVE_STEP = 0.1 # perspective increment per key press _PERSPECTIVE_FLOOR = 1e-9 # below this, the descent lands on Orthographic _DISTANCE_FACTOR = 1.05 # viewing distance multiplier per key press -_MIN_VIEW_DISTANCE = 0.1 # closest the eye may be brought to the centre +_MIN_VIEW_DISTANCE = 0.1 # floor on Perspective.view_distance + +#: Seeds the p key when no perspective has been used this session. +_DEFAULT_INTERACTIVE_PERSPECTIVE = Perspective(strength=_PERSPECTIVE_STEP) _HELP_TEXT = """\ Arrows Rotate Shift+Arrows Pan , . Roll + = - Zoom -p P Perspective d D Distance +p P Perspective d D Distance (persp) b Bonds o Outlines e Polyhedra u Unit cell a Axes r Reset view @@ -179,7 +182,12 @@ def _apply_key_action( strength=min(1.0, proj.strength + _PERSPECTIVE_STEP), ) case _: - view.projection = Perspective(strength=_PERSPECTIVE_STEP) + # A parallel projection cannot hold a viewing distance, + # so re-entering perspective restores the one the + # session last used rather than resetting it. + view.projection = replace( + state["last_perspective"], strength=_PERSPECTIVE_STEP, + ) elif key == "P": match view.projection: case Perspective() as proj: @@ -190,13 +198,18 @@ def _apply_key_action( if strength > _PERSPECTIVE_FLOOR: view.projection = replace(proj, strength=strength) else: + state["last_perspective"] = proj view.projection = Orthographic() + case _: + return "none" elif key == "d": match view.projection: case Perspective() as proj: view.projection = replace( proj, view_distance=proj.view_distance * _DISTANCE_FACTOR, ) + case _: + return "none" elif key == "D": match view.projection: case Perspective() as proj: @@ -207,6 +220,8 @@ def _apply_key_action( proj.view_distance / _DISTANCE_FACTOR, ), ) + case _: + return "none" # -- Style toggles (no recomputation needed) -- elif key == "b": @@ -305,7 +320,8 @@ def render_mpl_interactive( - **+** / **=** / **-** zoom in/out. - **Shift+Arrow** keys pan the view. - **p** / **P** increase/decrease perspective strength. - - **d** / **D** increase/decrease viewing distance. + - **d** / **D** increase/decrease viewing distance + (perspective mode only). - **b** toggle bonds, **o** toggle outlines, **e** toggle polyhedra, **u** toggle unit cell, **a** toggle axes widget. - **[** / **]** step to the previous/next frame; @@ -419,6 +435,13 @@ def render_mpl_interactive( "input_mode": None, "input_buffer": "", "indicator_visible": False, + # Carries the viewing distance across an orthographic + # excursion, which cannot represent one. + "last_perspective": ( + scene.view.projection + if isinstance(scene.view.projection, Perspective) + else _DEFAULT_INTERACTIVE_PERSPECTIVE + ), } # Snapshot for the reset key. diff --git a/src/hofmann/rendering/projection.py b/src/hofmann/rendering/projection.py index 39f8044c..ae0e11d7 100644 --- a/src/hofmann/rendering/projection.py +++ b/src/hofmann/rendering/projection.py @@ -4,7 +4,14 @@ import numpy as np -from hofmann.model import Perspective, StructureScene, ViewState +from typing import assert_never + +from hofmann.model import ( + Orthographic, + Perspective, + StructureScene, + ViewState, +) from hofmann.model.composition import Composition, _OCCUPANCY_TOLERANCE from hofmann.rendering.precompute import _compute_atom_radii @@ -94,13 +101,17 @@ def _scene_extent( # worst-case magnification for an atom at distance *d* from the # view centre is when it is rotated to depth z = +d (closest to # the camera). - if isinstance(view.projection, Perspective) and len(dists) > 0: - p = view.projection - worst_depth = float(np.max(dists)) - denom = p.view_distance - worst_depth * p.strength - # Not max(denom, 1e-6): that would alter 0 < denom < 1e-6. - persp_scale = p.view_distance / (denom if denom > 0 else 1e-6) - max_extent *= persp_scale + match view.projection: + case Perspective() as p if len(dists) > 0: + worst_depth = float(np.max(dists)) + denom = p.view_distance - worst_depth * p.strength + # Not max(denom, 1e-6): that would alter 0 < denom < 1e-6. + persp_scale = p.view_distance / (denom if denom > 0 else 1e-6) + max_extent *= persp_scale + case Perspective() | Orthographic(): + pass + case _: + assert_never(view.projection) return float(max_extent * view.zoom) diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 52acc005..7cccd6e5 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -79,6 +79,24 @@ def test_projected_radii_perspective(self): expected = 10.0 / np.sqrt(99.0) np.testing.assert_allclose(proj_r, [expected], rtol=1e-6) + def test_zoom_scales_perspective_positions(self): + """Zoom must apply under perspective, not only orthographic.""" + coords = np.array([[1.0, 2.0, 3.0]]) + plain = ViewState(projection=Perspective(0.7, 12.0)) + zoomed = ViewState(zoom=2.5, projection=Perspective(0.7, 12.0)) + xy_plain, _, _ = plain.project(coords) + xy_zoomed, _, _ = zoomed.project(coords) + np.testing.assert_allclose(xy_zoomed, xy_plain * 2.5) + + def test_zoom_scales_perspective_radii(self): + coords = np.array([[0.0, 0.0, 1.0]]) + radii = np.array([0.8]) + plain = ViewState(projection=Perspective(0.7, 12.0)) + zoomed = ViewState(zoom=2.5, projection=Perspective(0.7, 12.0)) + _, _, r_plain = plain.project(coords, radii) + _, _, r_zoomed = zoomed.project(coords, radii) + np.testing.assert_allclose(r_zoomed, r_plain * 2.5) + def test_projected_radii_larger_than_point_scale(self): """Silhouette radii should exceed naive r * scale under perspective.""" vs = ViewState(projection=Perspective(1.0, 10.0)) @@ -323,7 +341,7 @@ def test_modes_are_frozen(self): def test_modes_reject_unknown_attributes(self): """slots keeps a typo loud rather than setting an inert attribute.""" - with pytest.raises((AttributeError, TypeError)): + with pytest.raises(TypeError): Orthographic().anything = 1 def test_setters_return_self_for_chaining(self): diff --git a/tests/test_rendering/test_interactive.py b/tests/test_rendering/test_interactive.py index c6597575..a185fb6f 100644 --- a/tests/test_rendering/test_interactive.py +++ b/tests/test_rendering/test_interactive.py @@ -78,6 +78,7 @@ def _key_action_fixtures(): "indicator_visible": False, "input_mode": None, "input_buffer": "", + "last_perspective": Perspective(strength=0.1), } initial_view = { "rotation": view.rotation.copy(), @@ -229,18 +230,35 @@ def test_perspective_steps_out_to_orthographic(self): assert view.projection == Orthographic() def test_perspective_descent_reaches_orthographic_exactly(self): - """Float residue must not strand the descent just above zero.""" + """Float residue must not strand the descent just above zero. + + 0.4 is chosen deliberately: repeated subtraction leaves a + positive residue (~+2.8e-17), which a bare ``> 0.0`` test would + keep as a Perspective. 0.3 leaves a negative residue and so + would pass either way. + """ + view, style, state, iv = _key_action_fixtures() + view.projection = Perspective(0.4) + for _ in range(4): + _do_key("P", view, style, state, iv) + assert view.projection == Orthographic() + + def test_view_distance_survives_an_orthographic_excursion(self): + """A parallel projection cannot hold a distance; the session can.""" view, style, state, iv = _key_action_fixtures() - view.projection = Perspective(0.3) + view.projection = Perspective(0.3, 25.0) + state["last_perspective"] = view.projection for _ in range(3): _do_key("P", view, style, state, iv) assert view.projection == Orthographic() + _do_key("p", view, style, state, iv) + assert view.projection.view_distance == 25.0 # -- Distance -- def test_distance_increase(self): view, style, state, iv = _key_action_fixtures() - view.projection = Perspective(0.5) + view.projection = Perspective(0.7, 20.0) old = view.projection.view_distance kind = _do_key("d", view, style, state, iv) assert view.projection.view_distance == pytest.approx(old * 1.05) @@ -248,7 +266,7 @@ def test_distance_increase(self): def test_distance_decrease(self): view, style, state, iv = _key_action_fixtures() - view.projection = Perspective(0.5) + view.projection = Perspective(0.7, 20.0) old = view.projection.view_distance _do_key("D", view, style, state, iv) assert view.projection.view_distance == pytest.approx(old / 1.05) diff --git a/tests/test_rendering/test_projection.py b/tests/test_rendering/test_projection.py index 5bcc5914..7fd8bd52 100644 --- a/tests/test_rendering/test_projection.py +++ b/tests/test_rendering/test_projection.py @@ -287,3 +287,72 @@ def test_project_point_agrees_with_project_camera(self): xy, scale = _project_point(point, view) np.testing.assert_array_equal(xy, batch_xy[i]) assert scale == batch_scale[i] + + +class TestCellEdgeSubSegmentPairing: + """Sub-segment screen positions must stay paired with their depths. + + Edges are split at atom depths and each piece is filed in the depth + slot its own midpoint falls into, so atoms occlude the pieces + behind them and not the ones in front. Projecting a whole edge's + endpoints in one call makes it possible to pair a piece with + another piece's depth; the point-on-segment check above cannot see + that, being invariant under permuting the pieces. + """ + + def test_piece_depth_matches_its_own_screen_position(self): + """Recover each piece's depth from its geometry, independently.""" + from hofmann.model import CellEdgeStyle + from hofmann.rendering.cell_edges import ( + _cell_edges_3d, + _collect_cell_edges, + ) + + lattice = np.eye(3) * 12.0 + # Orthographic: screen position maps back to camera x/y exactly, + # so a piece's depth can be recovered from where it was drawn. + view = ViewState() + view.look_along([1.0, 0.6, 0.4]) + g = np.arange(3) * 5.0 + xs, ys, zs = np.meshgrid(g, g, g, indexing="ij") + coords = np.column_stack([xs.ravel(), ys.ravel(), zs.ravel()]) + depth = coords @ view.rotation[2] + + by_slot = _collect_cell_edges( + lattice=lattice, view=view, cell_style=CellEdgeStyle(), + depth=depth, order=np.argsort(depth), pad=30.0, coords=coords, + radii_3d=np.full(len(coords), 0.4), + ) + assert by_slot, "expected cell edges to be drawn" + + # Camera-space edges, against which a drawn midpoint is located. + starts, ends = _cell_edges_3d(lattice) + cam_s = (starts - view.centre) @ view.rotation.T + cam_e = (ends - view.centre) @ view.rotation.T + + checked = 0 + for pieces in by_slot.values(): + for polygon, _colour, piece_depth in pieces: + mid_xy = np.asarray(polygon).mean(axis=0) / view.zoom + # Find the edge this piece lies on, and how far along. + best = None + for c_s, c_e in zip(cam_s, cam_e): + seg = (c_e - c_s)[:2] + L = float(seg @ seg) + if L == 0.0: + continue + t = float(np.clip((mid_xy - c_s[:2]) @ seg / L, 0.0, 1.0)) + gap = float(np.linalg.norm(c_s[:2] + t * seg - mid_xy)) + if best is None or gap < best[0]: + best = (gap, c_s, c_e, t) + gap, c_s, c_e, t = best + assert gap < 1e-9, "piece does not lie on any cell edge" + # Depth varies linearly along a straight camera-space + # edge, so the piece's own position fixes its depth. + expected = c_s[2] + t * (c_e[2] - c_s[2]) + assert abs(expected - piece_depth) < 1e-9, ( + f"piece drawn at t={t:.4f} along its edge has depth " + f"{expected:.6f}, but was filed under {piece_depth:.6f}" + ) + checked += 1 + assert checked > 12, f"only {checked} pieces checked" From afe76b6e4727ce53146c3ac49833d3b2a35935fa Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 09:13:48 +0100 Subject: [PATCH 09/13] Fix the version-dependent frozen assertion and close two review gaps test_modes_reject_unknown_attributes pinned TypeError, which is a CPython detail that moved: through 3.12 the slots check fires first and raises TypeError, from 3.13 the frozen check fires first and raises FrozenInstanceError. CI caught it on 3.13 and 3.14. Assert the behaviour that actually matters instead -- the write is refused and leaves nothing behind -- and accept either type. Close the projection sum in _foreshortening_distance, which returned None by fallthrough for an unrecognised mode rather than failing the type check like the other dispatch sites. Reset now clears the stashed perspective as well as the projection. Without it, a distance dialled in before a reset could be restored by a later p, so reset did not fully reset. --- src/hofmann/rendering/bond_geometry.py | 4 ++++ src/hofmann/rendering/interactive.py | 4 ++++ tests/test_model/test_view_state.py | 15 ++++++++++++--- tests/test_rendering/test_interactive.py | 13 +++++++++++++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/hofmann/rendering/bond_geometry.py b/src/hofmann/rendering/bond_geometry.py index c129f514..e4f6c7dc 100644 --- a/src/hofmann/rendering/bond_geometry.py +++ b/src/hofmann/rendering/bond_geometry.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import assert_never + import numpy as np from hofmann.model import Orthographic, Perspective, ViewState @@ -30,6 +32,8 @@ def _foreshortening_distance(view: ViewState) -> float: return p.view_distance case Orthographic(): return _PARALLEL_EYE_DISTANCE + case _: + assert_never(view.projection) def _clip_bond_3d( diff --git a/src/hofmann/rendering/interactive.py b/src/hofmann/rendering/interactive.py index 3d8049fc..5e5082c2 100644 --- a/src/hofmann/rendering/interactive.py +++ b/src/hofmann/rendering/interactive.py @@ -279,6 +279,9 @@ def _apply_key_action( view.zoom = initial_view["zoom"] view.centre = initial_view["centre"].copy() view.projection = initial_view["projection"] + # Reset means reset: a distance dialled in earlier must not + # survive to be restored by a later p. + state["last_perspective"] = initial_view["last_perspective"] # -- Help overlay -- elif key == "h": @@ -450,6 +453,7 @@ def render_mpl_interactive( "zoom": view.zoom, "centre": view.centre.copy(), "projection": view.projection, + "last_perspective": state["last_perspective"], } _DRAG_SENSITIVITY = 0.01 # radians per pixel diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index 7cccd6e5..e226945d 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -340,9 +340,18 @@ def test_modes_are_frozen(self): Perspective().strength = 0.9 def test_modes_reject_unknown_attributes(self): - """slots keeps a typo loud rather than setting an inert attribute.""" - with pytest.raises(TypeError): - Orthographic().anything = 1 + """slots keeps a typo loud rather than setting an inert attribute. + + The exception type is a CPython detail that moved: up to 3.12 + the slots check fires first and raises ``TypeError``, from 3.13 + the frozen check fires first and raises + ``FrozenInstanceError``. What matters here is that the write + is refused and leaves nothing behind, so both are accepted. + """ + mode = Orthographic() + with pytest.raises((AttributeError, TypeError)): + mode.anything = 1 + assert not hasattr(mode, "anything") def test_setters_return_self_for_chaining(self): vs = ViewState() diff --git a/tests/test_rendering/test_interactive.py b/tests/test_rendering/test_interactive.py index a185fb6f..7f51d3c6 100644 --- a/tests/test_rendering/test_interactive.py +++ b/tests/test_rendering/test_interactive.py @@ -85,6 +85,7 @@ def _key_action_fixtures(): "zoom": view.zoom, "centre": view.centre.copy(), "projection": view.projection, + "last_perspective": state["last_perspective"], } return view, style, state, initial_view @@ -243,6 +244,18 @@ def test_perspective_descent_reaches_orthographic_exactly(self): _do_key("P", view, style, state, iv) assert view.projection == Orthographic() + def test_reset_clears_the_stashed_viewing_distance(self): + """Reset means reset: p must not restore a pre-reset distance.""" + view, style, state, iv = _key_action_fixtures() + view.projection = Perspective(0.3, 25.0) + for _ in range(3): + _do_key("P", view, style, state, iv) + _do_key("r", view, style, state, iv) + _do_key("p", view, style, state, iv) + assert view.projection.view_distance == ( + iv["last_perspective"].view_distance + ) + def test_view_distance_survives_an_orthographic_excursion(self): """A parallel projection cannot hold a distance; the session can.""" view, style, state, iv = _key_action_fixtures() From fc871e497f034141b58206b01ab43e1e5ebbb419 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 12:11:19 +0100 Subject: [PATCH 10/13] Warn at the eye plane and pin three unguarded fixes project_camera is the single mapping every geometry path routes through, so guard the degenerate case there once rather than in each renderer. A point at or behind the perspective eye plane is drawn mirrored through the origin at an ordinary radius and sorted as if nearest the viewer, which renders as a plausible ghost rather than anything obviously wrong. The scale is left infinite or negative rather than clamped, so the geometry stays reproducible and the warning is what tells the caller. Three fixes from the previous round were unguarded, each confirmed by mutation in a sandbox with imports pinned away from the live tree: the no-op redraw kind was discarded by its test, the frozen-instance test passed on frozen alone so slots could be dropped, and the seeded perspective's strength was overwritten on read. Trim the interactive documentation back to the contract. Stepping out to an orthographic view at the bottom of the ladder is how the keys have always behaved; that the session remembers the viewing distance across the excursion is bookkeeping the reader does not need. State the silhouette approximation's symptom accurately: the error does not vanish as strength falls, so a radius does not converge to its orthographic value. --- docs/api.rst | 6 ++++ docs/changelog.rst | 11 ++++---- docs/interactive.rst | 3 +- src/hofmann/__init__.py | 6 ++-- src/hofmann/model/view_state.py | 35 ++++++++++++++++++------ src/hofmann/rendering/interactive.py | 8 ++++-- tests/test_model/test_view_state.py | 17 ++++++++++++ tests/test_rendering/test_interactive.py | 9 ++++-- 8 files changed, 73 insertions(+), 22 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 54cc2751..bab1e917 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -49,6 +49,12 @@ Data model .. autoclass:: Perspective :members: +.. data:: Projection + + Type alias for the projection modes: + ``Orthographic | Perspective``. Use it to annotate code that + accepts any projection. + .. autoclass:: hofmann.model.atom_data.AtomData :members: n_atoms, ranges, labels diff --git a/docs/changelog.rst b/docs/changelog.rst index 27f80774..01a82cf3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,16 +14,15 @@ Unreleased becomes ``set_perspective(0.3)`` and ``perspective = 0.0`` becomes ``set_orthographic()``. A ``view_distance`` set alongside a perspective strength moves into the same call: - ``set_perspective(0.3, 5.0)``. A perspective strength no longer doubles as - an on/off switch, so a viewing distance can no longer be set on a - view that ignores it. + ``set_perspective(0.3, 5.0)``. A perspective strength no longer + doubles as an on/off switch, so a viewing distance can no longer be + set on a view that ignores it. - In interactive sessions, the ``d`` and ``D`` viewing-distance keys now act only in perspective mode, where previously they adjusted a distance that had no effect until perspective was switched on. - ``P`` steps out to an orthographic projection once the strength - reaches the bottom of the ladder, and ``p`` restores the viewing - distance the session was last using rather than resetting it. + ``P`` returns to an orthographic view at the bottom of the ladder, + as before. 0.20.0 ------ diff --git a/docs/interactive.rst b/docs/interactive.rst index fdea5b3e..12ccb7f1 100644 --- a/docs/interactive.rst +++ b/docs/interactive.rst @@ -75,7 +75,8 @@ Pan and perspective * - Shift + Arrow keys - Pan the view * - ``p`` / ``P`` - - Increase / decrease perspective strength + - Increase / decrease perspective strength (``P`` returns to an + orthographic view) * - ``d`` / ``D`` - Increase / decrease viewing distance (perspective only) diff --git a/src/hofmann/__init__.py b/src/hofmann/__init__.py index ce5dafad..d472e610 100644 --- a/src/hofmann/__init__.py +++ b/src/hofmann/__init__.py @@ -32,15 +32,15 @@ Frame, LegendItem, LegendStyle, + Orthographic, + Perspective, Polyhedron, PolygonLegendItem, PolyhedronLegendItem, PolyhedronSpec, + Projection, RenderStyle, SlabClipMode, - Orthographic, - Perspective, - Projection, StructureScene, ViewState, WidgetCorner, diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index 013a06e9..ee858d4f 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -1,6 +1,7 @@ from __future__ import annotations import math +import warnings from dataclasses import dataclass, field from typing import assert_never @@ -18,9 +19,10 @@ class Perspective: Screen positions are scaled by ``D / (D - z * s)`` for an atom at camera depth *z*, writing *D* for :attr:`view_distance` and *s* - for :attr:`strength`. That places the eye at ``D / s``. A *strength* of ``1.0`` is therefore a true pinhole - camera at :attr:`view_distance`; smaller values move the eye - further out, weakening the foreshortening. + for :attr:`strength`. That places the eye at ``D / s``, so a + *strength* of ``1.0`` is a true pinhole camera at + :attr:`view_distance`; smaller values move the eye further out, + weakening the foreshortening. Attributes: strength: Perspective strength. Must be positive; use @@ -139,7 +141,10 @@ def project( # Silhouette radius: r * D / sqrt(d^2 - r^2). # Exact for an eye at D; the eye is at D / strength, # for which the exact form carries (r * strength)^2. - # The two agree at full strength. + # The two agree at full strength only -- the error does + # not vanish as strength falls, so a radius does not + # converge to its orthographic value (r=1.5, D=10 at + # depth 0 gives 1.5172 at every strength). denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) projected_radii = radii * p.view_distance / denom * self.zoom case Orthographic(): @@ -174,10 +179,24 @@ def project_camera( camera = np.asarray(camera, dtype=float) match self.projection: case Perspective() as p: - scale = p.view_distance / ( - p.view_distance - camera[:, 2] * p.strength - ) - xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom + denom = p.view_distance - camera[:, 2] * p.strength + if np.any(denom <= 0): + warnings.warn( + "one or more points lie at or behind the " + f"perspective eye plane (view_distance=" + f"{p.view_distance:g}, strength={p.strength:g}); " + "they are drawn mirrored through the origin and " + "sorted as if nearest the viewer. Increase " + "view_distance or reduce strength.", + UserWarning, + stacklevel=2, + ) + # Not clamped: the scale is left infinite or negative + # so the geometry stays reproducible, and the warning + # above is what tells the caller. + with np.errstate(divide="ignore", invalid="ignore"): + scale = p.view_distance / denom + xy = camera[:, :2] * scale[:, np.newaxis] * self.zoom case Orthographic(): # Reported as ones for the caller, but not applied: # a parallel projection does not scale with depth. diff --git a/src/hofmann/rendering/interactive.py b/src/hofmann/rendering/interactive.py index 5e5082c2..abf99e1e 100644 --- a/src/hofmann/rendering/interactive.py +++ b/src/hofmann/rendering/interactive.py @@ -71,8 +71,10 @@ def _rotation_z(angle: float) -> np.ndarray: _DISTANCE_FACTOR = 1.05 # viewing distance multiplier per key press _MIN_VIEW_DISTANCE = 0.1 # floor on Perspective.view_distance -#: Seeds the p key when no perspective has been used this session. -_DEFAULT_INTERACTIVE_PERSPECTIVE = Perspective(strength=_PERSPECTIVE_STEP) +#: Seeds the viewing distance the p key restores when no perspective +#: has been used this session. Only view_distance is read back: p sets +#: the strength itself. +_DEFAULT_INTERACTIVE_PERSPECTIVE = Perspective() _HELP_TEXT = """\ Arrows Rotate Shift+Arrows Pan @@ -182,6 +184,7 @@ def _apply_key_action( strength=min(1.0, proj.strength + _PERSPECTIVE_STEP), ) case _: + # Any non-perspective mode is replaced wholesale. # A parallel projection cannot hold a viewing distance, # so re-entering perspective restores the one the # session last used rather than resetting it. @@ -201,6 +204,7 @@ def _apply_key_action( state["last_perspective"] = proj view.projection = Orthographic() case _: + # Nothing to step down from under a parallel mode. return "none" elif key == "d": match view.projection: diff --git a/tests/test_model/test_view_state.py b/tests/test_model/test_view_state.py index e226945d..eefca722 100644 --- a/tests/test_model/test_view_state.py +++ b/tests/test_model/test_view_state.py @@ -1,6 +1,7 @@ """Tests for ViewState projection, look_along, slab clipping, and validation.""" import dataclasses +import warnings import numpy as np import pytest @@ -97,6 +98,18 @@ def test_zoom_scales_perspective_radii(self): _, _, r_zoomed = zoomed.project(coords, radii) np.testing.assert_allclose(r_zoomed, r_plain * 2.5) + def test_points_at_or_behind_the_eye_plane_warn(self): + """Behind-eye points draw mirrored and sort frontmost.""" + vs = ViewState(projection=Perspective(1.0, 10.0)) + with pytest.warns(UserWarning, match="eye plane"): + vs.project(np.array([[1.0, 0.0, 11.0]])) + + def test_no_eye_plane_warning_for_ordinary_depths(self): + vs = ViewState(projection=Perspective(1.0, 10.0)) + with warnings.catch_warnings(): + warnings.simplefilter("error") + vs.project(np.array([[1.0, 0.0, 2.0]])) + def test_projected_radii_larger_than_point_scale(self): """Silhouette radii should exceed naive r * scale under perspective.""" vs = ViewState(projection=Perspective(1.0, 10.0)) @@ -352,6 +365,10 @@ def test_modes_reject_unknown_attributes(self): with pytest.raises((AttributeError, TypeError)): mode.anything = 1 assert not hasattr(mode, "anything") + # frozen alone would satisfy the raise above, so pin the slots + # that make an unknown attribute unstorable in the first place. + assert not hasattr(mode, "__dict__") + assert not hasattr(Perspective(), "__dict__") def test_setters_return_self_for_chaining(self): vs = ViewState() diff --git a/tests/test_rendering/test_interactive.py b/tests/test_rendering/test_interactive.py index 7f51d3c6..e94e59f7 100644 --- a/tests/test_rendering/test_interactive.py +++ b/tests/test_rendering/test_interactive.py @@ -294,10 +294,15 @@ def test_distance_clamped_min(self): @pytest.mark.parametrize("key", ["P", "d", "D"]) def test_perspective_keys_are_inert_without_perspective(self, key): - """Only p enters perspective mode; the rest need it already set.""" + """Only p enters perspective mode; the rest need it already set. + + The redraw kind is asserted too: a key that changes nothing + must not ask for a redraw, or the no-op costs a frame. + """ view, style, state, iv = _key_action_fixtures() - _do_key(key, view, style, state, iv) + kind = _do_key(key, view, style, state, iv) assert view.projection == Orthographic() + assert kind == "none" # -- Style toggles -- From 13856bd1eac14261f87d10e52b8cc2a7e3f30b28 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 12:22:32 +0100 Subject: [PATCH 11/13] Pin the perspective ladder against arbitrary strengths set_perspective accepts any positive strength, so the ladder must step by a fixed increment from wherever a caller leaves it rather than assuming a grid. Nothing covered a starting strength that was not a multiple of the step; a change that snapped to one passed the suite. --- tests/test_rendering/test_interactive.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_rendering/test_interactive.py b/tests/test_rendering/test_interactive.py index e94e59f7..aefe2252 100644 --- a/tests/test_rendering/test_interactive.py +++ b/tests/test_rendering/test_interactive.py @@ -256,6 +256,30 @@ def test_reset_clears_the_stashed_viewing_distance(self): iv["last_perspective"].view_distance ) + @pytest.mark.parametrize("start", [0.37, 0.05, 0.999]) + def test_ladder_descends_from_any_programmatic_strength(self, start): + """set_perspective accepts any strength, so the ladder must + step by a fixed amount from wherever it starts, keeping the + caller's fractional part, and exit to Orthographic when the + next step would leave nothing. + """ + view, style, state, iv = _key_action_fixtures() + view.projection = Perspective(start, 25.0) + state["last_perspective"] = view.projection + + seen = [start] + for _ in range(15): + _do_key("P", view, style, state, iv) + if isinstance(view.projection, Orthographic): + break + seen.append(view.projection.strength) + else: + raise AssertionError("descent never reached Orthographic") + + # Each step is exactly one increment. + for before, after in zip(seen, seen[1:]): + assert before - after == pytest.approx(_PERSPECTIVE_STEP) + def test_view_distance_survives_an_orthographic_excursion(self): """A parallel projection cannot hold a distance; the session can.""" view, style, state, iv = _key_action_fixtures() From 958618e9fac170b2d9cd362e84d1f18ad53cb5f8 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 12:40:25 +0100 Subject: [PATCH 12/13] Warn at the eye plane in _scene_extent, and finish two review nits project_camera warns when the scene reaches the perspective eye plane; _scene_extent has the same denominator and the same blank-canvas pathology but stayed silent, so the two sites disagreed. Warn here too. This is the site reachable from the keyboard -- p to full strength, then hold D. The magnification maths is untouched; the degenerate case is now loud rather than silent, which was the policy set for project_camera. Alphabetise Orthographic/Perspective/Projection in __all__ (the import statement was ordered earlier but the export list was missed), and state the reason the silhouette radius keeps its approximate form: bond end caps foreshorten to the same reference distance, so the two must be corrected together or the atoms desync from their bonds. --- src/hofmann/__init__.py | 4 ++-- src/hofmann/model/view_state.py | 10 ++++++---- src/hofmann/rendering/projection.py | 18 +++++++++++++++-- tests/test_rendering/test_projection.py | 26 +++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/hofmann/__init__.py b/src/hofmann/__init__.py index d472e610..0e757757 100644 --- a/src/hofmann/__init__.py +++ b/src/hofmann/__init__.py @@ -64,10 +64,10 @@ "Frame", "LegendItem", "LegendStyle", - "Polyhedron", - "PolygonLegendItem", "Orthographic", "Perspective", + "Polyhedron", + "PolygonLegendItem", "PolyhedronLegendItem", "PolyhedronSpec", "Projection", diff --git a/src/hofmann/model/view_state.py b/src/hofmann/model/view_state.py index ee858d4f..69b9cae1 100644 --- a/src/hofmann/model/view_state.py +++ b/src/hofmann/model/view_state.py @@ -141,10 +141,12 @@ def project( # Silhouette radius: r * D / sqrt(d^2 - r^2). # Exact for an eye at D; the eye is at D / strength, # for which the exact form carries (r * strength)^2. - # The two agree at full strength only -- the error does - # not vanish as strength falls, so a radius does not - # converge to its orthographic value (r=1.5, D=10 at - # depth 0 gives 1.5172 at every strength). + # Left as-is deliberately: bond end caps foreshorten to + # the same reference distance D (see + # bond_geometry._foreshortening_distance), so correcting + # only the silhouette here would desync atom radii from + # the bonds meeting them. Both move together, or + # neither does. denom = np.sqrt(np.maximum(d**2 - radii**2, 1e-12)) projected_radii = radii * p.view_distance / denom * self.zoom case Orthographic(): diff --git a/src/hofmann/rendering/projection.py b/src/hofmann/rendering/projection.py index ae0e11d7..da6a83f1 100644 --- a/src/hofmann/rendering/projection.py +++ b/src/hofmann/rendering/projection.py @@ -2,10 +2,11 @@ from __future__ import annotations -import numpy as np - +import warnings from typing import assert_never +import numpy as np + from hofmann.model import ( Orthographic, Perspective, @@ -105,6 +106,19 @@ def _scene_extent( case Perspective() as p if len(dists) > 0: worst_depth = float(np.max(dists)) denom = p.view_distance - worst_depth * p.strength + if denom <= 0: + # The scene reaches the eye plane, the same degenerate + # case ViewState.project_camera warns about; here it + # would blow the viewport up to a blank canvas. + warnings.warn( + "the scene reaches the perspective eye plane " + f"(view_distance={p.view_distance:g}, " + f"strength={p.strength:g}); the view cannot be " + "sized and renders blank. Increase view_distance " + "or reduce strength.", + UserWarning, + stacklevel=2, + ) # Not max(denom, 1e-6): that would alter 0 < denom < 1e-6. persp_scale = p.view_distance / (denom if denom > 0 else 1e-6) max_extent *= persp_scale diff --git a/tests/test_rendering/test_projection.py b/tests/test_rendering/test_projection.py index 7fd8bd52..cbf9deb4 100644 --- a/tests/test_rendering/test_projection.py +++ b/tests/test_rendering/test_projection.py @@ -1,6 +1,7 @@ """Tests for projection helpers — _project_point and _scene_extent.""" import math +import warnings import matplotlib.pyplot as plt import numpy as np @@ -61,6 +62,31 @@ def test_perspective_increases_extent(self): e_yes = _scene_extent(scene, view_persp, 0, atom_scale=0.5) assert e_yes > e_no + def test_scene_reaching_the_eye_plane_warns(self): + """The blank-canvas degenerate case must not be silent.""" + scene = StructureScene( + species=["C", "C"], + frames=[Frame(coords=np.array([ + [0.0, 0.0, -9.0], + [0.0, 0.0, 9.0], + ]))], + atom_styles={"C": AtomStyle(1.0, (0.5, 0.5, 0.5))}, + ) + view = ViewState(projection=Perspective(1.0, 9.0)) + with pytest.warns(UserWarning, match="eye plane"): + _scene_extent(scene, view, 0, atom_scale=0.5) + + def test_ordinary_perspective_scene_does_not_warn(self): + scene = StructureScene( + species=["C"], + frames=[Frame(coords=np.array([[0.0, 0.0, 0.0]]))], + atom_styles={"C": AtomStyle(1.0, (0.5, 0.5, 0.5))}, + ) + view = ViewState(projection=Perspective(0.5, 20.0)) + with warnings.catch_warnings(): + warnings.simplefilter("error") + _scene_extent(scene, view, 0, atom_scale=0.5) + def test_empty_scene(self): """An empty scene (zero atoms) should return a positive extent.""" scene = StructureScene( From 2f67a7a4163594ecdcc03bc6c977e63905be49ab Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 16 Aug 2026 12:54:04 +0100 Subject: [PATCH 13/13] Sort the model __all__ to match the package __all__ The two export lists used different orderings for the same names. Sort the model list the way the top-level one already is, rather than moving only the three projection names and leaving the file split between two schemes. --- src/hofmann/model/__init__.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hofmann/model/__init__.py b/src/hofmann/model/__init__.py index dbc8b0eb..d7e89361 100644 --- a/src/hofmann/model/__init__.py +++ b/src/hofmann/model/__init__.py @@ -45,20 +45,20 @@ "BondSpec", "CellEdgeStyle", "CmapSpec", - "LegendItem", - "LegendStyle", "Colour", "Composition", "Frame", - "Polyhedron", + "LegendItem", + "LegendStyle", + "Orthographic", + "Perspective", "PolygonLegendItem", + "Polyhedron", "PolyhedronLegendItem", "PolyhedronSpec", + "Projection", "RenderStyle", "SlabClipMode", - "Orthographic", - "Perspective", - "Projection", "StructureScene", "ViewState", "WidgetCorner",