Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented Feb 10, 2021

Locally I was able to reproduce #8757 (comment) and this small fix did the trick for me. Let's see how it is on Circle.

@GuillaumeFavelier GuillaumeFavelier self-assigned this Feb 10, 2021
@GuillaumeFavelier
Copy link
Contributor Author

I think it's worth restarting the CIs, even though processEvents() is not called directly, I don't want to introduce regressions.

with _qt_disable_paint(self.plotter):
with self._ensure_minimum_sizes():
self.show()
self._update()
Copy link
Member

Choose a reason for hiding this comment

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

This will only happen if we call setup_time_viewer, which only happens if time_viewer=True (effectively) is used, right? This suggests that if time_viewer=False then the bug will still be there. It seems like we Probably need an equivalent change in very similar code in _pyvista.py, or some other fix, or maybe this belongs at the Brain level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're totally right. And this code in Brain can be removed since self.show() calls self._renderer.show(). I'll update on _pyvista

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... and cannot be removed since ensure_minimum_size has a different version for Brain.

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Feb 10, 2021

This is ready on my end.

@larsoner larsoner merged commit 5faf637 into mne-tools:main Feb 10, 2021
@larsoner
Copy link
Member

Thanks @GuillaumeFavelier

@GuillaumeFavelier GuillaumeFavelier deleted the fix/paintEvent branch February 10, 2021 21:00
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