Talk:Proposal TrackPanel Evolution

From Audacity Wiki
Jump to: navigation, search

Use of Talk page

We use talk page for talk - especially divergent views. Main page (generally) for non-contentious aspects of proposal.

Paul's Thoughts

PRL 3 Jul 2015: Since the page was last updated, old track panel gained some nice new things like spectral selection and scrubbing, which surely we want in some form in future versions with the newer appearance.

  • JKC: Scrubbing already exists in the AU14 version. In AU14 if you drag the selection left edge whilst playing faster than the play cursor, it pushes the play cursor to that position. There is no spectrogram (yet) in AU14. All track types can show in both vertical and horizontal mode, using the same code. All track types can be used as overlays, rather than as just tracks on their own. Selection needs to be, or could be, generalised beyond spectral selection and multiple selection. It is just an overlay over multiple tracks. There are also (not implemented) the ideas of fuzzy selection, i.e. a 0-to-1 weighted selection, and generated-selection, e.g. select-where-clipped, which is updated as you make other changes. This doesn't invalidate the point about re-using significant existing code. It is just a pointer to where the API for selections should be heading.

Much coding and testing effort has already been expended to make old track panel features work right. Users accustomed to the old behavior and appearance may prefer them. I hope the intention is to preserve as much as possible of the good things in the trusted code, rather than burning it all down and rebuilding from nothing and discovering many bugs to fix all over again. But I am uncertain what is really proposed on the page. Some of what I read suggests one thing, some the other. (I was told that the AU14 experimental track panel did not even have its own spectrogram. Do we really want to rebuild spectrogram code all from nothing?)

Defining abstract class interfaces that break the compilation and linkage dependency of TrackPanel on the special cases is a first step toward migration to a real plug-in protocol that would allow existing features to be salvaged as dynamically loaded modules on a par with the proposed new features and recombinable with them. Class TrackPanel has too many responsibilities now, but breaking up that monolith only needs patience and is not an intractable problem. I see no reason why existing parts of track panel cannot be recoded to new API protocols -- once we decide what those should be.

I propose a cleanup chore for the many click and drag interactions and popup menus that TrackPanel now supports, which are not addressed at length on the page, which mostly discusses appearance.

This proposal alone will be much effort to add no new capabilities and fix no bugs, but it will move existing features in the right direction for salvage in a new framework. TrackPanel.cpp will be thousands of lines shorter than its present 10k+ line count, and class TrackPanel will have fewer member variables. The old TrackPanel will be more of a high level manager class and dispatcher of events, oblivious to detailed handling, as it should be.

  • Define a new abstract class UIHandle. All methods take a track rectangle and a pointer to a track among their arguments -- not necessarily for the same track during all events of a click-drag-release motion. All but the first indicate whether to redraw after exit. Methods include:
    • Hit test, which takes mouse state (including modifier key states), and returns a pair of pointer to cursor and a status bar message, or NULLs.
    • Button down, taking a mouse event, assuming hit test was positive, and returning an indication whether dragging has begun.
    • Move, assuming dragging is in progress.
    • Button up, taking a wxEvtHandler* argument (which points to the TrackPanel), assuming dragging was in progress.
    • Cancel, as for ESC key, assuming dragging was in progress, returning a value to indicate whether cancel was handled.
  • Class Track has an abstract method returning an array of pointers to UIHandle objects.
    • TrackPanel uses this method but is not responsible for memory management.
    • Each subclass of Track overrides the method.
    • Storage will probably be per-project and per-subclass -- not per-track, because the track in question is always given as an argument, but neither global, as there may be per-project state as for the selection. (Or the project might become an argument too and the arrays could then be global?)
    • Cursor tool will affect the behavior of the handles, but that will be in the special code for the handle classes. TrackPanel will be oblivious to the choice of tool.
  • When not dragging, TrackPanel determines which track contains the mouse position, calls its method to obtain the array of handles, and calls the handles in sequence until one reports a hit. The sequence of handles in the array is thus significant. (Maybe call methods for two tracks when the pointer is near enough to the bottom of one and top of another.)
  • Track panel updates cursor and status message according to the hit test.
  • Track panel passes mouse button clicks to the UIHandle that was hit. If the button down method indicates that drag begins, Track panel remembers a pointer to that handle.
  • During dragging, TrackPanel dispatches movement, button up, and ESC keypress events to the current handle. Button up always ends dragging, and ESC key ends dragging if the Cancel method of the UIHandle agrees.

