ENH: add a vtk.js backend for MNE's 3D renderer (JupyterLite split 3/5) - #14144
Conversation
VTK cannot load in WebAssembly, so the JupyterLite notebooks need a renderer that draws with vtk.js. MNE does its geometry in numpy and only hands the result to a renderer, so replacing that last step leaves the transform maths to MNE.
c20eae7 to
5a823e2
Compare
|
Maybe you have looked and I am coming late to this... have you thought about adding it as a type of AbstractRenderer and using https://github.com/tkoyama010/pyvista-js ? Maybe it's not too much work to use that in place of our PyVista calls... but if you've tried or looked I could be way off! |
Hi Eric, it already uses pyvista-js. On AbstractRenderer, it sits in doc/ as a string the setup cell appends, to keep browser-only code out of mne/. But it already implements 21 of the 22 abstract methods, so converting it is mostly moving and registering it, not a rewrite. Happy to do that here, or land this as is and convert in a follow-up. Which would you prefer? Thanks! |
|
Yeah if there is some way for it to be a plain renderer and then |
The renderer was a 560-line string literal, which no linter or formatter could see. It now lives in _lite_renderer_cell.py as ordinary Python and the cell is read from there, so ruff covers it like any other file. The code itself is unchanged apart from what the formatter did to it.
Done in eb3c814, it's a plain module now and LITE_RENDERER_CELL is read from it, so ruff and the formatter cover it. |
|
I think it's probably best to move this to the I don't want to add 700 lines with no unit tests, things are bound to break / or be broken... |
Done, moved to mne/viz/backends/_lite.py as a real _AbstractRenderer subclass. That turned up a bug, it reported a _kind that isn't "notebook", sending _coreg.py into _qt_app_exec with no Qt event loop in a browser. pyvista-js is pure Python, so no selenium, 18 pytest cases plus one nbexec test. It ships in the wheel now too, so the setup cell drops from ~24,500 characters to 438. I didn't register it in VALID_3D_BACKENDS, since that pulls in the 17 widget classes _do_widget_tests expects. Happy to if you meant that. |
|
Thanks for the quick response @natinew77-creator. I only have phone at the moment but will look again tomorrow morning |
Matches what the review asked for and lines the backend name up with _kind. The capability table goes back to upstream's three columns, with the browser backend described in the notes instead.
It was handing back only the last color group and dropping the rest.
Drops a wrong claim about plot_bem, makes the drawing tests assert geometry rather than actor counts, and stops accepting arguments without saying why they cannot be honoured.
mne-toolsgh-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.
* upstream/main: (37 commits) Speed up evoked time plotting (mne-tools#14249) Persist the numba JIT cache, sysmon coverage, refleak freeze (mne-tools#14265) Fix interpolate_to spline target positions (mne-tools#14266) CI reduce package builds on pull requests (mne-tools#14264) Avoid copying all epochs data in GetEpochsMixin._getitem (mne-tools#14262) Add Report.save(only_if_changed=True) (mne-tools#14261) Add jamica to related software [ci skip] (mne-tools#14260) Remove debugging cruft (mne-tools#14259) ENH: add Raw annotation span conversion (mne-tools#14240) Interactive dipole fitting: add STC mesh controls (mne-tools#14256) Document code principles in AGENTS.md (mne-tools#14239) MAINT: Update dependency specifiers (mne-tools#14257) [dependabot]: Bump the actions group with 2 updates (mne-tools#14258) ENH: Add JAMICA as an ICA method (mne-tools#14247) Reuse the MEF session across reads (mne-tools#14254) Read KIT data in cache-sized blocks (mne-tools#14255) Read EGI simple-binary event channels in blocks (mne-tools#14250) Decode Persyst and Nihon Kohden data in cache-sized blocks (mne-tools#14251) Normalize byte order before calibrating strided integer buffers (mne-tools#14252) Speed up EDF and BDF reading (mne-tools#14237) ...
|
Okay I added it as a proper |
Thanks Eric, this turned out much better than what I had. I appreciate you taking the time to convert it into a proper renderer and to fix the rotation edge case along the way. |
|
Working on fixing the Windows timeout... I don't think it's related so I'm going to do it in another branch, then merge |
Sounds good, thanks. |
|
It's just scipy's intersphinx inventory which has been having trouble all day |
Part 3 of the split of #13925. Parts 1 and 2 are #14128 and #14135.
Adds a drawing backend for MNE's 3D renderer that uses vtk.js, since VTK itself cannot load in WebAssembly. MNE's 3D functions do their geometry and coordinate-frame work in numpy and only hand the result to a renderer, so replacing that last step leaves the transform maths with MNE. That matters here, because a subtly wrong head or device transform still produces a plausible-looking picture.
Supported: meshes, surfaces, spheres, tubes and glyphs, which covers the static figures the docs render. Not supported: the interactive
Braintime viewer, which needs dock widgets, and scalar colormaps, which pyvista-js 0.15 does not have.It is a plain string constant, so nothing in the build touches it yet. The setup cell that appends it is #14150.