Skip to content

Conversation

@larsoner
Copy link
Member

@larsoner larsoner commented Mar 3, 2021

On main:

Peek.2021-03-03.12-05.mp4

On this PR:

Peek.2021-03-03.12-05_2.mp4

I don't know a good wait to test this on CIs unfortunately since it has to do with actual rendering.

@larsoner larsoner added the backport-candidate on-merge: backport to maint/1.11 label Mar 3, 2021
@GuillaumeFavelier
Copy link
Contributor

we could check that layered_mesh._rng == brain._cmap_range?

@larsoner
Copy link
Member Author

larsoner commented Mar 3, 2021

we could check that layered_mesh._rng == brain._cmap_range?

Great idea, pushed. Will merge and backport once CIs come back happy

@larsoner
Copy link
Member Author

larsoner commented Mar 4, 2021

Okay I fought through a bit of a nightmare with control flow with LUT updates. Now:

  1. by use of a context manager we hopefully avoid one param change causing multiple .update calls (avoid time-based update limiting in the process)
  2. make it so programmatic brain.update_lut(...) calls are reflected in the widgets
  3. fix rng[0] == rng[1] not displaying properly -- now it creates what is essentially a binary mask at rng[0] which is what I'd expect

This also makes interaction smoother for me, which is nice. Ready for review/merge from my end @GuillaumeFavelier, hopefully CIs come back happy with this one...

@larsoner larsoner changed the title BUG: Fix interactive update of mapping WIP, BUG: Fix interactive update of mapping Mar 4, 2021
@larsoner
Copy link
Member Author

larsoner commented Mar 4, 2021

Actually I think I realized a simpler pattern, will implement it tomorrow

@larsoner larsoner changed the title WIP, BUG: Fix interactive update of mapping MRG, BUG: Fix interactive update of mapping Mar 4, 2021
@larsoner
Copy link
Member Author

larsoner commented Mar 4, 2021

Okay @GuillaumeFavelier should be good to go here

Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier left a comment

Choose a reason for hiding this comment

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

LGTM, it is indeed very snappy now 👍

@GuillaumeFavelier GuillaumeFavelier merged commit 0135fca into mne-tools:main Mar 4, 2021
@GuillaumeFavelier
Copy link
Contributor

Thanks @larsoner !

@larsoner larsoner deleted the fix-scalars branch March 4, 2021 15:21
@larsoner larsoner added backported and removed backport-candidate on-merge: backport to maint/1.11 labels Mar 4, 2021
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