Pop-up menu code will also be moved out of TrackPanel.

  • Construction, population, and destruction of menu objects, and handler functions for the menu items, will not be in class TrackPanel, but in the UIHandle subclasses.
  • A UIHandle object can simply call ::PopupMenu() as needed to display the menus.
  • The UIHandle class can use the wxEvtHandler* argument in button-up to Connect() command event handlers to the TrackPanel, so that menu clicks dispatch correctly to it, and Disconnect() again when the PopupMenu() call returns.

Subclasses of UIHandle will handle:

  • For all track types:
    • Bottom edge of track, for resizing
    • Track control buttons
      • Close
      • Minimize/Maximize
      • Drop-down menu
    • Default for track control to toggle selection state and reorder the tracks
    • Default time selection adjustment (not that Track* argument during move allows toggling of selected status of other tracks than that in which button down happened)
    • Default for new time selection
    • (Fisheye handles for resizing, zooming, and moving)
  • For Time track:
    • Moving or deleting existing control points, adding new points (perhaps one UIHandle only is needed)
    • (New vertical ruler for changing linear/log scale with a menu, similar to proposal for Wave track here?)
  • For Label track:
    • Circle
    • Arrowhead
    • Text box
  • For Wave track:
    • Separator of stereo channels
    • Additional track controls
      • Mute
      • Solo
      • Pan
      • Zoom
    • Vertical ruler
    • Envelope
    • Samples
    • Cut line
    • Boundary between touching clips
    • Adjusting existing selection, including all the spectral selection details
    • Default handler for new selection, zoom tool, time-shift tool, double-click selection of clip (sharing some time selection code with that for all track types)

What's not in the scope of this proposal:

  • Class AdornedRulerPanel implements the time ruler independently of TrackPanel. Perhaps a similar dispatch protocol could be implemented, reusing the same UIHandle class.
  • I haven't discussed mouse wheel rotation handlers. That may be an easy extension.
  • Drawing of tracks might also be handled by virtual methods of tracks, but for now TrackPanel and TrackArtist share that responsibility, treating the track objects as mostly passive containers of data, so some special purpose stuff will remain in class TrackPanel.
  • The TrackPanel timer method still updates the display for playback and recording and still polls mouse position to determine the speed and direction changes for scrubbing. More special case stuff that would remain in the class.

