Also how many data can we have on a hemi? I would like to avoid duplication for that too so I'll go ahead with the naive solution next.

Waiting for the first CI report then I'll push again.

Store normals to avoid unnecessary computation and waste of memory

These should already live in self.geo[hemi].nn, if not then load_geometry should be updated to add it using mne.surface._compute_normals

Also how many data can we have on a hemi? I would like to avoid duplication for that too so I'll go ahead with the naive solution next.

Eventually we will want to support having multiple data overlaid, possibly with different colormaps (?), but we probably need to refactor quite a bit to support it anyway

Running mayavi on this PR (should be the same as master):

MNE_3D_BACKEND=mayavi BUILD_DEV_HTML=1 mprof run sphinx-build -D plot_gallery=1 -D sphinx_gallery_conf.filename_pattern=source_space_snr -b html -d _build/doctrees  -nWT --keep-going . _build/html

I get:

Adding time_viewer=False, show_traces=False (to make it more equivalent) and MNE_3D_BACKEND=pyvista I get something very similar:

On this PR it looks like memory is reduced (hooray!), but only by a few hundred MB (better than nothing):

Removing the time_viewer=False, show_traces=False (implicitly thus setting them to True) we then get the monster:

So it must be something about these options. Let's try show_traces=False, time_viewer=True (good!):

And then show_traces=True, time_viewer=False (also good, must be because show_traces has no effect when time_viewer=False):

@GuillaumeFavelier it seems that the problem is really having both of these enabled at once, which means it's something about how we do show_traces=True I wonder if it's the matplotlib callbacks or something... In any case, how about this plan:

• You sort out the actor/data reuse in this PR, and we merge it (the few hundred MB savings are worth it already, and orthogonal to the memory usage issue)
• we make a new PR to make show_traces=True, time_viewer=False raise an error (currently you have to have time_viewer=True to have show_traces actually work)
• deal with the show_traces+time_viewer problem in another PR
• I add a _MNE_BRAIN_TIME_VIEWER_AUTO=false to CircleCI that will be respected by _check_time_viewer_compatibility in the meantime -- it will allow us to get nice traces on our plots in MRG, ENH: Scrape traces when available #7927 while we work on the show_traces+time_viewer problem

Never mind that last point, it doesn't make any sense. I'll work on looking into the show_traces mem problem...

I think I found the circular ref, it was passing the times and act_data[vert_idx] to mpl_canvas.plot, which created a nasty circular reference. Passing times.copy() and act_data[vert_idx].copy() seems to fix things. Will push to #7927

Other huge memory blowup is actually storing self.act_data[hemi] = smooth_mat.dot(act_data), i.e., full-resolution data (!). We shouldn't need to do this and can just smooth_mat[idx].dot as needed

While messing with this, I notice this in testing:

15.11s call     mne/viz/tests/test_3d.py::test_plot_alignment[pyvista]
8.61s call     mne/viz/tests/test_3d.py::test_plot_alignment[mayavi]

PyVista is much slower for plot_alignment. It's possible that this is because of the normal computation. We should be able to pass our own computed nn directly to renderer.mesh(..., normals=nn) to avoid this.

Basically I think every call to renderer.mesh(...) in our codebase should pass normals and (in PyVista) set smooth_shading=False. I'll add it to the todo, but we can remove if you disagree.

I'll add it to the todo

It's a good idea. I mean if those infos are available, we should use them instead of re-calculating from scratch.

... and even if they're not available:

1. We know that PyVista's computation via VTK can be problematic / cause crashes on some platforms
2. We need to compute them for Mayavi anyway, so it reduces the number of code paths we need to consider by always computing and passing the normals directly to each renderer backend

In the current state, we're close to the goal but I'm not too sure about the memory gain anymore, I need to run mprof. Also smooth shading was already disabled by default for renderer.mesh().

At least we don't re-create a PolyData when possible and use the given normals data.

Also smooth shading was already disabled by default for renderer.mesh().

Yes, we passed smooth_shading=False to PyVista. This fixed some VTK segfault bug, but introduced a nasty visual regression in that the surfaces were not smooth shaded anymore.

In this PR, because we provide the normals directly/manually, the surfaces should be smooth shaded now. Are they?

When the normals parameter is passed to mesh(), the infos are used to allow Phong smooth shading.

Great, Phong is back! Thanks @GuillaumeFavelier