Hello Ted,

Either there is an issue here or the function is poorly named.  But if I add tracks, I do not expect the NotifyTracksDeleted to be called (it doesn't sound correct).

Modified: trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp
===================================================================
--- trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -124,6 +124,12 @@
         if (track) track->setPosition(i->second);
     }

+    // Notify composition observers of the delete
+    std::vector<TrackId> trackIds;
+    for (size_t i = 0; i < m_newTracks.size(); ++i)
+        trackIds.push_back(m_newTracks[i]->getId());
+    m_composition->notifyTracksDeleted(trackIds);
+
     m_detached = true;
}

Sincerely,
Julie S.

--- On Tue, 12/20/11, tedfelix@users.sourceforge.net <tedfelix@users.sourceforge.net> wrote:

From: tedfelix@users.sourceforge.net <tedfelix@users.sourceforge.net>
Subject: [Rosegarden-bugs] SF.net SVN: rosegarden:[12730] trunk/rosegarden/src
To: rosegarden-bugs@lists.sourceforge.net
Date: Tuesday, December 20, 2011, 8:12 PM

Revision: 12730
          http://rosegarden.svn.sourceforge.net/rosegarden/?rev=12730&view=rev
Author:   tedfelix
Date:     2011-12-21 01:11:59 +0000 (Wed, 21 Dec 2011)
Log Message:
-----------
This is the first commit for the Composition Notification Project.

The key change is the renaming of Composition::notifyTrackDeleted() to
notifyTracksDeleted() and making it public.  The idea is that those who are
modifying the Composition (DeleteTracksCommand::execute() and
AddTracksCommand::unexecute()) know best when the modification is complete
and the Composition is once again in a consistent state.  Therefore, they
should be responsible for notifying Composition's observers of the completion
of their modifications.  The rest of the code changes are in support of this
new approach.

The final result is that TrackButtons can now add itself as a
CompositionObserver and find out when tracks are deleted.  It can then update
itself to stay in sync.  This has been tested by commenting out the call to
m_trackButtons->slotUpdateTracks() in TrackEditor::paintEvent().  It appears
to work fine.

Testing should include deleting tracks, undoing the addition of tracks, and
continued use of the software noting any problems. 

Feedback on this will help shape future changes.  I am particularly concerned
about the notification responsibilities being shifted outside of Composition.
I think it's an ok move for a large system like rg.  Others might not. 
Dissenting opinions and discussion appreciated.

Detailed Change Breakdown

* Composition
  - Changed notifyTrackDeleted() to notifyTracksDeleted() and made it
    public.  This is the key change. 
  - Removed call to notifyTrackDeleted() from detachTrack().  The
    responsibility has been shifted outside of Composition to
    AddTracksCommand and DeleteTracksCommand.
  - CompositionObserver::trackDeleted() changed to tracksDeleted().
  - These delete notification routines now take a list of tracks that
    have been deleted.  This should reduce unnecessary updates when
    multiple tracks are being deleted.
* AddTracksCommand
  - unexecute() now calls Composition::notifyTracksDeleted() once the
    added tracks have been deleted.
* DeleteTracksCommand
  - execute() now calls Composition::notifyTracksDeleted() once the
    tracks have been deleted.
* SequenceManager
  - trackDeleted() has been changed to tracksDeleted() and the code
    has been updated to handle the list of deleted tracks.
* TrackParameterBox
  - setDocument() now subscribes to Composition as an observer.  This
    was a bug.
  - trackDeleted() has been changed to tracksDeleted() and the code
    has been updated to handle a list of deleted tracks.
* TrackButtons
  - TrackButtons now derives from CompositionObserver and adds itself
    as an observer in its ctor.
  - tracksDeleted() has been added and it calls slotUpdateTracks() to
    make sure the track buttons are in sync with the Composition.
* Miscellaneous changes
  - Commented out the unused Composition::deleteTrack().
  - Removed a noisy RG_DEBUG from CompositionModelImpl.
  - Some code reformatting.

