Merge branch 'bug1672'

src/gui/dialogs/CheckForParallelsDialog.cpp

@@ -30,6 +30,9 @@
 #include "base/BaseProperties.h"
 #include "gui/widgets/FileDialog.h"
 #include "misc/ConfigGroups.h"
+#include "gui/editors/notation/NotationView.h"
+#include "gui/editors/notation/NotationScene.h"
+#include "gui/editors/notation/NotationStaff.h"
 
 
 #include <QDialog>

src/gui/dialogs/CheckForParallelsDialog.h

@@ -29,9 +29,6 @@
 #include "base/Composition.h"
 #include "base/Studio.h"
 #include "document/RosegardenDocument.h"
-#include "gui/editors/notation/NotationStaff.h"
-#include "gui/editors/notation/NotationScene.h"
-#include "gui/editors/notation/NotationView.h"
 
 #include <QTextBrowser>
 #include <QDialog>
@@ -65,6 +62,11 @@ namespace Rosegarden
 {
 
 
+class NotationView;
+class NotationScene;
+class NotationStaff;
+
+
 class CheckForParallelsDialog : public QDialog
 {
     Q_OBJECT

src/gui/editors/notation/NotationScene.cpp

@@ -1099,29 +1099,30 @@ NotationScene::getInsertionTime(bool allowEndTime) const
 NotationScene::CursorCoordinates
 NotationScene::getCursorCoordinates(timeT t) const
 {
-    if (m_staffs.empty() || !m_hlayout) return CursorCoordinates();
+    if (m_staffs.empty() || !m_hlayout)
+        return CursorCoordinates();
 
-    NotationStaff *topStaff = nullptr;
-    NotationStaff *bottomStaff = nullptr;
+    const NotationStaff *topStaff = nullptr;
+    const NotationStaff *bottomStaff = nullptr;
+
+    // Find the topmost and bottom-most staves.
     for (uint i = 0; i < m_staffs.size(); ++i) {
-        if (!m_staffs[i]) continue;
-        if (!topStaff || m_staffs[i]->getY() < topStaff->getY()) {
-            topStaff = m_staffs[i];
-        }
-        if (!bottomStaff || m_staffs[i]->getY() > bottomStaff->getY()) {
-            bottomStaff = m_staffs[i];
-        }
-    }
+        const NotationStaff *staff = m_staffs[i];
+        if (!staff)
+            continue;
 
-    NotationStaff *currentStaff = nullptr;
-    if (m_currentStaff < (int)m_staffs.size()) {
-        currentStaff = m_staffs[m_currentStaff];
+        // If first or higher, save it.
+        if (!topStaff  ||  staff->getY() < topStaff->getY())
+            topStaff = staff;
+        // If first or lower, save it.
+        if (!bottomStaff  ||  staff->getY() > bottomStaff->getY())
+            bottomStaff = staff;
     }
 
-    timeT snapped = snapTimeToNoteBoundary(t, true);
+    const timeT snapped = snapTimeToNoteBoundary(t, true);
 
-    double x = m_hlayout->getXForTime(t);
-    double sx = m_hlayout->getXForTimeByEvent(snapped);
+    const double x = m_hlayout->getXForTime(t);
+    const double sx = m_hlayout->getXForTimeByEvent(snapped);
 
     StaffLayout::StaffLayoutCoords top =
         topStaff->getSceneCoordsForLayoutCoords
@@ -1134,6 +1135,10 @@ NotationScene::getCursorCoordinates(timeT t) const
     StaffLayout::StaffLayoutCoords singleTop = top;
     StaffLayout::StaffLayoutCoords singleBottom = bottom;
 
+    const NotationStaff *currentStaff = nullptr;
+    if (m_currentStaff < (int)m_staffs.size())
+        currentStaff = m_staffs[m_currentStaff];
+
     if (currentStaff) {
         singleTop =
             currentStaff->getSceneCoordsForLayoutCoords
@@ -2240,5 +2245,38 @@ NotationScene::dumpBarDataMap()
     m_hlayout->dumpBarDataMap();
 }
 
+#if 0
+void
+NotationScene::setCurrentStaff(const timeT t)
+{
+    // ??? This is an attempt to fix Bug #1672.  It is currently not being
+    //     used as it introduces new problems.  Need to come up with
+    //     a better solution if possible.
+    //
+    //     See NotationWidget::updatePointer() which has a commented out
+    //     call to this.
+
+    NotationStaff *currentStaff = getCurrentStaff();
+
+    // Get the pointer scene coords for t (regardless of staff).
+    const double pointerSX = m_hlayout->getXForTime(t);
+    // To avoid vertical jumps, use the current staff's Y.
+    const double pointerSY = currentStaff->getY();
+    // figure out which staff that is in
+    // ??? Does this do "nearest"?  Probably need to test some
+    //     wild staff arrangements to make sure this works for all.
+    NotationStaff *staff =
+            getStaffForSceneCoords(pointerSX, pointerSY);
+    // If this is a different staff from the current...
+    if (staff != currentStaff) {
+        // set this as the current staff
+        setCurrentStaff(staff);
+        // ??? Should we change the selection to indicate new current staff?
+        //     That's NotationView::slotEditSelectWholeStaff() that we
+        //     usually use.  So the caller will likely need to do that.
+    }
+}
+#endif
+
 
 }

src/gui/editors/notation/NotationScene.h

@@ -91,6 +91,8 @@ class NotationScene : public QGraphicsScene,
     int getCurrentStaffNumber() { return m_currentStaff; }
     NotationStaff *getCurrentStaff();
     void setCurrentStaff(NotationStaff *);
+    /// Set current staff to the staff nearest time t.
+    void setCurrentStaff(timeT t);
 
     NotationStaff *getStaffAbove(timeT t);
     NotationStaff *getStaffBelow(timeT t);
@@ -374,7 +376,7 @@ public slots:
     bool m_editRepeated;   // Direct edition of repeated segments is allowed
     bool m_haveInittedCurrentStaff;
 
-    NotationStaff * m_previewNoteStaff;  // Remember where the preview note was
+    NotationStaff *m_previewNoteStaff;  // Remember where the preview note was
 
     // Remember current labels of tracks
     std::map<int, std::string> m_trackLabels;

src/gui/editors/notation/NotationSelector.cpp

@@ -403,7 +403,7 @@ void NotationSelector::slotMoveInsertionCursor()
     // and we don't want to see any move of the staves
     m_widget->setScroll(false);
 
-    ///! Warning, this short-circuits NotationView::setCurrentStaff...
+    // ! Warning, this short-circuits NotationView::setCurrentStaff...
     m_scene->setCurrentStaff(m_pointerStaff);
     m_widget->setPointerPosition(m_pointerTime);
 

src/gui/editors/notation/NotationSelector.h

@@ -33,13 +33,16 @@ class QGraphicsRectItem;
 namespace Rosegarden
 {
 
+
 class ViewElement;
 class NotationWidget;
 class NotationElement;
 class EventSelection;
 class Event;
 class NotationStaff;
 
+
+/// The Notation Arrow Tool
 /**
  * Rectangular note selection
  */

src/gui/editors/notation/NotationWidget.cpp

@@ -923,14 +923,24 @@ NotationWidget::updatePointer(timeT t)
 
     SequenceManager *seqMgr = m_document->getSequenceManager();
 
-    bool rolling =
+    const bool rolling =
             (seqMgr  &&
              (seqMgr->getTransportStatus() == PLAYING  ||
               seqMgr->getTransportStatus() == RECORDING));
 
     //RG_DEBUG << "updatePointer(" << t << "): rolling = " << rolling;
 
-    NotationScene::CursorCoordinates cursorPos =
+    // Avoid jumping around when stop is pressed.
+    // Bug #1672.
+    // ??? Unfortunately, this also breaks the current segment wheel.
+    //     Maybe we should only do it on stop?
+    //if (!rolling)
+    //    m_scene->setCurrentStaff(t);
+
+    // This limits the cursor to within the current staff.  That can
+    // cause the notation view to jump unexpectedly.
+    // Bug #1672.
+    const NotationScene::CursorCoordinates cursorPos =
             m_scene->getCursorCoordinates(t);
 
     // While rolling, display a playback position pointer that stretches
@@ -1460,6 +1470,7 @@ NotationWidget::setPointerPosition(timeT t)
     // Fixes problem with sustaining notes while adding notes with
     // the pencil tool.  Also avoids moving playback position in
     // playback mode, allowing editing of a loop in real-time.
+    // ??? A flag in RMW would be faster.  E.g. RMW::m_enableSetPointerPosition.
     disconnect(m_document, &RosegardenDocument::pointerPositionChanged,
                RosegardenMainWindow::self(),
                &RosegardenMainWindow::slotSetPointerPosition);

src/gui/editors/notation/NotationWidget.h

@@ -148,7 +148,13 @@ class NotationWidget : public QWidget,
     void dispatchMouseDoubleClick(const NotationMouseEvent *);
     void dispatchWheelTurned(int, const NotationMouseEvent *);
 
-    //  Valid or inhibit scrolling to kept the cursor in the view
+    /// Temporarily disable/enable follow mode.
+    /**
+     * Used only by NotationSelector::slotMoveInsertionCursor() to turn off
+     * follow mode temporarily.
+     *
+     * See m_scrollToFollow.
+     */
     void setScroll(bool scroll) { m_noScroll = !scroll; }
 
 signals:
@@ -269,13 +275,16 @@ signals :
     void currentSegmentNext();
 
 private:
+
     RosegardenDocument *m_document; // I do not own this
     Panned *m_view; // I own this
     Panner *m_hpanner; // I own this
     NotationScene *m_scene; // I own this
     int m_leftGutter;
     NotationToolBox *m_toolBox;
     NotationTool *m_currentTool;
+
+    /// Follow mode.
     bool m_scrollToFollow;
 
     double m_hZoomFactor;
@@ -355,7 +364,8 @@ signals :
 
     bool m_updatesSuspended;
 
-    bool m_noScroll;    // If true, don't scroll to keep the cursor in the view
+    /// Used to temporarily disable follow mode.  See setScroll().
+    bool m_noScroll;
 
     void locatePanner(bool vertical);