ngscopeclient
Delete key doesn't delete filters
Hi,
Just downloaded and built "ngscopeclient 0.1-dev-9107f172". In the 'Filter Graph' window I can't delete the filters with the delete key - I can delete the connections between filters - but the filters can't be deleted. No message appears to be printed anywehere.
There also seems to be no other way to delete them (I would, from a normal UI view, expect to be able to delete them from the RMB pop-up menu on a filter).
Thanks, Matt
Filter deletion per se is not currently possible due to some awkwardness in the data model.
Tl;dr filters are reference counted and self-delete (in theory, there are resource leakage bugs that can make them un-deleteable - if this happens you can save the session and reload and it should be gone) when the last graph, filter input, etc. using their output is removed. This was a bad decision that made sense back in 2015 or whenever I started implementing filter graph stuff and is now firmly entrenched enough that ripping off the band-aid will be painful.
We currently track dependencies only in reverse direction (i.e. given a filter, what inputs does it depend on) and there's no way to iterate in the forward direction (given this filter, who is using it). This makes it impossible to safely/easily delete a filter since you have no way of knowing who might still be holding a reference to you that you need to disconnect.
This is planned as part of a major refactoring in between v0.1 and v0.2 but isn't going to happen before v0.1 since we just need to tie up some loose ends on the build system and packaging and fix a few small bugs before that happens. It's going to touch everything - every portion of the GUI that deals with waveforms, every instrument driver, every filter. The diff will probably be 100K+ lines and I'm dreading starting it because it can't be done atomically, it's going to be making fundamental object model changes that have to be done across the entire application at once.
Keeping this ticket open since I don't think we've clearly explained this limitation anywhere.
Just to give a few more details on what will be involved for anybody curious (and to explain why this is such a big job):
- Filters are currently one of the few key objects that use raw new/delete rather than std::shared_ptr, this needs to change
- Filters and instrument channels are derived from the same base class, where the refcounting is implemented. When refcount hits zero, filters self-delete while instrument channels are turned off in hardware to avoid wasting bandwidth downloading waveform data nobody is going to use.
- The number of direct calls to AddRef/Release is fairly small (14/16 from some quick grepping), so at least fixing these won't be too bad.
- All code that handles instrument channel or filter objects will have to change from working with
InstrumentChannel*tostd::shared_ptr<InstrumentChannel> - We need to make the filter graph data structure bidirectional (adding a list of sinks to each source) and make some new APIs to keep them in sync. These APIs will have to be breaking (changing variable names or making things private/protected) in order to enforce that all existing code is updated to use this new API and we never manipulate e.g. m_inputs directly
- We need to figure out how to turn off instrument channels when not in use given that we no longer have the Release() hook point
- We need to figure out how to manage instrument channels themselves since they will now have to be shared_ptr's and this will likely break some things drivers do
Once all of this is done, we can start thinking about using these new tools to implement the delete-filter feature everybody wants.
(this refactoring is probably also going to be a good time to address https://github.com/ngscopeclient/scopehal/issues/970 which is also sorely needed but didn't have a ticket filed AFAIK until now)