Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Prototype of MultiPlotter #8997

Closed

Conversation

GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented Mar 5, 2021

I have been experimenting with the new PyVistaQt class MultiPlotter to bring proper SSAO and/or EDL to MNE on #8771 (comment) (among other features like multiple toolbars, menu bars and multi-renderers). And I don't think it's reasonable to do that in one go.

This PR is far from being mergeable but I believe it could drive smaller PRs that can help with 3d backend refactoring in general. For instance:

  • Fix show
  • Create proper Renderer bindings for add_mesh and add_actor
  • Generic way to pick a 3d viewer
  • Get rid of self.interactor in Brain
  • Get rid of self.plotter in Brain and use self._renderer instead
  • Get rid of self.plotter in _Renderer and use self.figure instead
  • Generic way to subplot
  • Generic way to iterate through the 3d viewers
  • Trace dock widget
  • Separate the concepts of "plotter" and 3d "viewer"

@larsoner
Copy link
Member

larsoner commented Mar 5, 2021

I would add to this list, if we haven't done it already:

  • Avoid using PyVista's toolbar and menu bar and create/use our own as needed

@GuillaumeFavelier
Copy link
Contributor Author

Done in here on this PR:

self.store["menu_bar"] = False
self.store["toolbar"] = False

@GuillaumeFavelier
Copy link
Contributor Author

Update after dd0d67d (with experimental margin=False):

image

I see the main challenges being 1) ensuring correct size and 2) managing picking nicely for all the viewers

@larsoner
Copy link
Member

Can you check to make sure volumetric rendering looks okay? Just want to make sure it plays nicely with the ambient occlusion etc.

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Mar 16, 2021

There are some issues with the colorbar:

BackgroundPlotter has no attribute named scalar_bar
SSAO=False SSAO=True
image image
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 stc.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=True, size=(800, 600), background='k',
    show_traces=False,
    **kwargs)

and plot_beamformer_lcmv:

image

@GuillaumeFavelier
Copy link
Contributor Author

It's better after #9121

image

@GuillaumeFavelier
Copy link
Contributor Author

This PR now requires pyvista/pyvistaqt#90.

I plan to move the mplcanvas to a bottom dock and use the new resize_content() function to replace ensure_minimum_size.

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Mar 19, 2021

In my experiments, the mplcanvas as a dock has several advantages:

  • it can be made separate dynamically with the default dock features (meaning that the "separate" particular case can be removed)
  • it does not mess with the central widget layout meaning that the resize_content() function is enough to ensure that the viewer has correct dimensions (important for screenshot). I have to demonstrate that by updating the tests.
  • the size of the dock can be handled separately

This is how it looks locally:

image

Probably the margins can be nullified.

@larsoner
Copy link
Member

Agreed docking is useful, can that be a first PR, separate from the MultiPlotter stuff? It seems orthogonal, easier to review, and will get in sooner than if we tie it into this PR

@GuillaumeFavelier
Copy link
Contributor Author

Circle does not seem ready yet. I'll investigate locally:

RuntimeWarning: No 3D backend found. Resorting to matplotlib 3d

@larsoner larsoner added this to the 0.24 milestone Apr 19, 2021
@larsoner larsoner modified the milestones: 0.24, 0.25 Oct 22, 2021
@GuillaumeFavelier
Copy link
Contributor Author

Closing for now. It already served the purpose of refactor _Renderer and the pyvista 3d backend. I can reopen when I get back to SSAO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants