Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

This PR is extracted from #10224:

  • adds a slider for the head surface opacity (semi-automatic behaviour)

@GuillaumeFavelier GuillaumeFavelier self-assigned this Jan 26, 2022
@hoechenberger
Copy link
Member

hoechenberger commented Jan 26, 2022

Generally I would approve, BUT the default of 0.95 doesn't work for me (in fact, not even 0.99 works for me). I'm pretty convinced there's a bug somewhere, probably in pyvista. As soon as even the tiniest bit of transparency is enabled, everything gets WAY too transparent and the usual "the back of the head looks like the front" happens for me. It's almost like a binary switch between "not transparent at all" and "full-blown transparency" (which, admittedly, can be slightly adjusted using the slider later on) I've never seen such behavior in any software I've used. This just cannot be right

@GuillaumeFavelier I can demonstrate to you what I mean later – out for a coffee now

@larsoner
Copy link
Member

Easy enough to reproduce:

Peek.2022-01-26.10-16.mp4

Enabling depth peeling (just pushed) fixes it for me, but 0.95 seems way too opaque:

Peek.2022-01-26.10-18.mp4

@hoechenberger want to try on my commit and see what the ideal transparency value is to you? To me 0.8 works well because you can clearly see the bad black points:

Screenshot from 2022-01-26 10-20-43

@hoechenberger
Copy link
Member

@larsoner Perfect!! I changed the default to 0.8 and pushed.

@GuillaumeFavelier WDYT?

@GuillaumeFavelier
Copy link
Contributor Author

This is a way better transition 💯

@GuillaumeFavelier
Copy link
Contributor Author

Shall we update mne/gui/tests/test_coreg_gui.py and mne/gui/__init__.py too ?

diff --git a/mne/gui/__init__.py b/mne/gui/__init__.py
index 53719bf5d..ba9a1b00c 100644
--- a/mne/gui/__init__.py
+++ b/mne/gui/__init__.py
@@ -151,7 +151,7 @@ def coregistration(tabbed=False, split=True, width=None, inst=None,
         advanced_rendering = \
             config.get('MNE_COREG_ADVANCED_RENDERING', 'true') == 'true'
     if head_opacity is None:
-        head_opacity = config.get('MNE_COREG_HEAD_OPACITY', 0.95)
+        head_opacity = config.get('MNE_COREG_HEAD_OPACITY', 0.8)
     if head_inside is None:
         head_inside = \
             config.get('MNE_COREG_HEAD_INSIDE', 'true').lower() == 'true'
diff --git a/mne/gui/tests/test_coreg_gui.py b/mne/gui/tests/test_coreg_gui.py
index cfdf6dc84..386f7f979 100644
--- a/mne/gui/tests/test_coreg_gui.py
+++ b/mne/gui/tests/test_coreg_gui.py
@@ -185,7 +185,7 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt):
     assert coreg._mark_inside
     assert_allclose(
         coreg._head_opacity,
-        float(config.get('MNE_COREG_HEAD_OPACITY', '0.95')))
+        float(config.get('MNE_COREG_HEAD_OPACITY', '0.8')))
     assert coreg._project_eeg == \
         (config.get('MNE_COREG_PROJECT_EEG', '') == 'true')
     assert coreg._hpi_coils

@larsoner
Copy link
Member

Please do @GuillaumeFavelier

@hoechenberger
Copy link
Member

CI errors are due to download problems. We should try to restart in a few minutes (or in an hour??) or so

@larsoner larsoner merged commit 0accf8c into mne-tools:main Jan 26, 2022
@larsoner
Copy link
Member

Thanks @GuillaumeFavelier !

@hoechenberger
Copy link
Member

🥳

@GuillaumeFavelier GuillaumeFavelier deleted the enh/coreg_opacity branch January 26, 2022 18:08
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.

3 participants