Skip to content

MRG, BUG: Fix interactive update of mapping#8985

Merged
GuillaumeFavelier merged 7 commits intomne-tools:mainfrom
larsoner:fix-scalars
Mar 4, 2021
Merged

MRG, BUG: Fix interactive update of mapping#8985
GuillaumeFavelier merged 7 commits intomne-tools:mainfrom
larsoner:fix-scalars

Conversation

@larsoner
Copy link
Copy Markdown
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.12 label Mar 3, 2021
@GuillaumeFavelier
Copy link
Copy Markdown
Contributor

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

@larsoner
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

larsoner commented Mar 4, 2021

Okay @GuillaumeFavelier should be good to go here

Copy link
Copy Markdown
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
Copy Markdown
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.12 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