Thread: [Audacity-devel] Refactoring of TrackPanel now in CVS
A free multi-track audio editor and recorder
Brought to you by:
aosiniao
From: James C. <cr...@in...> - 2003-04-25 18:14:03
|
I've started incrementally refactoring TrackPanel. Changes are in CVS. No change whatsoever in functionality (I hope!). So far have extracted a LabelPanel (which has the widgets to the left of the track) out of TrackPanel and have extracted an 'AdornedRulerPanel' (which has the select mark and play-triangle on it) out of track panel. Files Affected ========== TrackPanel.cpp/.h Ruler.cpp/.h |
From: Joshua H. <jo...@re...> - 2003-04-25 19:20:08
|
On Fri, 2003-04-25 at 11:15, James Crook wrote: > I've started incrementally refactoring TrackPanel. > Changes are in CVS. No change whatsoever > in functionality (I hope!). Have you talked this over with Dominic? This kind of refactoring of TrackPanel and the Track hierarchy is definitely needed, but Dominic, Matt, and I have discussed this some and in our minds there is a long-term vision of what this will look like. You are definitely welcome to participate in architectural changes like this (especially since you are the one actually doing work right now), but you should be aware of the ideas already brewing. Josh |
From: James C. <cr...@in...> - 2003-04-25 20:45:43
|
Josh, I had a look at the comments about the plans that were in TrackPanel.cpp and Todo docs - a move towards using wxSizers and towards getting wxWindows to do the dispatching - in the long term. I don't think we'll be able to get there in one step. I've taken a small step, so far. If you want to check how in-synch I am with the general plans, have a look at the additional comments in the start of TrackPanel.cpp. What I'm trying to do is break up the monolith into separate visual objects. It should be easier to see what we're dealing with as this process progresses, collaboratively. I'm not intending anything too radical without consultation first! I've also deliberately not created new files for the new classes as the names of the classes (and hence of the files) are in my experience one of the last things to be finalised. Have a look and tell me what you think should happen next. --James. ----- Original Message ----- From: Joshua Haberman <jo...@re...> To: <aud...@li...> Sent: Friday, April 25, 2003 8:18 PM Subject: Re: [Audacity-devel] Refactoring of TrackPanel now in CVS > On Fri, 2003-04-25 at 11:15, James Crook wrote: > > I've started incrementally refactoring TrackPanel. > > Changes are in CVS. No change whatsoever > > in functionality (I hope!). > Have you talked this over with Dominic? This kind of refactoring of > TrackPanel and the Track hierarchy is definitely needed, but Dominic, > Matt, and I have discussed this some and in our minds there is a > long-term vision of what this will look like. You are definitely > welcome to participate in architectural changes like this (especially > since you are the one actually doing work right now), but you should be > aware of the ideas already brewing. > Josh |
From: Joshua H. <jo...@re...> - 2003-04-25 21:49:57
|
On Fri, 2003-04-25 at 13:46, James Crook wrote: > Josh, > > I had a look at the comments about the plans that were > in TrackPanel.cpp and Todo docs - a move towards > using wxSizers and towards getting wxWindows to do > the dispatching - in the long term. > > I don't think we'll be able to get there in one step. > I've taken a small step, so far. > > If you want to check how in-synch I am with > the general plans, have a look at the additional comments > in the start of TrackPanel.cpp. What I'm trying to do > is break up the monolith into separate visual objects. > It should be easier to see what we're dealing with as > this process progresses, collaboratively. OK, a couple of things. First of all, I think you are thinking of the term "panel" in a little bit different way than we (or at least I) do. Whereas you are separating the visual elements and calling each one a "panel," I think of the TrackPanel as the entire area on the screen that has tracks in it. In my mind there aren't sub-panels of any kind, those are just tracks and parts of tracks. This may seem just a matter of terminology, but it also leads me to my next point. I think what you are doing is not in the same direction that we have planned. You are modularizing the drawing code in the context of TrackPanel. What we are looking to do is remove most of the drawing code from TrackPanel entirely, and move it to the Track hierarchy. TrackPanel would then be a small (comparatively) class that just asks tracks to draw themselves, dispatches events to individual tracks, handles scrolling, and draws miscellaneous things. More about the Track hierarchy: our plan is to rewrite the hierarchy to look like this: Track / \ / \ / \ GUITrack WaveTrack \ / \ / \ / GUIWaveTrack The same will happen with NoteTrack and LabelTrack, but you get the idea. Track contains the common interface for all tracks (cut/copy/paste, offset). WaveTrack contains all the code for representing waveforms, but contains no GUI code (and no wx library calls except for wxBase). GUITracks draws the things common to every track and receives events that every track handles the same. GUIWaveTrack draws the things specific to WaveTracks, and does whatever else doesn't belong in the other classes. TrackArtist will disappear entirely. The even bigger idea here is to separate GUI code from backend code. This involves taking each system and factoring out the non-GUI operations into a library that doesn't make wx calls (except wxBase). Then we can build this backend as a library (tentatively called 'libaudacity') and build it separately from Audacity. This will facilitate ports if anyone is interested, unit testing, and just good software design. I may have gotten small things wrong or left holes, but hopefully this gives you an idea for where we're headed. I see this kind of thing as post-1.2, just because these are fairly sweeping changes. I don't know about everyone else, but summer is always the time where I do the most open-source hacking, so this might be the time for these changes. If you're dying to work on this now, you'd probably want to branch off the trunk onto an experimental branch. Josh |
From: James C. <cr...@in...> - 2003-04-26 13:50:51
|
----- Original Message ----- From: Joshua Haberman <jo...@re...> Subject: Re: [Audacity-devel] Refactoring of TrackPanel now in CVS Josh / Other Developers, In Brief: ====== > If you're dying to work on this now, you'd probably > want to branch off the trunk onto an experimental branch. No. My experience of forking is that it costs everyone very dearly in time, so starting a new fork is not an option for me. So - for now I will hold off on making further structural changes to TrackPanel, hoping that we will get general agreement that we are actually discussing compatible refactoring changes, not conflicting ones. I'm also looking for agreement that we are best to approach this incrementally, over a period of time, in small steps, rather than go for a big-bang all in one change. This is the 'extreme programming' way of doing things, but may not be the norm here. --------- Longer Response: ============ I totally agree with a 'libaudacity' approach as the correct direction to go - for automated testing, scriptability and the other reasons Josh cites. (see for example my comment on APIs in the RFEs). Yes. Totally agree. As well as libaudacity though I also see Dominic already placing generic-GUI candidates for migration to wxWindows in the widgets directory. Hidden within TrackPanel in Audacity is code that is aching to become an enhanced wxBoxSizer class that lets the components be resized by click and drag. In time this code needs to be extracted too. It's general GUI functionality, of use beyond Audacity. My picture is that the main forces separating existing classes is the pull of these two libraries. These are not conflicting goals. libaudacity code should be sound without gui. candidate wxWindows code should be GUI without sound. Taking a small step, as I have done, towards extracting some more generic GUI code helps rather than hinders the overall process of separating the monolithic classes. Josh - if you're saying my change is incompatible with the changes you plan, then I disagree. If you actually refactor down to GuiWaveTracks, each with one wave, you then run into a problem of who draws the 'LabelPanel' when dealing with stereo tracks. Should the right channel or the left channel draw it? Would you have a special StereoGuiWaveTrack to fix this? Since a LabelPanel serves multiple waves, I think you'd just find yourself factoring out the LabelPanel later on in the refactoring anyway. The middle ground, the part for which you have drawn the diagram, is actually the hardest part to get right. It's much easier to chip away from the ends, moving code into generic GUI classes or specific Audio classes, and then see what's left. With fewer distractions around it's easier to see what to do. I strongly expect when we reach the middle we will find that a one-to-one relation between data (WaveTrack) and view (GuiWaveTrack) is too restrictive - for example if we want to extend Audacity so that we can view a spectrum of a wave and the wave-form view of a wave at the same time. So I expect we will be looking to go for the data-view separation by reference rather than by the inheritance shown in the diagram to achieve that. I think overall we're arguing about a lot less than it appears on the surface. Class names are important and I'm not attached to the names I've used. Try instead: GuiWaveTrack where I have TrackPanel. TrackPanel where I have ProjectPanel. If this makes you happier I will make the change in TrackPanel.cpp. Whatever we disagree on, we can agree that a major goal in the refactoring is to separate GUI and audio code. --James. |
From: Joshua H. <jo...@re...> - 2003-04-27 00:38:32
|
On Sat, 2003-04-26 at 06:50, James Crook wrote: > I'm also looking for agreement that we are best to > approach this incrementally, over a period of > time, in small steps, rather than go for a big-bang > all in one change. This is the 'extreme programming' > way of doing things, but may not be the norm here. I know conventional wisdom is to work incrementally, and I'm sure there's a lot to be said for that. In the past, we've done more of a big bang approach though. Someone (usually Dominic) works on a sweeping change for a while, then checks it all in at once. This works surprisingly well. Examples: - Support for bit depths other than 16-bit - Splitting WaveTrack into Sequence/WaveTrack - Importer rewrite - BlockFile rewrite If more than one person is working on a major project like this, there is usually a branch in CVS they all work from, then checking in the result on the trunk all at once. > I totally agree with a 'libaudacity' approach as the > correct direction to go - for automated testing, scriptability > and the other reasons Josh cites. (see for example > my comment on APIs in the RFEs). Yes. Totally agree. > > As well as libaudacity though I also see Dominic already > placing generic-GUI candidates for migration to > wxWindows in the widgets directory. I'm not aware that this is part of a general strategy to factor out 'candidate wx code,' they're just custom widgets. But I see your bigger point. > Hidden within > TrackPanel in Audacity is code that is aching to become > an enhanced wxBoxSizer class that lets the components > be resized by click and drag. In time this code needs to > be extracted too. It's general GUI functionality, of use > beyond Audacity. > > My picture is that the main forces separating existing > classes is the pull of these two libraries. These are not > conflicting goals. > > libaudacity code should be sound without gui. > candidate wxWindows code should be GUI > without sound. > > Taking a small step, as I have done, towards extracting some > more generic GUI code helps rather than hinders the overall > process of separating the monolithic classes. If what you are doing is along the lines of generic GUI code that you see operating at the wx level (like the custom widgets Dominic wrote), then it is probably orthogonal to the future rewrite of the Track hierarchy, don't you think? > Josh - if you're saying my change is incompatible with the > changes you plan, then I disagree. If you actually refactor down > to GuiWaveTracks, each with one wave, you then run into a > problem of who draws the 'LabelPanel' when dealing with stereo > tracks. Should the right channel or the left channel draw it? > Would you have a special StereoGuiWaveTrack to fix this? > Since a LabelPanel serves multiple waves, I think you'd just find > yourself factoring out the LabelPanel later on in the refactoring > anyway. That's already part of the plan -- that's why we have the class GUITrack. GUITrack is designed to contain drawing code common to all tracks, such as drawing the label. > The middle ground, the part for which you have drawn the diagram, > is actually the hardest part to get right. It's much easier to > chip away from the ends, moving code into generic GUI classes > or specific Audio classes, and then see what's left. With fewer > distractions around it's easier to see what to do. > > I strongly expect when we reach the middle we will find > that a one-to-one relation between data (WaveTrack) and > view (GuiWaveTrack) is too restrictive - for example if we want > to extend Audacity so that we can view a spectrum of a wave > and the wave-form view of a wave at the same time. So I > expect we will be looking to go for the data-view separation > by reference rather than by the inheritance shown in the > diagram to achieve that. The one-to-one relationship between GUITracks and Tracks is deliberate. We discussed using a reference scheme but in the end favored using inheritance. The strength of this approach is that "a track is a track is a track." A list of tracks is the basic data structure on which everything operates. The TrackPanel draws a list of tracks. AudioIO plays a list of tracks. Effects operate on a list of tracks. Importers create a list of tracks and exporters export a list of tracks. If GUITracks and Tracks were not equivalent, it would mean constantly converting between the idea of GUITracks and Tracks. An Importer can only create Tracks, not GUITracks (since it's part of libaudacity) so the GUI code would have to create GUITracks separately and assign them to the correct WaveTracks on import. Likewise with the recording code or any backend code that constructs tracks. Inheritance buys a great deal of consistency and simplicity, and I can't think of a case where it will be too restrictive for our needs. For your scenario of wanting to have two views of a wave at a time, this is easily accomplished by building this functionality into GUIWaveTrack. Dominic's the one really calling the shots, these are just my opinions. Josh |
From: James C. <cr...@in...> - 2003-04-27 14:42:26
|
----- Original Message ----- From: Joshua Haberman <jo...@re...> > I'm not aware that this is part of a general strategy to factor out > 'candidate wx code,' they're just custom widgets. But I see your bigger > point. > If what you are doing is along the lines of generic GUI code that you > see operating at the wx level (like the custom widgets Dominic wrote), > then it is probably orthogonal to the future rewrite of the Track > hierarchy, don't you think? Yes I am interested in generic GUI code, specifically a mouse-event resizable wxBoxSizer. I'd also like us to leverage existing wxWindows event dispatch code, so that we aren't hit testing mute,solo,title, close,gain,pan ourselves but are having wxWindows do it for us. These seem logical changes that won't make other changes later on any harder. Whether this is 'orthogonal' to the planned re-write of Tracks depends on your meaning of 'orthogonal'. One motivation of your plans is to isolate the 'sound' code. I'm interested in isolating purely 'Gui' code. Is your glass half empty or half full? If you take sound code out of the Gui code is that 'orthogonal' to taking Gui code out of the sound code? I see both as working different ends of the same problem, which in TrackPanel has become quite tangled. Doing either progresses the code separation. > The one-to-one relationship between GUITracks and Tracks is deliberate. > We discussed using a reference scheme but in the end favored using > inheritance. The strength of this approach is that "a track is a track > is a track." A list of tracks is the basic data structure on which > everything operates. The TrackPanel draws a list of tracks. AudioIO > plays a list of tracks. Effects operate on a list of tracks. Importers > create a list of tracks and exporters export a list of tracks. If > GUITracks and Tracks were not equivalent, it would mean constantly > converting between the idea of GUITracks and Tracks. An Importer can > only create Tracks, not GUITracks (since it's part of libaudacity) so > the GUI code would have to create GUITracks separately and assign them > to the correct WaveTracks on import. Likewise with the recording code > or any backend code that constructs tracks. > Inheritance buys a great deal of consistency and simplicity, and I can't > think of a case where it will be too restrictive for our needs. For > your scenario of wanting to have two views of a wave at a time, this is > easily accomplished by building this functionality into GUIWaveTrack. Hmm. I'd agree there is no urgent need for one-to-many, and so that it is better to keep it simple for now. But I'd also doubt that changing to one-to-many later on would be difficult - if it does turn out to be needed. > Dominic's the one really calling the shots, these are just my opinions. Whatever Dominic thinks, it's not good if developers have strong disagreements. So I'd like to put on record that I'd be really happy to see the changes you describe. I think they would be very good changes for Audacity as it is now and going forward. For now the right thing seems to be for me to hold off making further architectural change in TrackPanel. Maybe we can work collaboratively on this once we are on feature-freeze time. --James. |
From: Dominic M. <do...@mi...> - 2003-04-27 19:47:38
|
Yay! SMTP is working again; I can respond to messages in real time. :) James Crook wrote: > Yes I am interested in generic GUI code, specifically a mouse-event > resizable wxBoxSizer. I'd also like us to leverage existing wxWindows > event dispatch code, so that we aren't hit testing mute,solo,title, > close,gain,pan ourselves but are having wxWindows do it for us. > These seem logical changes that won't make other changes later > on any harder. Sounds good. I only have two concerns: * wxWindows is far from optimal in its hit-testing routines, - it often does a linear scan of a list of controls. We should investigate its speed at handling hundreds of controls inside the project window before committing to this. * It would make it slightly more difficult to handle actions that go between tracks, e.g. selecting two tracks by clicking in one and releasing in the other. Perhaps selecting should be handled by the parent class, rather than the Track class, because it needs to know about all of the tracks, not just the one that initially receives the click. > Whether this is 'orthogonal' to the planned re-write of Tracks depends > on your meaning of 'orthogonal'. Is their dot-product equal to zero? :) >>Dominic's the one really calling the shots, these are just my opinions. > > Whatever Dominic thinks, it's not good if developers have strong > disagreements. So I'd like to put on record that I'd be really happy > to see the changes you describe. I think they would be very good > changes for Audacity as it is now and going forward. The last thing I want is for an issue to be settled because "I said so". I don't want to make decisions by fiat. If you disagree with my opinion, or Josh's, or anyone else's, please keep discussing it until we can come to consensus. There have been some big disagreements in the past, but we've always been able to come to consensus. The most recent example I can think of is the switch to PortAudio v19: Josh really wanted to switch now, and I wanted to stick with v18. We ended up compromising: Josh added v18 support for his new Audio I/O code, and I added OSS support to PortAudio v19. Now everyone's happy: we have stable audio I/O now, and we're 90% of the way towards supporting v19. > For now the right thing seems to be for me to hold off making further > architectural change in TrackPanel. Maybe we can work collaboratively > on this once we are on feature-freeze time. Yes - though see my previous email (the one that should have arrived 24 hours ago) - I'm not unhappy with your changes and I'd like to at least take advantage of them to make TrackPanel.cpp less unwieldy. Though I don't want to do much more refactoring until after the freeze, I'd be happy to continue discussing it. Let's go into more detail as to exactly how the new track hierarchy might work. In particular, I like your idea of making use of a generic wxWindows widget for a user-resizable "sizer", and I'd like to discuss how that might fit in. Should GUITrack derive from that class? Other open questions: What is a TrackList now? Should there be a GUITrackList? If we end up using classes like a GUITrackGroup, does that mean that there will have to be a TrackGroup that non-GUI classes (e.g. effects) see? Is there a way that effects could see a flattened view of the tracks, but the TrackPanel could see a hierarchical view? This assumes that you accept our basic idea of the Track/GUITrack/ WaveTrack/GUIWaveTrack diamond. If not, please continue to discuss until either you're happy or you've convinced us otherwise. - Dominic > --James. > > > > > > ------------------------------------------------------- > This sf.net email is sponsored by:ThinkGeek > Welcome to geek heaven. > http://thinkgeek.com/sf > _______________________________________________ > Audacity-devel mailing list > Aud...@li... > https://lists.sourceforge.net/lists/listinfo/audacity-devel |
From: Dominic M. <do...@mi...> - 2003-04-29 01:13:05
|
Hi James, Josh is right in that this isn't really how we intended to rewrite the GUI code. However, the existing code was quite poorly organized, and one could argue that ANY change would be a step up from that, even if it doesn't necessarily move us closer to the reorganization we intend. In other words, even if it doesn't move us closer, it doesn't move us farther, either. One possible immediate benefit to your change is that we could speed up compilation. TrackPanel.cpp is now 4,800 lines of code, and it takes ridiculously long to compile - enough that it sometimes discourages me from fiddling with it. So with your changes we could split the LabelPanel into a different file and speed things up in general. Assuming that your code is stable, I don't see any reason to rollback the changes. It looks clean to me, and it ought to make things a little easier to work with for the short term. My main concern is that we don't make any changes that could introduce new bugs before version 1.2.0. Every time we refactor or redesign, inevitably new bugs are introduced. All this does is delay 1.2.0 even more. I do want to fork, but the opposite way as was suggested. As soon as we're feature-complete for 1.2.0, I want to create a CVS branch for 1.2.0 and its stable followers. Then we can start going crazy with the CVS head. Then, for the most part only bug fixes need to be ported between the two branches. If it's okay with you, I'd like to rename "LabelPanel" into "TrackLabels" and stick it in its own source file. I'm also going to take all of the code in TrackPanel::DisplaySelection() and put it in its own source file. More comments below: James Crook wrote: > ----- Original Message ----- > From: Joshua Haberman <jo...@re...> > Subject: Re: [Audacity-devel] Refactoring of TrackPanel now in CVS > > > Josh / Other Developers, > > In Brief: > ====== > > >>If you're dying to work on this now, you'd probably >>want to branch off the trunk onto an experimental branch. > > > No. My experience of forking is that it costs everyone > very dearly in time, so starting a new fork is not an > option for me. > > So - for now I will hold off on making further structural > changes to TrackPanel, hoping that we will get > general agreement that we are actually discussing > compatible refactoring changes, not conflicting ones. I agree that forks are generally bad, but the exception is that I like the idea of creating a branch whenever we do a stable release. That allows us to release new stable versions quickly when we find bugs, without worrying about introducing new ones because of new features. So as soon as we're ready for 1.2.0, I don't have any concerns about starting to do some serious refactoring. > I'm also looking for agreement that we are best to > approach this incrementally, over a period of > time, in small steps, rather than go for a big-bang > all in one change. This is the 'extreme programming' > way of doing things, but may not be the norm here. I agree, except that every now and then I think that a big band is necessary, or at least a medium-sized bang. > --------- > > Longer Response: > ============ > > I totally agree with a 'libaudacity' approach as the > correct direction to go - for automated testing, scriptability > and the other reasons Josh cites. (see for example > my comment on APIs in the RFEs). Yes. Totally agree. > > As well as libaudacity though I also see Dominic already > placing generic-GUI candidates for migration to > wxWindows in the widgets directory. Hidden within > TrackPanel in Audacity is code that is aching to become > an enhanced wxBoxSizer class that lets the components > be resized by click and drag. In time this code needs to > be extracted too. It's general GUI functionality, of use > beyond Audacity. Good idea. > My picture is that the main forces separating existing > classes is the pull of these two libraries. These are not > conflicting goals. > > libaudacity code should be sound without gui. > candidate wxWindows code should be GUI > without sound. > > Taking a small step, as I have done, towards extracting some > more generic GUI code helps rather than hinders the overall > process of separating the monolithic classes. > > Josh - if you're saying my change is incompatible with the > changes you plan, then I disagree. If you actually refactor down > to GuiWaveTracks, each with one wave, you then run into a > problem of who draws the 'LabelPanel' when dealing with stereo > tracks. Should the right channel or the left channel draw it? > Would you have a special StereoGuiWaveTrack to fix this? > Since a LabelPanel serves multiple waves, I think you'd just find > yourself factoring out the LabelPanel later on in the refactoring > anyway. One thing we discussed was to have TrackGroup and GUITrackGroup classes. A GUITrackGroup would contain two or more GUITracks, each with the ability to draw itself. The GUITrack would draw the labels, but call the method of each of its GUITracks to draw the waveforms/whatever (note that they wouldn't have to be wavetracks). > The middle ground, the part for which you have drawn the diagram, > is actually the hardest part to get right. It's much easier to > chip away from the ends, moving code into generic GUI classes > or specific Audio classes, and then see what's left. With fewer > distractions around it's easier to see what to do. > > I strongly expect when we reach the middle we will find > that a one-to-one relation between data (WaveTrack) and > view (GuiWaveTrack) is too restrictive - for example if we want > to extend Audacity so that we can view a spectrum of a wave > and the wave-form view of a wave at the same time. So I > expect we will be looking to go for the data-view separation > by reference rather than by the inheritance shown in the > diagram to achieve that. I seriously thought about this. This would be more compatible with the Model/View/Controller style of design. In other words, we'd have parallel hierarchies: Track GUITrack / \ / \ / \ / \ WaveTrack LabelTrack GUIWaveTrack GUILabelTrack The only potential complication then comes from adding, deleting, and rearranging tracks. For example, an Effect should be able to create a new track, but it shouldn't need to know about GUITracks. Loading and saving becomes more complicated, too, than it would with the inheritance structure. > I think overall we're arguing about a lot less than it appears > on the surface. Class names are important and I'm not attached to the > names I've used. Try instead: > > GuiWaveTrack where I have TrackPanel. > TrackPanel where I have ProjectPanel. > > If this makes you happier I will make the change in TrackPanel.cpp. > Whatever we disagree on, we can agree that a major goal > in the refactoring is to separate GUI and audio code. Agreed. Go ahead and change the names. Are you okay with TrackLabels instead of LabelPanel for now? That class will eventually disappear or eventually become a helper class for a GUITrack, but for now it at least splits TrackPanel into different logical pieces. But I hope you're okay with waiting on any more refactoring until we're ready to do the branch... - Dominic > > --James. > > > > > ------------------------------------------------------- > This sf.net email is sponsored by:ThinkGeek > Welcome to geek heaven. > http://thinkgeek.com/sf > _______________________________________________ > Audacity-devel mailing list > Aud...@li... > https://lists.sourceforge.net/lists/listinfo/audacity-devel |