Merge pull request #21033 from Eism/notation_view_range_start_playback_fix

fixed #11493: Playback incorrectly starts at the end of a selection

Author: RomanPudashkin

src/notation/tests/notationviewinputcontroller_tests.cpp

@@ -215,6 +215,7 @@ TEST_F(NotationViewInputControllerTests, DISABLED_WheelEvent_Zoom)
 /**
  * @brief Mouse_Press_Range_Start_Drag_From_Selected_Element
  * @details User pressed left mouse button on already selected note
+ *          The new note should be seeked and played, but no selected again
  */
 TEST_F(NotationViewInputControllerTests, Mouse_Press_Range_Start_Drag_From_Selected_Element)
 {
@@ -224,9 +225,6 @@ TEST_F(NotationViewInputControllerTests, Mouse_Press_Range_Start_Drag_From_Selec
     //! [GIVEN] Previous selected note
     INotationInteraction::HitElementContext oldContext = hitContext(score, true /*first note*/);
 
-    EXPECT_CALL(*m_interaction, hitElementContext())
-    .WillOnce(ReturnRef(oldContext));
-
     //! [GIVEN] User selected new note that was already selected
     INotationInteraction::HitElementContext newContext = hitContext(score, false /*last note*/);
     newContext.element->setSelected(true);
@@ -241,6 +239,11 @@ TEST_F(NotationViewInputControllerTests, Mouse_Press_Range_Start_Drag_From_Selec
     EXPECT_CALL(*m_interaction, setHitElementContext(newContext))
     .Times(1);
 
+    EXPECT_CALL(*m_interaction, hitElementContext())
+    .Times(2)
+    .WillOnce(ReturnRef(oldContext))
+    .WillOnce(ReturnRef(newContext));
+
     //! [GIVEN] There is a range selection
     ON_CALL(*m_selection, isRange())
     .WillByDefault(Return(true));
@@ -252,18 +255,143 @@ TEST_F(NotationViewInputControllerTests, Mouse_Press_Range_Start_Drag_From_Selec
     EXPECT_CALL(*m_playbackController, isPlaying())
     .WillOnce(Return(false));
 
-    //! [THEN] We will seek and play selected note
+    //! [THEN] We will seek and play selected note, but no select again
     EXPECT_CALL(*m_playbackController, seekElement(newContext.element))
     .Times(1);
 
     std::vector<const EngravingItem*> elements = { newContext.element };
     EXPECT_CALL(*m_playbackController, playElements(elements))
     .Times(1);
 
-    //! [THEN] We will not select already selected note
-    EXPECT_CALL(*m_interaction, select(_, _, _))
+    std::vector<EngravingItem*> selectElements = { newContext.element };
+    EXPECT_CALL(*m_interaction, select(selectElements, _, _))
     .Times(0);
 
     //! [WHEN] User pressed left mouse button
     m_controller->mousePressEvent(make_mousePressEvent(Qt::LeftButton, Qt::NoModifier, QPoint(100, 100)));
 }
+
+/**
+ * @brief Mouse_Press_Range_Start_Play_From_First_Playable_Element
+ * @details User selected a range in a note that is located after the previous selected note
+ *          The new note should be selected and played, but no seeked
+ *          The first note from a range should be seeked
+ */
+TEST_F(NotationViewInputControllerTests, Mouse_Press_Range_Start_Play_From_First_Playable_Element)
+{
+    //! [GIVEN] There is a test score
+    engraving::MasterScore* score = engraving::ScoreRW::readScore(TEST_SCORE_PATH);
+
+    //! [GIVEN] Previous selected note
+    INotationInteraction::HitElementContext oldContext = hitContext(score, true /*first note*/);
+
+    //! [GIVEN] User selected new note that is located after the previous selected note
+    INotationInteraction::HitElementContext newContext = hitContext(score, false /*last note*/);
+
+    EXPECT_CALL(*m_interaction, hitElement(_, _))
+    .WillOnce(Return(newContext.element));
+
+    EXPECT_CALL(*m_interaction, hitStaff(_))
+    .WillOnce(Return(newContext.element->staff()));
+
+    //! [GIVEN] The hew hit element context with new note will be set
+    EXPECT_CALL(*m_interaction, setHitElementContext(newContext))
+    .Times(1);
+
+    EXPECT_CALL(*m_interaction, hitElementContext())
+    .Times(2)
+    .WillOnce(ReturnRef(oldContext))
+    .WillOnce(ReturnRef(newContext));
+
+    //! [GIVEN] No note enter mode, no playing
+    EXPECT_CALL(m_view, isNoteEnterMode())
+    .WillOnce(Return(false));
+
+    EXPECT_CALL(*m_playbackController, isPlaying())
+    .WillOnce(Return(false));
+
+    //! [THEN] We will select and play selected note, but no seek
+    std::vector<EngravingItem*> selectElements = { newContext.element };
+    EXPECT_CALL(*m_interaction, select(selectElements, _, _))
+    .Times(1);
+
+    std::vector<const EngravingItem*> playElements = { newContext.element };
+    EXPECT_CALL(*m_playbackController, playElements(playElements))
+    .Times(1);
+
+    EXPECT_CALL(*m_playbackController, seekElement(newContext.element))
+    .Times(0);
+
+    //! [GIVEN] There is a range selection with two notes
+    ON_CALL(*m_selection, isRange())
+    .WillByDefault(Return(true));
+
+    selectElements.push_back(oldContext.element);
+    EXPECT_CALL(*m_selection, elements())
+    .WillOnce(Return(selectElements));
+
+    //! [THEN] We will seek first note in the range
+    EXPECT_CALL(*m_playbackController, seekElement(oldContext.element))
+    .Times(1);
+
+    //! [WHEN] User pressed left mouse button with ShiftModifier on the new note
+    m_controller->mousePressEvent(make_mousePressEvent(Qt::LeftButton, Qt::ShiftModifier, QPoint(100, 100)));
+}
+
+/**
+ * @brief Mouse_Press_On_Already_Selected_Element
+ * @details User pressed on already selected note
+ *          The selected note should not be selected again, but should be played and seeked
+ */
+TEST_F(NotationViewInputControllerTests, Mouse_Press_On_Already_Selected_Element)
+{
+    //! [GIVEN] There is a test score
+    engraving::MasterScore* score = engraving::ScoreRW::readScore(TEST_SCORE_PATH);
+
+    //! [GIVEN] Previous selected note
+    INotationInteraction::HitElementContext oldContext = hitContext(score, true /*first note*/);
+    oldContext.element->setSelected(true);
+
+    //! [GIVEN] User pressed on the previous selected note
+    INotationInteraction::HitElementContext newContext = oldContext;
+
+    EXPECT_CALL(*m_interaction, hitElement(_, _))
+    .WillOnce(Return(newContext.element));
+
+    EXPECT_CALL(*m_interaction, hitStaff(_))
+    .WillOnce(Return(newContext.element->staff()));
+
+    EXPECT_CALL(*m_interaction, setHitElementContext(newContext))
+    .Times(1);
+
+    EXPECT_CALL(*m_interaction, hitElementContext())
+    .Times(2)
+    .WillOnce(ReturnRef(oldContext))
+    .WillOnce(ReturnRef(newContext));
+
+    //! [GIVEN] No note enter mode, no playing
+    EXPECT_CALL(m_view, isNoteEnterMode())
+    .WillOnce(Return(false));
+
+    EXPECT_CALL(*m_playbackController, isPlaying())
+    .WillOnce(Return(false));
+
+    //! [THEN] We will no select already selected note, but play and seek
+    std::vector<EngravingItem*> selectElements = { newContext.element };
+    EXPECT_CALL(*m_interaction, select(selectElements, _, _))
+    .Times(0);
+
+    std::vector<const EngravingItem*> playElements = { newContext.element };
+    EXPECT_CALL(*m_playbackController, playElements(playElements))
+    .Times(1);
+
+    EXPECT_CALL(*m_playbackController, seekElement(newContext.element))
+    .Times(1);
+
+    //! [GIVEN] There is no a range selection
+    ON_CALL(*m_selection, isRange())
+    .WillByDefault(Return(false));
+
+    //! [WHEN] User pressed left mouse button with ShiftModifier on the new note
+    m_controller->mousePressEvent(make_mousePressEvent(Qt::LeftButton, Qt::NoModifier, QPoint(100, 100)));
+}

src/notation/view/notationviewinputcontroller.cpp

@@ -39,6 +39,23 @@ using namespace mu::commonscene;
 
 static constexpr int PIXELSSTEPSFACTOR = 5;
 
+static bool seekAllowed(const mu::engraving::EngravingItem* element)
+{
+    if (!element) {
+        return false;
+    }
+
+    static const ElementTypeSet playableTypes = {
+        ElementType::NOTE,
+        ElementType::REST,
+        ElementType::MMREST,
+        ElementType::MEASURE,
+        ElementType::BAR_LINE
+    };
+
+    return contains(playableTypes, element->type());
+}
+
 NotationViewInputController::NotationViewInputController(IControlledView* view)
     : m_view(view)
 {
@@ -579,21 +596,11 @@ void NotationViewInputController::mousePressEvent(QMouseEvent* event)
     m_physicalBeginPoint = event->pos();
     m_logicalBeginPoint = logicPos;
 
-    if (hitElement) {
-        switch (hitElement->type()) {
-        case ElementType::NOTE:
-        case ElementType::REST:
-        case ElementType::MMREST:
-        case ElementType::MEASURE:
-        case ElementType::BAR_LINE: {
+    if (playbackController()->isPlaying()) {
+        if (seekAllowed(hitElement)) {
             playbackController()->seekElement(hitElement);
-            break;
-        }
-        default: break;
         }
-    }
 
-    if (playbackController()->isPlaying()) {
         return;
     }
 
@@ -622,6 +629,11 @@ void NotationViewInputController::mousePressEvent(QMouseEvent* event)
         viewInteraction()->select({ hitElement }, selectType, hitStaffIndex);
     }
 
+    EngravingItem* playbackStartElement = resolveStartPlayableElement();
+    if (playbackStartElement) {
+        playbackController()->seekElement(playbackStartElement);
+    }
+
     if (button == Qt::LeftButton) {
         handleLeftClick(ctx);
     } else if (button == Qt::RightButton) {
@@ -1129,3 +1141,33 @@ void NotationViewInputController::togglePopupForItemIfSupports(const EngravingIt
         m_view->toggleElementPopup(type, item->canvasBoundingRect());
     }
 }
+
+EngravingItem* NotationViewInputController::resolveStartPlayableElement() const
+{
+    EngravingItem* hitElement = hitElementContext().element;
+
+    INotationSelectionPtr selection = viewInteraction()->selection();
+    if (!selection->isRange()) {
+        return hitElement;
+    }
+
+    EngravingItem* playbackStartElement = hitElement;
+
+    for (EngravingItem* element: selection->elements()) {
+        if (!element || element == playbackStartElement) {
+            continue;
+        }
+
+        if (!seekAllowed(element)) {
+            continue;
+        }
+
+        if (playbackStartElement && playbackStartElement->tick() <= element->tick()) {
+            continue;
+        }
+
+        playbackStartElement = element;
+    }
+
+    return playbackStartElement;
+}

src/notation/view/notationviewinputcontroller.h

@@ -178,6 +178,8 @@ class NotationViewInputController : public actions::Actionable, public async::As
     bool startTextEditingAllowed() const;
     void updateTextCursorPosition();
 
+    EngravingItem* resolveStartPlayableElement() const;
+
     IControlledView* m_view = nullptr;
 
     QList<int> m_possibleZoomPercentages;

src/playback/internal/playbackcontroller.cpp

@@ -1164,10 +1164,6 @@ void PlaybackController::setupSequencePlayer()
             return;
         }
 
-        if (!isPlaying()) {
-            return;
-        }
-
         setCurrentPlaybackTime(msecs);
         m_tickPlayed.send(m_currentTick);
     });