James' Thoughts

  • TrackPanel is owner-draw rather than being standard widgets. For AU14 TrackPanel I have in effect reimplemented some aspects of wxWidgets, such as window sizing, click and drag dispatching. The reimplementation appears to be needed to do things like overlays and the multiscroller style of scroller.
  • I have chosen to implement this as a flow graph in an attempt to make mouse clicks and drags and changes in animation state be just like any other change in a variable. In the flow graph each change causes consequential changes to propagate. This didn't work out. There is still special case code for mouse.
    • PRL: Can you elaborate on the "flow graph?"
    • What's to say? It is a data flow paradigm. It's a DAG. Each node tracks its state. Each node is reasonably meaty, not just a single int variable, to make the tracking overhead worthwhile. Otherwise why not just recalculate immediately? Sub window sizing is part of the DAG. I push value-invalidation in the direction of the DAG arrows. One of the top nodes is the screen. Every 1/30th of a second I pull the screen state png and display it. Compared to wxWidgets this completely eliminates the flicker that typically happens when you resize a window. In an ideal DAG, for efficiency, you want to be able to say which parts are push-on-change and which parts are pull-to-get. In this hybrid you pay a bit more for the tracking, but you don't have to specify where is push and where is pull, and you only evaluate nodes the minimum number of times needed.
      • PRL: So it's all about graphical update of only those rectangles that really need it?
      • JKC: And for more complex displays would avoid recalculating intermediate data (e.g. FFTs) where the underlying waveform hasn't changed. It's a general system, so whatever intermediate data is, be it data about clipping, or RMS, you only recalculate it if you need to, and you wouldn't write separate kinds of code for data invalidation for each kind of cache. I didn't implement those, but that's the idea.
    • A point the DAG model does bring out is that you do not want lots of different data-flow connection point types. You want the connections to all be compatible so that you are not re-writing the 'same' update code over and over. That argues against having an API for click, drag, zoom, key, sound-packet, time-step, data-change within the tracks. Once you're in the DAG every thing wants to be data-change. It shouldn't matter whether the data change originated with a key press, a foot pedal, a mouse click, a timer, a menu item or a mouse drag. The track just wants to respond to some of its data changing.
  • Completely redoing the TrackPanel allows a new license to be used for that code. We could have a non-GPL compatible license that prevents eBay resellers. We could also dual license, so that long term developers could do proprietary forks that are funded. Vaughan has successfully done consulting on custom (GPL) versions of Audacity. This consulting is one thing that makes his work on Audacity sustainable.
  • Completely redoing the Trackpanel allows a break with the past. The AU14 TrackPanel does not have modal editing (i.e. tool modes). Instead what you are hovering over determines what your clicks and drags do.
    • PRL: It would be good to remove the modes and just have handles, yes. But I don't think that alone argues for throwing away all of the old code that performs the clicks and drags.
    • JKC: To be properly modeless and use handles lots of the old code would go though...
  • The outline of the UIHandle class is OK, as far as it goes. I use a Node class as the base. Either way, that class is a minor part of the design. A more significant part of the design is how components get wired together, how that is specified.
    • PRL: This does not sound like a vote against attempting this reorganization of old code and salvage of old parts of the user interface. Surely the UIHandle class would not be enough to migrate old parts into a new framework, but it would be a beginning.
    • JKC: I'm not voting against it. I think it will help us get each Track Type properly in its own Track file, and I am in favour of that.
  • Track types will need to expose their preference variables, so that they are available to a central system.