Modified Paths:
--------------
    trunk/rosegarden/src/base/Composition.cpp
    trunk/rosegarden/src/base/Composition.h
    trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp
    trunk/rosegarden/src/commands/segment/DeleteTracksCommand.cpp
    trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.cpp
    trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.h
    trunk/rosegarden/src/gui/editors/segment/TrackButtons.cpp
    trunk/rosegarden/src/gui/editors/segment/TrackButtons.h
    trunk/rosegarden/src/gui/editors/segment/compositionview/CompositionModelImpl.cpp
    trunk/rosegarden/src/gui/seqmanager/SequenceManager.cpp
    trunk/rosegarden/src/gui/seqmanager/SequenceManager.h

Modified: trunk/rosegarden/src/base/Composition.cpp
===================================================================
--- trunk/rosegarden/src/base/Composition.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/base/Composition.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -1686,7 +1686,8 @@
     }
}

-
+#if 0
+// unused
void Composition::deleteTrack(Rosegarden::TrackId track)
{
     trackiterator titerator = m_tracks.find(track);
@@ -1706,6 +1707,7 @@
     }
     
}
+#endif

bool Composition::detachTrack(Rosegarden::Track *track)
{
@@ -1728,7 +1730,6 @@
     m_tracks.erase(it);
     updateRefreshStatuses();
     checkSelectedAndRecordTracks();
-    notifyTrackDeleted(track->getId());

     return true;
}
@@ -2156,11 +2157,13 @@
}

void
-Composition::notifyTrackDeleted(TrackId t) const
+Composition::notifyTracksDeleted(std::vector<TrackId> trackIds) const
{
+    //RG_DEBUG << "Composition::notifyTracksDeleted() notifying" << m_observers.size() << "observers";
+
     for (ObserverSet::const_iterator i = m_observers.begin();
-     i != m_observers.end(); ++i) {
-    (*i)->trackDeleted(this, t);
+         i != m_observers.end(); ++i) {
+        (*i)->tracksDeleted(this, trackIds);
     }
}


Modified: trunk/rosegarden/src/base/Composition.h
===================================================================
--- trunk/rosegarden/src/base/Composition.h    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/base/Composition.h    2011-12-21 01:11:59 UTC (rev 12730)
@@ -167,7 +167,8 @@
     /**
      * Delete a Track by index
      */
-    void deleteTrack(TrackId track);
+    // unused
+//    void deleteTrack(TrackId track);

     /**
      * Detach a Track (revert ownership of the Track object to the
@@ -823,6 +824,8 @@
     void    addObserver(CompositionObserver *obs) { m_observers.push_back(obs); }
     void removeObserver(CompositionObserver *obs) { m_observers.remove(obs); }

+    void notifyTracksDeleted(std::vector<TrackId> trackIds) const;
+
     //////
     // DEBUG FACILITIES
     void dump(std::ostream&, bool full=false) const;
@@ -963,7 +966,6 @@
     void notifySegmentEndMarkerChange(Segment *s, bool shorten);
     void notifyEndMarkerChange(bool shorten) const;
     void notifyTrackChanged(Track*) const;
-    void notifyTrackDeleted(TrackId) const;
     void notifyMetronomeChanged() const;
     void notifyTimeSignatureChanged() const;
     void notifySoloChanged() const;
@@ -1108,9 +1110,9 @@
     virtual void trackChanged(const Composition *, Track*) { }

     /**
-     * Called when a track has been deleted
+     * Called when tracks have been deleted
      */
-    virtual void trackDeleted(const Composition *, TrackId) { }
+    virtual void tracksDeleted(const Composition *, std::vector<TrackId> &/*trackIds*/) { }

     /**
      * Called when some time signature has changed

Modified: trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp
===================================================================
--- trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/commands/segment/AddTracksCommand.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -124,6 +124,12 @@
         if (track) track->setPosition(i->second);
     }

+    // Notify composition observers of the delete
+    std::vector<TrackId> trackIds;
+    for (size_t i = 0; i < m_newTracks.size(); ++i)
+        trackIds.push_back(m_newTracks[i]->getId());
+    m_composition->notifyTracksDeleted(trackIds);
+
     m_detached = true;
}


Modified: trunk/rosegarden/src/commands/segment/DeleteTracksCommand.cpp
===================================================================
--- trunk/rosegarden/src/commands/segment/DeleteTracksCommand.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/commands/segment/DeleteTracksCommand.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -113,6 +113,8 @@
         }
     }

+    m_composition->notifyTracksDeleted(m_tracks);
+
     m_detached = true;
}


Modified: trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.cpp
===================================================================
--- trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -90,11 +90,11 @@
         : RosegardenParameterBox(tr("Track"),
                                  tr("Track Parameters"),
                                  parent),
-                                 m_doc(doc),
-                                 m_highestPlayable(127),
-                                 m_lowestPlayable(0),
-                                 m_lastInstrumentType(-1),
-                                 m_selectedTrackId((int)NO_TRACK)
+          m_doc(doc),
+          m_highestPlayable(127),
+          m_lowestPlayable(0),
+          m_selectedTrackId((int)NO_TRACK),
+          m_lastInstrumentType(-1)
{
     setObjectName("Track Parameter Box");

@@ -479,6 +479,7 @@
     if (m_doc != doc) {
         RG_DEBUG << "TrackParameterBox::setDocument\n";
         m_doc = doc;
+        m_doc->getComposition().addObserver(this);
         slotPopulateDeviceLists();
     }
}
@@ -722,10 +723,18 @@
}

void
-TrackParameterBox::trackDeleted(const Composition *comp, TrackId id)
+TrackParameterBox::tracksDeleted(const Composition *, std::vector<TrackId> &trackIds)
{
-    std::cerr << "TrackParameterBox::trackDeleted(" << id << "), selected is " << m_selectedTrackId << std::endl;
-    if ((int)id == m_selectedTrackId) slotSelectedTrackChanged();
+    //RG_DEBUG << "TrackParameterBox::tracksDeleted(), selected is " << m_selectedTrackId;
+
+    // For each deleted track
+    for (unsigned i = 0; i < trackIds.size(); ++i) {
+        // If this is the selected track
+        if ((int)trackIds[i] == m_selectedTrackId) {
+            slotSelectedTrackChanged();
+            return;
+        }
+    }
}

void

Modified: trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.h
===================================================================
--- trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.h    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/editors/parameters/TrackParameterBox.h    2011-12-21 01:11:59 UTC (rev 12730)
@@ -66,7 +66,7 @@

     // CompositionObserver interface
     //
-    virtual void trackDeleted(const Composition *, TrackId);
+    virtual void tracksDeleted(const Composition *, std::vector<TrackId> &trackIds);

public slots:
     void slotSelectedTrackChanged();

Modified: trunk/rosegarden/src/gui/editors/segment/TrackButtons.cpp
===================================================================
--- trunk/rosegarden/src/gui/editors/segment/TrackButtons.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/editors/segment/TrackButtons.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -121,10 +121,15 @@
     //
     setMinimumHeight(overallHeight);

+    m_doc->getComposition().addObserver(this);
}

-TrackButtons::~TrackButtons()
-{}
+TrackButtons::~TrackButtons() {
+    // CRASH!  Probably m_doc is gone...
+    // Probably don't need to disconnect as we only go away when the
+    // doc and composition do.  shared_ptr would help here.
+//    m_doc->getComposition().removeObserver(this);
+}

void
TrackButtons::makeButtons()
@@ -273,6 +278,12 @@
void
TrackButtons::slotUpdateTracks()
{
+#if 0
+    RG_DEBUG << "TrackButtons::slotUpdateTracks()";
+    static QTime t;
+    RG_DEBUG << "  elapsed: " << t.restart();
+#endif
+
     Composition &comp = m_doc->getComposition();
     unsigned int newNbTracks = comp.getNbTracks();
     Track *track = 0;
@@ -1157,6 +1168,21 @@

}

+#if 0
+// Definitely not ready for primetime.
+void
+TrackButtons::trackChanged(const Composition *, Track*) {
+    RG_DEBUG << "TrackButtons::trackChanged()";
+//    slotUpdateTracks();
+}
+#endif

+void
+TrackButtons::tracksDeleted(const Composition *, std::vector<TrackId> &/*trackIds*/) {
+    //RG_DEBUG << "TrackButtons::tracksDeleted()";
+
+    slotUpdateTracks();
}
+
+}
#include "TrackButtons.moc"

