[FuzzBug11] Correctly handle selection when undoing. CC #116

src/timeline2/model/timelinemodel.cpp

@@ -79,7 +79,7 @@ RTTR_REGISTRATION
         .method("requestTrackInsertion", select_overload<bool(int, int &, const QString &, bool)>(&TimelineModel::requestTrackInsertion))(
             parameter_names("pos", "id", "trackName", "audioTrack"))
         .method("requestTrackDeletion", select_overload<bool(int)>(&TimelineModel::requestTrackDeletion))(parameter_names("trackId"))
-        .method("requestClearSelection", select_overload<void(bool)>(&TimelineModel::requestClearSelection))(parameter_names("onDeletion"))
+        .method("requestClearSelection", select_overload<bool(bool)>(&TimelineModel::requestClearSelection))(parameter_names("onDeletion"))
         .method("requestAddToSelection", &TimelineModel::requestAddToSelection)(parameter_names("itemId", "clear"))
         .method("requestRemoveFromSelection", &TimelineModel::requestRemoveFromSelection)(parameter_names("itemId"))
         .method("requestSetSelection", select_overload<bool(const std::unordered_set<int> &)>(&TimelineModel::requestSetSelection))(parameter_names("itemIds"))
@@ -944,8 +944,8 @@ bool TimelineModel::requestClipInsertion(const QString &binClipId, int trackId,
 bool TimelineModel::requestClipInsertion(const QString &binClipId, int trackId, int position, int &id, bool logUndo, bool refreshView, bool useTargets,
                                          Fun &undo, Fun &redo)
 {
-    std::function<bool(void)> local_undo = []() { return true; };
-    std::function<bool(void)> local_redo = []() { return true; };
+    Fun local_undo = []() { return true; };
+    Fun local_redo = []() { return true; };
     qDebug() << "requestClipInsertion " << binClipId << " "
              << " " << trackId << " " << position;
     bool res = false;
@@ -1906,9 +1906,7 @@ Fun TimelineModel::deregisterClip_lambda(int clipId)
 {
     return [this, clipId]() {
         // qDebug() << " // /REQUEST TL CLP DELETION: " << clipId << "\n--------\nCLIPS COUNT: " << m_allClips.size();
-        if (getCurrentSelection().count(clipId) > 0) {
-            requestClearSelection();
-        }
+        requestClearSelection(true);
         clearAssetView(clipId);
         Q_ASSERT(m_allClips.count(clipId) > 0);
         Q_ASSERT(getClipTrackId(clipId) == -1); // clip must be deleted from its track at this point
@@ -2213,6 +2211,7 @@ Fun TimelineModel::deregisterComposition_lambda(int compoId)
     return [this, compoId]() {
         Q_ASSERT(m_allCompositions.count(compoId) > 0);
         Q_ASSERT(!m_groups->isInGroup(compoId)); // composition must be ungrouped at this point
+        requestClearSelection(true);
         clearAssetView(compoId);
         m_allCompositions.erase(compoId);
         m_groups->destructGroupItem(compoId);
@@ -2934,12 +2933,13 @@ int TimelineModel::getNextTrackId(int trackId)
     return it == m_allTracks.end() ? trackId : (*it)->getId();
 }
 
-void TimelineModel::requestClearSelection(bool onDeletion)
+bool TimelineModel::requestClearSelection(bool onDeletion)
 {
     QWriteLocker locker(&m_lock);
     TRACE();
     if (m_currentSelection == -1) {
-        return;
+        TRACE_RES(true);
+        return true;
     }
     if (isGroup(m_currentSelection)) {
         if (m_groups->getType(m_currentSelection) == GroupType::Selection) {
@@ -2950,6 +2950,8 @@ void TimelineModel::requestClearSelection(bool onDeletion)
     }
     m_currentSelection = -1;
     emit selectionChanged();
+    TRACE_RES(true);
+    return true;
 }
 void TimelineModel::requestClearSelection(bool onDeletion, Fun &undo, Fun &redo)
 {

src/timeline2/model/timelinemodel.hpp

@@ -609,7 +609,7 @@ public:
     /** @brief Clear the current selection
         @param onDeletion is true when the selection is cleared as a result of a deletion
      */
-    Q_INVOKABLE void requestClearSelection(bool onDeletion = false);
+    Q_INVOKABLE bool requestClearSelection(bool onDeletion = false);
     // same function with undo/redo accumulation
     void requestClearSelection(bool onDeletion, Fun &undo, Fun &redo);
 

tests/regressions.cpp

@@ -1229,3 +1229,89 @@ TEST_CASE("FuzzBug10")
     }
     pCore->m_projectManager = nullptr;
 }
+
+TEST_CASE("FuzzBug11")
+{
+    auto binModel = pCore->projectItemModel();
+    binModel->clean();
+    std::shared_ptr<DocUndoStack> undoStack = std::make_shared<DocUndoStack>(nullptr);
+    std::shared_ptr<MarkerListModel> guideModel = std::make_shared<MarkerListModel>(undoStack);
+    TimelineModel::next_id = 0;
+    {
+        Mock<ProjectManager> pmMock;
+        When(Method(pmMock, undoStack)).AlwaysReturn(undoStack);
+        ProjectManager &mocked = pmMock.get();
+        pCore->m_projectManager = &mocked;
+        TimelineItemModel tim_0(&reg_profile, undoStack);
+        Mock<TimelineItemModel> timMock_0(tim_0);
+        auto timeline_0 = std::shared_ptr<TimelineItemModel>(&timMock_0.get(), [](...) {});
+        TimelineItemModel::finishConstruct(timeline_0, guideModel);
+        Fake(Method(timMock_0, adjustAssetRange));
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        createProducer(reg_profile, "red", binModel, 20, true);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        createProducer(reg_profile, "blue", binModel, 0, true);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        createProducer(reg_profile, "green", binModel, 20, true);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        TrackModel::construct(timeline_0, -1, -1, "$", true);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        TrackModel::construct(timeline_0, -1, -1, "", false);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        createProducerWithSound(reg_profile, binModel);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        {
+            int dummy_3;
+            bool res = timeline_0->requestClipInsertion("5", 2, 3, dummy_3, true, true, false);
+            REQUIRE(res == true);
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        {
+            bool res = timeline_0->requestSetSelection({4});
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+    }
+    pCore->m_projectManager = nullptr;
+}