Add per sweep update for OVS and lots of minor stuff on the way to that #573
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
t-b
force-pushed
the
feature/add-remove-support-for-graphs-v2
branch
from
eeecdb0 to
0bed3c5
Compare
Collaborator
|
@t-b dis/enable OS with PA active results in a unknown subset of selected sweeps being plotted. disable OS |
t-b
force-pushed
the
feature/add-remove-support-for-graphs-v2
branch
from
0bed3c5 to
5f0e6d1
Compare
Collaborator
Author
|
@timjarsky That was dumb. Please try again. |
timjarsky
previously approved these changes
Jul 16, 2020
This allows us to call static functions from the tests as well.
ScaleToIndex was rectified in egde cases in latest IP8 nightly.
By properly faking a SweepBrowser we avoid future problems when the checks on the graph properties are tightened.
This also makes the call sites easier to understand.
No callers are interested in the current return value of a string with "Sweep_XXX" formatting. So by just returning a numeric wave with the sweep numbers the call sites will have less code. This also fixes a theoretical bug in DB_FirstAndLastSweepAcquired as it assumed that the sweep numbers are continous. We are now returning the first and last values instead.
The only differences in DB_CheckProc_ChangedSetting/SB_XXX and DB_ButtonProc_RestoreData/SB_XXX were either minor bugs or insignificant. So let's have common routines. This also allows us to get rid of BSP_SetCSButtonProc.
There is no point in doing it in code when it is fixed.
We used to always branch out when fetching the labnotebook waves. This is error prone, duplicated code and can be done better. We now have BSP_GetNumericalValues and BSP_GetTextualValues which return a wave with the labnotebooks of all requested sweeps. For easier usage it returns the labnotebook wave directly when only the labnotebook for a single sweep is requested. The sweep browser labnotebook functions now also behave the same way. This also allows us to remove DB_UpdateOverlaySweepWaves.
We need to turn off OVS and select none as well in the popup menu.
…cedures This mait easy to find missing ones manually.
GUI control procedures can act on mouse over events. So the code outside the switch cases should be fast. But we had quite some cases where we started setting some variables. Move these into each case.
We don't have that dimension label anymore since 0b9e8de (OverlaySweeps: Don't replot when we want to highlight a sweep, 2020-07-13).
This allows to optimize for the common case of Artefact removal being turned off.
Since forever we just bugged out on an invalid headstage in the ignore list. We now complain and ignore invalid ones.
Forgotten in 0b9e8de (OverlaySweeps: Don't replot when we want to highlight a sweep, 2020-07-13).
We always pass it.
…on wave We always pass a valid wave now from the only caller in CreateTiledChannelGraph.
Our existing handling of the returned wave from OVS_ParseIgnoreList is duplicated in two places. Future per-sweep additions will require us to have that functionality in even more places, so it is a good time to move that funtionality into its own function.
t-b
force-pushed
the
feature/add-remove-support-for-graphs-v2
branch
from
5f0e6d1 to
2d8c75f
Compare
Collaborator
Author
|
@timjarsky Thanks for the fast testing and approving. But I left out a crucial speedup. Could you retest? Thanks! |
t-b
force-pushed
the
feature/add-remove-support-for-graphs-v2
branch
2 times, most recently
from
892c3ed to
7c8d506
Compare
We always want to layout again if we are finished with the plots itself.
For the Databrowser/Sweepbrowser we currently always replot everything when something changed. This is quite wasteful. The newly added functions AddSweepToGraph/RemoveSweepFromGraph and UpdateSweepInGraph allows to do that.
These are unused since a17f821 (Fix plotting diverse data, 2020-07-13).
This is much faster than a full replot.
This allows us to really always use the generic function UpdateSweepPlot. The only two call sites which are affected now need to set the other control correctly. One of them did that already.
…ilable for all This makes the code cleaner and is also slightly faster for the SweepBrowser as we now use the plain sweep list instead of the one with full names for the popup menue.
This makes the code easier to understand.
The transition is faithful as OVS_ChangeSweepSelectionState also check if OVS is active.
…ingle sweep When OVS_ChangeSweepSelectionState is called we don't need to update all sweeps but that function can just update the selected sweep. We still need to do a full update if overlay sweeps is not on, but that is also cheap as we just have a single sweep.
We don't need that here.
In both cases we don't need to update all sweeps. When a new sweep is selected/unselected we can just add/remove that one. And on finish editing we just update that sweep.
We used to always replot everything if the OVS listbox selection or the contents wave changed. This is quite wasteful. The new functions OVS_BeginIncrementalUpdate and OVS_EndIncrementalUpdate allow to only update the sweeps which need updating.
t-b
force-pushed
the
feature/add-remove-support-for-graphs-v2
branch
from
7c8d506 to
2f32f68
Compare
timjarsky
approved these changes
Jul 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.