Skip to content

Commit d26e124

Browse files
FIX: hand back an instance cloud from instanced_mesh
gh-13074 made mne/viz/_3d.py write channel names onto the second value instanced_mesh returns, which broke plot_alignment here three ways: a pyvista-js PolyData has no field_data, several colours came back as a list, and empty positions came back as None. Return the per-instance point cloud instead, always one object with the mapping attached, which is what _PyVistaRenderer returns.
1 parent 33dfaf8 commit d26e124

2 files changed

Lines changed: 88 additions & 29 deletions

File tree

mne/viz/backends/_lite.py

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,22 @@ def _lite_get_view(plotter):
156156
_lite_live_plotters = []
157157

158158

159+
def _lite_instance_cloud(positions):
160+
"""Return the per-instance point cloud ``instanced_mesh`` hands back.
161+
162+
``_PyVistaRenderer`` glyphs its template over a ``PolyData`` of the instance
163+
positions and returns that object, and :mod:`mne.viz._3d` hangs channel
164+
names off its ``field_data`` (gh-13074). vtk.js has no equivalent, and a
165+
pyvista-js ``PolyData`` carries no ``field_data`` of its own, so build the
166+
same cloud here and give it the mapping. Nothing in the browser reads it
167+
back: the one reader is the dipole-fit GUI, which needs a picker vtk.js
168+
does not provide.
169+
"""
170+
cloud = pv.PolyData(points=np.asarray(positions, dtype=float).reshape(-1, 3))
171+
cloud.field_data = dict()
172+
return cloud
173+
174+
159175
def _lite_release_plotter(plotter):
160176
"""Hand back a plotter's meshes, JS arrays and GPU buffers.
161177
@@ -628,14 +644,18 @@ def instanced_mesh(
628644
cylinders) point the way MNE intended rather than all along +x.
629645
pyvista-js has no per-vertex color, so instances are grouped by the
630646
color they asked for and each group becomes one mesh -- a handful of
631-
actors for a sensor array instead of one per sensor. That means one
632-
distinct color returns ``(actor, mesh)`` like ``_PyVistaRenderer``
633-
does, and several return the lists of both.
647+
actors for a sensor array instead of one per sensor. One distinct color
648+
hands back that single actor, several hand back the list of them.
649+
650+
The second return value is the per-instance point cloud, always one
651+
object whatever the colors did, because that is what
652+
``_PyVistaRenderer`` returns and what mne/viz/_3d.py writes channel
653+
names onto.
634654
"""
635655
positions = np.atleast_2d(np.asarray(positions, dtype=float))[:, :3]
636656
n_pos = len(positions)
637657
if not n_pos:
638-
return None, None
658+
return None, _lite_instance_cloud(positions)
639659
rots = None
640660
if quats is not None:
641661
rots = np.asarray(
@@ -651,7 +671,9 @@ def instanced_mesh(
651671
groups = [(uniq[k], idx[inverse == k]) for k in range(len(uniq))]
652672
else:
653673
groups = [(colors, idx)]
654-
actors, meshes = list(), list()
674+
# only the actors are collected: _add already registers each mesh with
675+
# the plotter, and the object callers want back is the instance cloud
676+
actors = list()
655677
for color, sel in groups:
656678
group_scales = None
657679
if scales is not None:
@@ -661,15 +683,15 @@ def instanced_mesh(
661683
points, faces = self._tile(
662684
rr, tris, positions[sel], scales=group_scales, rots=group_rots
663685
)
664-
actor, mesh = self._add(points, faces, color, opacity)
686+
actor, _ = self._add(points, faces, color, opacity)
665687
actors.append(actor)
666-
meshes.append(mesh)
667688
# one group is the common case and matches _PyVistaRenderer, which
668-
# colors per instance inside a single actor; hand back the pair then,
669-
# and the whole set when the colors had to be split across meshes
689+
# colors per instance inside a single actor; hand that actor back on
690+
# its own, and the whole set when the colors had to be split
691+
cloud = _lite_instance_cloud(positions)
670692
if len(actors) == 1:
671-
return actors[0], meshes[0]
672-
return actors, meshes
693+
return actors[0], cloud
694+
return actors, cloud
673695

674696
def text2d(
675697
self,

mne/viz/backends/tests/test_lite.py

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,16 @@
2222
_TRIS = np.array([[0, 1, 2], [0, 2, 3]])
2323

2424

25+
def _drawn(renderer):
26+
"""Return the geometry of the mesh the renderer drew last.
27+
28+
sphere() and the other instanced_mesh callers hand back the instance cloud
29+
rather than the drawn geometry, matching _PyVistaRenderer, so read what
30+
actually reached the plotter instead of the return value.
31+
"""
32+
return renderer.plotter.actors[-1]["mesh"]
33+
34+
2535
def test_is_a_registered_backend(renderer_lite):
2636
"""``set_3d_backend("jupyterlite_notebook")`` must hand out this renderer."""
2737
assert renderer_lite.get_3d_backend() == "jupyterlite_notebook"
@@ -159,8 +169,8 @@ def test_draws_every_primitive(renderer_lite):
159169
assert_allclose(np.asarray(mesh.points), _RR, atol=1e-6)
160170

161171
# scale 0.1 means radius 0.05, centered where it was asked for
162-
_, mesh = r.sphere(np.array([[1.0, 0, 0]]), "green", 0.1)
163-
points = np.asarray(mesh.points)
172+
r.sphere(np.array([[1.0, 0, 0]]), "green", 0.1)
173+
points = np.asarray(_drawn(r).points)
164174
assert_allclose(points.mean(axis=0), [1, 0, 0], atol=1e-6)
165175
assert np.linalg.norm(points - [1, 0, 0], axis=1).max() == pytest.approx(0.05)
166176

@@ -264,11 +274,13 @@ def test_sphere_radius_matches_pyvista(renderer_lite):
264274
and no caller in ``_3d.py`` passes ``radius`` to say otherwise.
265275
"""
266276
r = renderer_lite._get_renderer(size=(200, 200))
267-
_, mesh = r.sphere(np.zeros((1, 3)), "red", 0.01)
268-
assert np.linalg.norm(np.asarray(mesh.points), axis=1).max() == pytest.approx(0.005)
277+
r.sphere(np.zeros((1, 3)), "red", 0.01)
278+
drawn = np.asarray(_drawn(r).points)
279+
assert np.linalg.norm(drawn, axis=1).max() == pytest.approx(0.005)
269280
# an explicit radius is used as-is, again matching _pyvista.py
270-
_, mesh = r.sphere(np.zeros((1, 3)), "red", 1.0, radius=0.02)
271-
assert np.linalg.norm(np.asarray(mesh.points), axis=1).max() == pytest.approx(0.02)
281+
r.sphere(np.zeros((1, 3)), "red", 1.0, radius=0.02)
282+
drawn = np.asarray(_drawn(r).points)
283+
assert np.linalg.norm(drawn, axis=1).max() == pytest.approx(0.02)
272284

273285

274286
def test_cylinder_center_is_turned_with_the_axis(renderer_lite):
@@ -290,31 +302,56 @@ def test_cylinder_center_is_turned_with_the_axis(renderer_lite):
290302
def test_instances_are_merged_per_color(renderer_lite):
291303
"""``instanced_mesh`` draws one actor per distinct color, not one per instance.
292304
293-
One color hands back ``(actor, mesh)``, the way ``_PyVistaRenderer`` always
294-
does; several hand back both lists, since vtk.js cannot color per instance
295-
inside a single actor.
305+
vtk.js cannot color per instance inside a single actor, so one color gives
306+
one actor and several give the list of them. The second return value is the
307+
instance cloud either way, matching ``_PyVistaRenderer``.
296308
"""
297309
quats = np.tile([1.0, 0, 0, 0], (3, 1))
298-
positions = np.zeros((3, 3))
310+
positions = np.array([[0.0, 0, 0], [1.0, 0, 0], [2.0, 0, 0]])
299311

300-
# one color: a single actor, and the pair _PyVistaRenderer also hands back
312+
# one color: a single actor
301313
r = renderer_lite._get_renderer(size=(200, 200))
302314
colors = np.tile([1.0, 0, 0], (3, 1))
303-
actor, mesh = r.instanced_mesh(_RR, _TRIS, positions, quats, colors=colors)
315+
actor, cloud = r.instanced_mesh(_RR, _TRIS, positions, quats, colors=colors)
304316
assert len(r.plotter.actors) == 1
305-
assert not isinstance(actor, list) and not isinstance(mesh, list)
317+
assert not isinstance(actor, list)
318+
assert_allclose(np.asarray(cloud.points), positions, atol=1e-6)
306319

307-
# two colors: one actor each, and both lists come back
320+
# two colors: one actor each, and the cloud is still a single object
308321
r = renderer_lite._get_renderer(size=(200, 200))
309322
colors = np.array([[1.0, 0, 0], [0, 1.0, 0], [1.0, 0, 0]])
310-
actors, meshes = r.instanced_mesh(_RR, _TRIS, positions, quats, colors=colors)
323+
actors, cloud = r.instanced_mesh(_RR, _TRIS, positions, quats, colors=colors)
311324
assert len(r.plotter.actors) == 2
312-
assert len(actors) == len(meshes) == 2
325+
assert len(actors) == 2
326+
assert not isinstance(cloud, list)
327+
assert_allclose(np.asarray(cloud.points), positions, atol=1e-6)
313328

314329
# sphere routes through instanced_mesh with a single color, so it has to
315330
# keep handing back the pair its own callers unpack
316-
actor, mesh = r.sphere(np.zeros((1, 3)), "red", 0.01)
317-
assert not isinstance(actor, list) and not isinstance(mesh, list)
331+
actor, cloud = r.sphere(np.zeros((1, 3)), "red", 0.01)
332+
assert not isinstance(actor, list) and not isinstance(cloud, list)
333+
334+
335+
def test_instance_cloud_takes_channel_names(renderer_lite):
336+
"""mne/viz/_3d.py writes channel names onto the cloud (gh-13074).
337+
338+
_PyVistaRenderer hands back a PolyData whose ``field_data`` takes them; a
339+
pyvista-js PolyData has no such attribute, so the renderer supplies one.
340+
An empty ``positions`` must still give a cloud, since the caller assigns
341+
without checking.
342+
"""
343+
r = renderer_lite._get_renderer(size=(200, 200))
344+
positions = np.array([[0.0, 0, 0], [1.0, 0, 0]])
345+
_, cloud = r.instanced_mesh(_RR, _TRIS, positions, colors=(1.0, 0, 0))
346+
# one cloud point per instance, in order: _3d.py indexes the names against
347+
# them, so a cloud that did not carry the positions would mislabel sensors
348+
assert_allclose(np.asarray(cloud.points), positions, atol=1e-6)
349+
cloud.field_data["ch_names"] = np.array(["MEG 0113", "MEG 0112"], dtype="U")
350+
assert list(cloud.field_data["ch_names"]) == ["MEG 0113", "MEG 0112"]
351+
352+
actor, cloud = r.instanced_mesh(_RR, _TRIS, np.zeros((0, 3)))
353+
assert actor is None
354+
cloud.field_data["ch_names"] = np.array([], dtype="U") # must not raise
318355

319356

320357
def test_draws_into_an_existing_figure(renderer_lite):

0 commit comments

Comments
 (0)