From 3daa2e87bbd86778620588f34c2755bac48eaf05 Mon Sep 17 00:00:00 2001 From: Ted Felix Date: Wed, 29 May 2024 21:17:14 -0400 Subject: [PATCH 1/3] Cleanup --- src/gui/editors/notation/NotationScene.cpp | 39 +++++++++++-------- src/gui/editors/notation/NotationSelector.cpp | 2 +- src/gui/editors/notation/NotationSelector.h | 3 ++ src/gui/editors/notation/NotationWidget.h | 14 ++++++- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/gui/editors/notation/NotationScene.cpp b/src/gui/editors/notation/NotationScene.cpp index 66fce1865..8e47ddf57 100644 --- a/src/gui/editors/notation/NotationScene.cpp +++ b/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 diff --git a/src/gui/editors/notation/NotationSelector.cpp b/src/gui/editors/notation/NotationSelector.cpp index f57f68099..59aa707c6 100644 --- a/src/gui/editors/notation/NotationSelector.cpp +++ b/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); diff --git a/src/gui/editors/notation/NotationSelector.h b/src/gui/editors/notation/NotationSelector.h index 0b6a39ded..c55592c6b 100644 --- a/src/gui/editors/notation/NotationSelector.h +++ b/src/gui/editors/notation/NotationSelector.h @@ -33,6 +33,7 @@ class QGraphicsRectItem; namespace Rosegarden { + class ViewElement; class NotationWidget; class NotationElement; @@ -40,6 +41,8 @@ class EventSelection; class Event; class NotationStaff; + +/// The Notation Arrow Tool /** * Rectangular note selection */ diff --git a/src/gui/editors/notation/NotationWidget.h b/src/gui/editors/notation/NotationWidget.h index 68b224e8b..c2e0e2ba3 100644 --- a/src/gui/editors/notation/NotationWidget.h +++ b/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,6 +275,7 @@ signals : void currentSegmentNext(); private: + RosegardenDocument *m_document; // I do not own this Panned *m_view; // I own this Panner *m_hpanner; // I own this @@ -276,6 +283,8 @@ signals : 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); From a4c7e85b6a13fdef948935176652a5e2dba7dc93 Mon Sep 17 00:00:00 2001 From: Ted Felix Date: Wed, 29 May 2024 22:12:00 -0400 Subject: [PATCH 2/3] Use forward declarations --- src/gui/dialogs/CheckForParallelsDialog.cpp | 3 +++ src/gui/dialogs/CheckForParallelsDialog.h | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/gui/dialogs/CheckForParallelsDialog.cpp b/src/gui/dialogs/CheckForParallelsDialog.cpp index 51d79059e..f5084b97f 100644 --- a/src/gui/dialogs/CheckForParallelsDialog.cpp +++ b/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 diff --git a/src/gui/dialogs/CheckForParallelsDialog.h b/src/gui/dialogs/CheckForParallelsDialog.h index c24bf9f1c..2119115b3 100644 --- a/src/gui/dialogs/CheckForParallelsDialog.h +++ b/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 #include @@ -65,6 +62,11 @@ namespace Rosegarden { +class NotationView; +class NotationScene; +class NotationStaff; + + class CheckForParallelsDialog : public QDialog { Q_OBJECT From c0107e2f950496f1a10a749fa09cb3fb7812e66a Mon Sep 17 00:00:00 2001 From: Ted Felix Date: Wed, 3 Jul 2024 13:56:26 -0400 Subject: [PATCH 3/3] Bug 1672 experiments --- src/gui/editors/notation/NotationScene.cpp | 33 +++++++++++++++++++++ src/gui/editors/notation/NotationScene.h | 4 ++- src/gui/editors/notation/NotationWidget.cpp | 15 ++++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/gui/editors/notation/NotationScene.cpp b/src/gui/editors/notation/NotationScene.cpp index 8e47ddf57..c2698f06d 100644 --- a/src/gui/editors/notation/NotationScene.cpp +++ b/src/gui/editors/notation/NotationScene.cpp @@ -2245,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 + } diff --git a/src/gui/editors/notation/NotationScene.h b/src/gui/editors/notation/NotationScene.h index 848d7edaf..64f736d14 100644 --- a/src/gui/editors/notation/NotationScene.h +++ b/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 m_trackLabels; diff --git a/src/gui/editors/notation/NotationWidget.cpp b/src/gui/editors/notation/NotationWidget.cpp index 246c72330..443acbbb2 100644 --- a/src/gui/editors/notation/NotationWidget.cpp +++ b/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);