Fix cell edge clipping by hidden atoms (closes #41)

Cell edge lines were clipped at atom spheres even when those atoms
were not drawn.  Atoms hidden by PolyhedronSpec.hide_centre,
AtomStyle.visible=False, hide_vertices, or slab clipping no longer
produce spurious gaps in cell outline edges.

The fix filters coords and radii to only visible atoms before passing
them to _collect_cell_edges().  Also renames the local hidden_atoms
variable in _draw_scene to all_hidden_atoms to avoid shadowing
precomputed.hidden_atoms.

Bumps version to 0.14.1.

Author: bjmorgan

docs/changelog.rst

@@ -1,6 +1,15 @@
 Changelog
 =========
 
+0.14.1
+------
+
+- Fixed cell edge lines being clipped at atoms that are not drawn.
+  Atoms hidden by :attr:`~hofmann.PolyhedronSpec.hide_centre`,
+  :attr:`~hofmann.AtomStyle.visible`, or slab clipping no longer
+  produce spurious gaps in cell outline edges.
+  (`#41 <https://github.com/bjmorgan/hofmann/issues/41>`_).
+
 0.14.0
 ------
 

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "hofmann"
-version = "0.14.0"
+version = "0.14.1"
 description = "A modern Python reimagining of the XBS ball-and-stick crystal structure viewer"
 readme = "README.md"
 requires-python = ">=3.11"

src/hofmann/rendering/painter.py

@@ -538,11 +538,11 @@ def _draw_scene(
     # AtomStyle.visible=False hiding is always applied.  Polyhedra-
     # driven hiding (hide_centre, hide_bonds, hide_vertices) is only
     # applied when polyhedra are actually being drawn.
-    hidden_atoms = set(precomputed.style_hidden_atoms)
-    hidden_bond_ids = set(precomputed.style_hidden_bond_ids)
+    all_hidden_atoms = set(precomputed.style_hidden_atoms)
+    all_hidden_bond_ids = set(precomputed.style_hidden_bond_ids)
     if show_polyhedra:
-        hidden_atoms |= precomputed.hidden_atoms
-        hidden_bond_ids |= precomputed.hidden_bond_ids
+        all_hidden_atoms |= precomputed.hidden_atoms
+        all_hidden_bond_ids |= precomputed.hidden_bond_ids
 
     face_by_depth_slot, vertex_max_face_slot = _collect_polyhedra_faces(
         precomputed, polyhedra_list, poly_skip, slab_visible,
@@ -557,7 +557,7 @@ def _draw_scene(
     if show_polyhedra and vertex_max_face_slot:
         atom_depths_sorted = depth[order]
         for vi, max_slot in vertex_max_face_slot.items():
-            if vi in hidden_atoms:
+            if vi in all_hidden_atoms:
                 continue
             vi_slot = int(np.searchsorted(atom_depths_sorted, depth[vi]))
             if vi_slot <= max_slot:
@@ -592,9 +592,13 @@ def _draw_scene(
                 (xy[:, 0].max() - xy[:, 0].min()) / 2,
                 (xy[:, 1].max() - xy[:, 1].min()) / 2,
             ) + cell_margin
+        # Only clip cell edges at atoms that are actually drawn (#41).
+        clip_visible = slab_visible.copy()
+        if all_hidden_atoms:
+            clip_visible[list(all_hidden_atoms)] = False
         cell_edge_by_depth_slot = _collect_cell_edges(
             lattice, view, style.cell_style, depth, order, cell_pad,
-            coords, radii_3d * atom_scale,
+            coords[clip_visible], (radii_3d * atom_scale)[clip_visible],
         )
 
     # Collect raw vertex arrays in painter's order, then batch-add
@@ -627,7 +631,7 @@ def _draw_scene(
                 bond_id = id(bond)
                 if bond_id in drawn_bonds:
                     continue
-                if bond_id in hidden_bond_ids:
+                if bond_id in all_hidden_bond_ids:
                     drawn_bonds.add(bond_id)
                     continue
                 if bond_id in poly_clip_hidden_bonds:
@@ -735,7 +739,7 @@ def _draw_scene(
 
         # Draw atom circle (unless hidden by a polyhedron spec or
         # deferred for polyhedron vertex ordering).
-        if (k not in hidden_atoms
+        if (k not in all_hidden_atoms
                 and k not in deferred_vertex_atoms):
             fc_atom = (*atom_colours[k], 1.0)
             all_verts.append(unit_circle * atom_screen_radii[k] + xy[k])

tests/test_rendering/test_cell_edges.py

@@ -10,6 +10,7 @@
     BondSpec,
     CellEdgeStyle,
     Frame,
+    PolyhedronSpec,
     RenderStyle,
     StructureScene,
     ViewState,
@@ -244,3 +245,138 @@ def test_cell_edges_clipped_at_corner_atoms(self):
         # edge polygons (clipping shortens edges but doesn't remove them).
         assert n_corner >= 1
         assert n_mid >= 1
+
+    def test_hidden_polyhedra_centre_does_not_clip_cell_edge(self):
+        """A polyhedron centre with hide_centre=True should not clip cell edges.
+
+        Reproduces https://github.com/bjmorgan/hofmann/issues/41.
+        """
+        a = 10.0
+        d = 1.8
+        ti = np.array([a / 2, 0.0, 0.0])
+        offsets = d / np.sqrt(3) * np.array([
+            [1, 1, 1], [1, -1, -1], [-1, 1, -1], [-1, -1, 1],
+        ])
+        coords = np.vstack([ti[None, :], ti + offsets])
+
+        def _make_scene(hide_centre):
+            return StructureScene(
+                species=["Ti", "O", "O", "O", "O"],
+                frames=[Frame(coords=coords, lattice=np.eye(3) * a)],
+                atom_styles={
+                    "Ti": AtomStyle(1.2, (0.2, 0.4, 0.9)),
+                    "O": AtomStyle(0.5, (0.9, 0.1, 0.1)),
+                },
+                bond_specs=[
+                    BondSpec(species=("Ti", "O"), min_length=0.5,
+                             max_length=3.5, radius=0.1,
+                             colour=(0.4, 0.4, 0.4)),
+                ],
+                polyhedra=[
+                    PolyhedronSpec(centre="Ti", hide_centre=hide_centre,
+                                   hide_bonds=True),
+                ],
+            )
+
+        render_kwargs = dict(show=False, show_polyhedra=True,
+                             show_cell=True)
+
+        # Centre hidden — should not clip cell edges.
+        fig_hidden = render_mpl(_make_scene(hide_centre=True),
+                                **render_kwargs)
+        n_hidden = len(fig_hidden.axes[0].collections[0].get_paths())
+        plt.close(fig_hidden)
+
+        # Centre visible — should clip cell edges.
+        fig_visible = render_mpl(_make_scene(hide_centre=False),
+                                 **render_kwargs)
+        n_visible = len(fig_visible.axes[0].collections[0].get_paths())
+        plt.close(fig_visible)
+
+        # The only difference is Ti visibility.  When hidden it should
+        # not split cell edges, producing fewer polygon paths.
+        assert n_hidden < n_visible
+
+    def test_invisible_atom_does_not_clip_cell_edge(self):
+        """An atom with AtomStyle.visible=False should not clip cell edges."""
+        a = 10.0
+        coords = np.array([[a / 2, 0.0, 0.0]])
+        # Scene with invisible atom on the cell edge.
+        scene_invisible = StructureScene(
+            species=["A"],
+            frames=[Frame(coords=coords, lattice=np.eye(3) * a)],
+            atom_styles={"A": AtomStyle(1.0, (0.5, 0.5, 0.5),
+                                        visible=False)},
+        )
+        # Baseline: atom far from any cell edge but at the same depth
+        # (z=0) so depth-splitting of cell edges is identical.
+        scene_no_clip = StructureScene(
+            species=["A"],
+            frames=[Frame(
+                coords=np.array([[a / 2, a / 2, 0.0]]),
+                lattice=np.eye(3) * a,
+            )],
+            atom_styles={"A": AtomStyle(1.0, (0.5, 0.5, 0.5),
+                                        visible=False)},
+        )
+        fig_inv = render_mpl(scene_invisible, show=False, show_cell=True)
+        n_inv = len(fig_inv.axes[0].collections[0].get_paths())
+        plt.close(fig_inv)
+
+        fig_no_clip = render_mpl(scene_no_clip, show=False, show_cell=True)
+        n_no_clip = len(fig_no_clip.axes[0].collections[0].get_paths())
+        plt.close(fig_no_clip)
+
+        # An invisible atom on a cell edge should produce the same
+        # number of paths as one that doesn't intersect any edge.
+        assert n_inv == n_no_clip
+
+    def test_hidden_polyhedra_vertices_do_not_clip_cell_edge(self):
+        """Vertex atoms hidden by hide_vertices=True should not clip cell edges."""
+        a = 10.0
+        d = 1.8
+        ti = np.array([a / 2, a / 2, a / 2])
+        # One O vertex sits on the cell edge at (a/2, 0, 0).
+        coords = np.array([
+            ti,
+            np.array([a / 2, 0.0, 0.0]),
+            ti + d * np.array([1, -1, -1]) / np.sqrt(3),
+            ti + d * np.array([-1, 1, -1]) / np.sqrt(3),
+            ti + d * np.array([-1, -1, 1]) / np.sqrt(3),
+        ])
+
+        def _make_scene(hide_vertices):
+            return StructureScene(
+                species=["Ti", "O", "O", "O", "O"],
+                frames=[Frame(coords=coords, lattice=np.eye(3) * a)],
+                atom_styles={
+                    "Ti": AtomStyle(0.5, (0.2, 0.4, 0.9)),
+                    "O": AtomStyle(1.0, (0.9, 0.1, 0.1)),
+                },
+                bond_specs=[
+                    BondSpec(species=("Ti", "O"), min_length=0.5,
+                             max_length=6.0, radius=0.1,
+                             colour=(0.4, 0.4, 0.4)),
+                ],
+                polyhedra=[
+                    PolyhedronSpec(centre="Ti",
+                                   hide_vertices=hide_vertices,
+                                   hide_bonds=True),
+                ],
+            )
+
+        render_kwargs = dict(show=False, show_polyhedra=True,
+                             show_cell=True)
+
+        fig_hidden = render_mpl(_make_scene(hide_vertices=True),
+                                **render_kwargs)
+        n_hidden = len(fig_hidden.axes[0].collections[0].get_paths())
+        plt.close(fig_hidden)
+
+        fig_visible = render_mpl(_make_scene(hide_vertices=False),
+                                 **render_kwargs)
+        n_visible = len(fig_visible.axes[0].collections[0].get_paths())
+        plt.close(fig_visible)
+
+        # When vertex is hidden, it should not clip the cell edge.
+        assert n_hidden < n_visible