Modified: trunk/rosegarden/src/gui/editors/segment/TrackButtons.h
===================================================================
--- trunk/rosegarden/src/gui/editors/segment/TrackButtons.h    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/editors/segment/TrackButtons.h    2011-12-21 01:11:59 UTC (rev 12730)
@@ -18,6 +18,7 @@
#ifndef _RG_TRACKBUTTONS_H_
#define _RG_TRACKBUTTONS_H_

+#include "base/Composition.h"
#include "base/MidiProgram.h"
#include "base/Track.h"
#include "gui/application/RosegardenMainWindow.h"
@@ -56,7 +57,7 @@
  *
  * These widgets are created based on the RosegardenDocument.
  */
-class TrackButtons : public QFrame
+class TrackButtons : public QFrame, CompositionObserver
{
     Q_OBJECT
public:
@@ -234,6 +235,11 @@
      */
     QColor getRecordLedColour(Rosegarden::Instrument *ins);

+    // CompositionObserver overrides
+//    virtual void trackChanged(const Composition *, Track*);
+    virtual void tracksDeleted(const Composition *, std::vector<TrackId> &trackIds);
+
+
     //--------------- Data members ---------------------------------

     RosegardenDocument                 *m_doc;

Modified: trunk/rosegarden/src/gui/editors/segment/compositionview/CompositionModelImpl.cpp
===================================================================
--- trunk/rosegarden/src/gui/editors/segment/compositionview/CompositionModelImpl.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/editors/segment/compositionview/CompositionModelImpl.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -380,7 +380,7 @@
                                   (eventStart,
                                    eventEnd - eventStart)));

-        RG_DEBUG << "CompositionModelImpl::updatePreviewCacheForNotationSegment: x = " << x << ", width = " << width << " (time = " << eventStart << ", duration = " << eventEnd - eventStart << ")";
+        //RG_DEBUG << "CompositionModelImpl::updatePreviewCacheForNotationSegment: x = " << x << ", width = " << width << " (time = " << eventStart << ", duration = " << eventEnd - eventStart << ")";

         if (x <= segStartX) {
             ++x;

Modified: trunk/rosegarden/src/gui/seqmanager/SequenceManager.cpp
===================================================================
--- trunk/rosegarden/src/gui/seqmanager/SequenceManager.cpp    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/seqmanager/SequenceManager.cpp    2011-12-21 01:11:59 UTC (rev 12730)
@@ -1834,9 +1834,13 @@
     }
}

-void SequenceManager::trackDeleted(const Composition *, TrackId t)
+void SequenceManager::tracksDeleted(const Composition *, std::vector<TrackId> &trackIds)
{
-    ControlBlock::getInstance()->setTrackDeleted(t, true);
+    //SEQMAN_DEBUG << "SequenceManager::tracksDeleted()";
+
+    for (unsigned i = 0; i < trackIds.size(); ++i) {
+        ControlBlock::getInstance()->setTrackDeleted(trackIds[i], true);
+    }
}

void SequenceManager::metronomeChanged(InstrumentId id,

Modified: trunk/rosegarden/src/gui/seqmanager/SequenceManager.h
===================================================================
--- trunk/rosegarden/src/gui/seqmanager/SequenceManager.h    2011-12-19 07:25:42 UTC (rev 12729)
+++ trunk/rosegarden/src/gui/seqmanager/SequenceManager.h    2011-12-21 01:11:59 UTC (rev 12730)
@@ -179,7 +179,7 @@
     virtual void segmentEndMarkerChanged   (const Composition*, Segment *, bool);
     virtual void endMarkerTimeChanged      (const Composition*, bool shorten);
     virtual void trackChanged              (const Composition*, Track*);
-    virtual void trackDeleted              (const Composition*, TrackId);
+    virtual void tracksDeleted             (const Composition *, std::vector<TrackId> &/*trackIds*/);
     virtual void timeSignatureChanged      (const Composition*);
     virtual void metronomeChanged          (const Composition*);
     virtual void soloChanged               (const Composition*, bool solo, TrackId selectedTrack);

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.


------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create
new or port existing apps to sell to consumers worldwide. Explore the
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
Rosegarden-bugs mailing list
Rosegarden-bugs@lists.sourceforge.net - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-bugs