James' Thoughts on the Evolutionary Route

  • Salvaging code from the 'old' track panel is clearly more evolutionary than jumping to the AU14 TrackPanel. I think the spectrogram code is the strongest argument for the evolutionary route. There is very little to salvage in envelope code and waveform code, and what is there is actually 'wrong'. Specifically if you use the existing waveform code you will not get a seamless zoom as you zoom in continuously. PRL: Drawing by other means is desirable, but beyond the scope of what I am suggesting to reorganize click and drag interactions. I don't see a directly relevant objection to my proposal, if it does not alone solve every problem of reuse. Also the data querying is entangled in a messy way with the drawing code. Untangling it is pretty close to doing a rewrite. Salvaging code increases the likelihood of lock-in to older interface ideas. Dragging individual points on the envelope is a very primitive way to change its shape. It's as if you were given only the sample dragging interface to edit waves. So a re-write encourages you to spot that wave is a special case of envelope, where the time points are at regular intervals. In a pure evolutionary approach you maintain old interface ideas, and having tidied up the old ways of zooming, selecting, modifying, you're invested in not replacing them with better ways.
      • PRL: I do not see how it follows that the evolutionary approach encourages such conservatism. Surely clicks, drags, and releases will be part of whatever interface is chosen and coding each click-drag-release motion to a common abstract base class interface is a good idea. You may discover the commonalities that lead to code simplification as you pull out the small specialist subclasses for the several interactions. You are left with complete classes for each, rather than the scattered functions for each that we now have, and it is easier to understand how to make a new interaction on that pattern or modify the existing ones.
      • JKC: It does encourage conservatism a bit. If you invest a lot of energy in making a cleaner version of the code with the same UI for zooming as before, you probably don't want to then replace that code with a different way of zooming. Because I was being radical I was doing cleaner UI and cleaner code at the same time. Cleaner UI and cleaner code motivated and changed the design of each other. You lose that a bit if you try to do one on its own.
    • Better zooming - Continuous via the ruler dragging
    • Better selection - Multiple selection, which if modified during playback provides scrubbing.
    • Better modifying - Modification of envelope in same way as modification of wave.
      • PRL: Again I don't see how any of those good ideas is incompatible with the reorganization.
      • JKC: They're compatible with it. But if you've just invested time in incrementally fixing up bugs in selection due to poor design of selection, that's probably a sign you don't intend to replace it.
  • I prefer the AU14 route, mainly because I've invested time and energy in it - about 6 months of coding. The tangible advantage is license flexibility. The insight from doing it is totally transferable to the code salvage route.
  • The AU14 route is actually very very good for low bugs. I am not aware of any. The main problem is that it loses features. AU14 can't yet load/save waveforms for example. I want to do that by Unitary Project File, rather than existing XML format. Existing Autosave, recently improved, is a bad format. Speed drops with size of project, and it should not. The loss of features though does mean that AU14 TrackPanel, were we to introduce it now, would need to be used as an auxiliary panel, e.g. for looping.
    • PRL: And the more ideas I add to the old track panel, the more divergence there is and the more is "lost." Besides that, users might become disloyal if the familiar old interface that they prefer goes away. So I think the old appearance and interactions are here to stay. The old track panel code is what I can see and figure out how to enhance. I sensed that you wanted to discourage my projects to enhance old track panel because of your ambitions for the new one. I would rather do what I can to take the conflict out of our different ambitions. That means finding some path to reuse of the present good things as components.
    • JKC: And I am fine with you finding such a path. Splitting Track Panel up into Tracks will progress us.
  • If I'm RM, will I block the code salvage route?
    • I will block a big-bang in-situ change to the track panel. We had that when multi-clip was introduced. It destabilised Audacity and led to the very long release cycle for 2.0.
      • PRL: Indeed, though multi-clip changed the persistent, core data, implying more burdens for maintaining data structure consistency, in ways that this will not. I myself fixed a few bugs that were still there in multi-clip, which allowed recording to make overlapping clips.
    • I want internal changes to be motivated by tangible user features. The most natural would be to get the view-data separation in. Then we could have two views at different scales on the same waveform, and use one as a navigation track. I would like that as mod-nav-track so that it is an optional add-in module, even though changes in core are required to make it happen.
      • PRL: quite right. Near term I need to persuade the team to take the other big bang for fisheye, which is supporting a major user facing feature, and one big bang per release may be all that QA wants to cope with.
      • PRL: view-data separation is of course another desirable thing, and another project on its own. As I said I didn't propose to fix every obstacle to making plug-ins by evolution. If you prioritize that higher, I could get ideas about how to try that too. There is no harm in my trying any of them in a fork.
    • Salvage looks like more work than I took to get to AU14. If I'm not doing that work but you (Paul) are, why would I be against it?
    • I was told years ago not to touch the TrackPanel, i.e. not make big structural changes, because others had other plans. I don't want to do the same to you Paul. I think that's unfair restriction. Instead do make changes, but each one needs to be motivated by a feature. I won't accept UIHandle on its own. I will accept a change that gives us mod-nav-track and that takes us a step towards UIHandle.
      • PRL: Well that makes it harder to justify. None of the good things like spectral selection or fisheye are impossible to do or much worse to maintain with the old scattered organization of code. What if I did enough work that, say, time track support could become a separately compiled and dynamically linked add-on? That would demonstrate successful evolution to a more pluggable program. Label tracks and waveform view types might follow. Still that's not really an advance in any capabilities for the user.
      • JKC: More valuable than forcing separate compilation is removing knowledge of the different Track Types from TrackPanel. HandleEnvelope and HandleSlide have no right to be in TrackPanel. They should be in Track or WaveTrack or EnvelopeTrack. That better indicates a more pluggable program. After that step, I'd sooner see LabelTrack as a separately compiled and dynamically linked add on than TimeTrack. It's much easier to think of worthwhile extensions to LabelTrack that an alternative plug-in could give.