MRG: Bump colorbar control points

[GuillaumeFavelier]

This PR furthers the discussion around the control over the end points of the colorbar in _TimeViewer and offers a prototype that 'snaps back' the value when they reach the limits (idea coming from @agramfort).

Here is an animation:

what do you think @larsoner ? It's an alternative for the bumping of the values.

It's an item of #7162

---

ToDo

• Set the limits of all three to be the same, min/max of the abs(data) (suggested in MRG: Bump colorbar control points #7188 (comment))
• Rollback event_type to default (suggested in MRG: Bump colorbar control points #7188 (comment))

[larsoner]

Rather than snap back I think it's more user friendly to bump the other values down

[larsoner]

(and I would set the limits of all three to be the same, min/max of the abs(data))

[larsoner]

... one reason for this is because as I user I expect that if I do an action, the GUI will accommodate it as much as possible. So If I say "I want the max to be X" via a click and drag, it should try to set the max to X, even if it means changing the mid and/or min. By contrast, what will happen with the current iteration is that the user will release the mouse, see the snap, then need to change mid to get their desired max. And maybe that won't even isn't enough because they might need to also change the min, then change the mid, then change the max to get their desired max value.

Even the extreme case where the user sets the max such that it's less than the min can be useful, for example, to make it so that it basically becomes a binary mask (on or off) and you can easily change the limits of the mask by sliding the min up or max down. Getting this behavior would be much more cumbersome with the current snap behavior.

A sort of in between would be what GIMP does with "levels" adjustment, which is to allow changing the min or max to slide the mid (actually they maintain ratios), but not allow min to exceed max or vice-versa. I can live with this, too, but still think I prefer my suggested behavior.

[codecov]

Codecov Report

Merging #7188 into master will increase coverage by <.01%.
The diff coverage is 91.5%.

@@            Coverage Diff             @@
##           master    #7188      +/-   ##
==========================================
+ Coverage   89.75%   89.76%   +<.01%     
==========================================
  Files         445      445              
  Lines       79726    79860     +134     
  Branches    12754    12772      +18     
==========================================
+ Hits        71562    71688     +126     
- Misses       5370     5374       +4     
- Partials     2794     2798       +4

[GuillaumeFavelier]

This is an animation using BumpColorbarPoints instead:

[larsoner]

Awesome! Now just need to make them all have the same left/right limits for the slider

[GuillaumeFavelier]

At the same time I tested a different event_type=always (that manages refresh rate) for the colorbar sliders to get a smoother experience but I noticed some 'lag' so I will rollback.

[drammock]

Awesome! Now just need to make them all have the same left/right limits for the slider

and then superimpose them, make 2 of them transparent, and color-code the dragging handles :)

[GuillaumeFavelier]

I tested this offline @drammock and I wasn't satisfied so far.

Source code: https://gist.github.com/GuillaumeFavelier/ee40c130b7bbf65e2877a0ef24b2d8eb

[drammock]

I tested this offline @drammock and I wasn't satisfied so far.

yeah, seems like it's not very robust. Too bad.

[larsoner]

It's really too bad that the auto-updating event_type='always' from your previous iteration does not work well for you. It really enhances the interactive experience for me. What was the problem you were hitting?

I tried a potential fix that doesn't do the draw callback any faster than 30 times a second as a heuristic, maybe it will fix things for you.

In that same commit I also have a fix for some "flashing" at the extreme of the colormap I was getting, probably because the old code would sometimes set fmax=fmid. This ensures that fmin<fmid<fmax is maintained, and that they differ from one another by at least eps.

eps is also now calculated relative to the limits, which is important because scales can change drasntically (MNE will be currents in Am, but the values actually are in the range of nAm, i.e. 1e-9, whereas dSPM will be noise-normalized and thus on the order of unity).

I'll push once I tweak it and test locally...

[larsoner]

Pushed a commit to:

1. Allow setting colorbar fmin=fmid=fmax in sliders, moving logic for how to handle this case gracefully in the calculate_lut function, as since it's really a display problem (and was much simpler than dealing with some eps that depends on the scales of fmin, fmid, fmax, etc.)
2. Ensure that we always have fmin <= fmid <= fmax in the calculate_lut call
3. Restore update on the fly (which is awesome!)
4. Don't allow drawing more than 30 FPS during on-the-fly updates, hopefully will help with lag issues

This fixed the flashing issues for me and everything works smoothly and as expected at my end now. @GuillaumeFavelier can you confirm the same at yours?

[GuillaumeFavelier]

Don't allow drawing more than 30 FPS during on-the-fly updates, hopefully will help with lag issues

This drastically improved the drag & slide experience and I don't notice any 'lag'. Thank you @larsoner !

[GuillaumeFavelier]

What do you think @agramfort ?

[larsoner]

In that case, can you set it to 1/60 instead and if that works, commit the change?

[agramfort]

works perfectly @GuillaumeFavelier ! thx