Skip to content

WIP: Fix bugs in Brain#8361

Merged
larsoner merged 6 commits intomne-tools:masterfrom
GuillaumeFavelier:disable_render
Oct 14, 2020
Merged

WIP: Fix bugs in Brain#8361
larsoner merged 6 commits intomne-tools:masterfrom
GuillaumeFavelier:disable_render

Conversation

@GuillaumeFavelier
Copy link
Copy Markdown
Contributor

This PR disables the rendering call each time a mesh is added to the scene in favour of only one call when everything is setup.

Quoting pyvista/pyvista#935 (comment):

There are 2 main reasons for this:

  • Huge speed gain when hiding and displaying meshes.
  • It looks much more professionnal when clicking a button to see all the changes at once.

@larsoner
Copy link
Copy Markdown
Member

@GuillaumeFavelier locally on master with a script similar to:

Details
import os.path as op
import mne
data_path = mne.datasets.sample.data_path()
sample_dir = op.join(data_path, 'MEG', 'sample')
subjects_dir = op.join(data_path, 'subjects')
inv = mne.minimum_norm.read_inverse_operator(op.join(
    sample_dir, 'sample_audvis-meg-vol-7-meg-inv.fif'))
    # sample_dir, 'sample_audvis-meg-oct-6-meg-inv.fif'))
evoked = mne.read_evokeds(op.join(sample_dir, 'sample_audvis-ave.fif'))[0]
evoked.apply_baseline((None, 0)).pick_types(meg=True)
stc, res = mne.minimum_norm.apply_inverse(
    evoked, inv, return_residual=True, pick_ori='vector')

initial_time = 0.1
# stc, kwargs = stc, dict(overlay_alpha=0.5, brain_alpha=0.5, vector_alpha=0.)
stc, kwargs = stc.magnitude(), dict(alpha=0.5, surface='white')
kwargs['src'] = inv['src']
meth = stc.plot_3d if isinstance(stc, mne.VolSourceEstimate) else plot
brain = meth(
    subjects_dir=subjects_dir, initial_time=initial_time,
    clim='auto', hemi='both', views=['sag', 'axi', 'cor'],
    view_layout='horizontal', smoothing_steps='nearest', verbose=True,
    time_viewer=False, size=(800, 300), background='k',
    **kwargs)

I see this "flashing" (sorry the window disappears -- I'm talking about the meshes and outlines appearing and disappearing):

Peek 2020-10-13 08-16

And on this branch and latest PyVista master it doesn't seem to be improved much. Do you see the same thing? Can you see if it can be improved?

Actually with this example script it's a bit worse -- the display doesn't update until I click on the window (ignore the "blue brain" in the background -- the window produced by the script above will appear on top of it, flash for a bit, then not display the final image until I click on the window to bring it to focus):

Peek 2020-10-13 08-26

@GuillaumeFavelier
Copy link
Copy Markdown
Contributor Author

Thanks for reporting, I can reproduce. I will investigate in this PR (already opened).

@GuillaumeFavelier GuillaumeFavelier changed the title MRG: Disable render in add_mesh WIP: Disable render in add_mesh Oct 13, 2020
@GuillaumeFavelier GuillaumeFavelier changed the title WIP: Disable render in add_mesh WIP: Fix bugs in Brain Oct 13, 2020
@GuillaumeFavelier
Copy link
Copy Markdown
Contributor Author

For me, 0372cd2 improved the situation greatly. In plot_source_estimates(), show() is called last, when everything else is set basically. Could you try it now @larsoner ?

Copy link
Copy Markdown
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, works with both time_viewer=True and False!

@larsoner
Copy link
Copy Markdown
Member

@GuillaumeFavelier I'm +1 for backporting, any reason not to?

@larsoner larsoner added the backport-candidate on-merge: backport to maint/1.12 label Oct 14, 2020
@GuillaumeFavelier
Copy link
Copy Markdown
Contributor Author

I see no problem here 👍

@larsoner larsoner merged commit 3c8f49d into mne-tools:master Oct 14, 2020
@larsoner
Copy link
Copy Markdown
Member

Thanks @GuillaumeFavelier

@GuillaumeFavelier GuillaumeFavelier deleted the disable_render branch October 14, 2020 12:18
@larsoner
Copy link
Copy Markdown
Member

Nope, not backporting -- the conflict due to the TimeViewer / Brain refactor was ugly

@larsoner larsoner removed the backport-candidate on-merge: backport to maint/1.12 label Oct 14, 2020
@GuillaumeFavelier
Copy link
Copy Markdown
Contributor Author

Ouch, my bad. I thought it was straightforward to cherry-pick this.

larsoner added a commit that referenced this pull request Oct 14, 2020
* Update _add_mesh

* Update mne/viz/backends/_pyvista.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Show the plotter when ready

* Remove empty line

* Fix save_movie action

* Fix show

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@larsoner
Copy link
Copy Markdown
Member

Actually I think I figured it out: 35662d0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants