[FuzzBug7] Tighter control on how a selection group is created

fuzzer/fuzzing.cpp

@@ -352,6 +352,12 @@ void fuzz(const std::string &input)
                             valid = valid && (groupId >= 0);
                             arguments.emplace_back(groupId);
                             // std::cout << "got clipId" << clipId << std::endl;
+                        } else if (arg_name == "logUndo") {
+                            bool a = false;
+                            ss >> a;
+                            // we enforce undo logging
+                            a = true;
+                            arguments.emplace_back(a);
                         } else if (arg_name == "itemIds") {
                             int count = 0;
                             ss >> count;

src/timeline2/model/timelinemodel.cpp

@@ -1612,10 +1612,15 @@ int TimelineModel::requestClipsGroup(const std::unordered_set<int> &ids, bool lo
 {
     QWriteLocker locker(&m_lock);
     TRACE(ids, logUndo, type);
+    if (type == GroupType::Selection) {
+        // this shouldn't be done here. Call requestSetSelection instead
+        TRACE_RES(-1);
+        return -1;
+    }
     Fun undo = []() { return true; };
     Fun redo = []() { return true; };
     int result = requestClipsGroup(ids, undo, redo, type);
-    if (result > -1 && logUndo && type != GroupType::Selection) {
+    if (result > -1 && logUndo) {
         PUSH_UNDO(undo, redo, i18n("Group clips"));
     }
     TRACE_RES(result);

src/timeline2/view/qml/timeline.qml

@@ -294,7 +294,7 @@ Rectangle {
 
                 // if the clip insertion succeeded, request the clips to be grouped
                 if (ids.length > 0) {
-                    timeline.groupClips(ids)
+                    timeline.selectItems(ids)
                     id = ids[0]
                 }
             }

src/timeline2/view/timelinecontroller.cpp

@@ -244,6 +244,12 @@ QList<int> TimelineController::selection() const
     return items;
 }
 
+void TimelineController::selectItems(const QList<int> &ids)
+{
+    std::unordered_set<int> ids_s(ids.begin(), ids.end());
+    m_model->requestSetSelection(ids_s);
+}
+
 void TimelineController::setScrollPos(int pos)
 {
     if (pos > 0 && m_root) {
@@ -1576,17 +1582,6 @@ void TimelineController::resetTrackHeight()
     m_model->dataChanged(modelStart, modelEnd, {TimelineModel::HeightRole});
 }
 
-int TimelineController::groupClips(const QList<int> &clipIds)
-{
-    std::unordered_set<int> theSet(clipIds.begin(), clipIds.end());
-    return m_model->requestClipsGroup(theSet, false, GroupType::Selection);
-}
-
-bool TimelineController::ungroupClips(int clipId)
-{
-    return m_model->requestClipUngroup(clipId);
-}
-
 void TimelineController::selectAll()
 {
     std::unordered_set<int> ids;

src/timeline2/view/timelinecontroller.h

@@ -104,6 +104,10 @@ public:
         @param addToSelect if true, the old selection is retained
     */
     Q_INVOKABLE void selectItems(const QVariantList &tracks, int startFrame, int endFrame, bool addToSelect);
+
+    /** @brief request a selection with a list of ids*/
+    Q_INVOKABLE void selectItems(const QList<int> &ids);
+
     /* @brief Returns true is item is selected as well as other items */
     Q_INVOKABLE bool isInSelection(int itemId);
 
@@ -163,16 +167,6 @@ public:
      * @return the ids of the inserted clips
      */
     Q_INVOKABLE QList<int> insertClips(int tid, int position, const QStringList &binIds, bool logUndo, bool refreshView);
-    /* @brief Request the grouping of the given clips
-     * @param clipIds the ids to be grouped
-     * @return the group id or -1 in case of faiure
-     */
-    Q_INVOKABLE int groupClips(const QList<int> &clipIds);
-    /* @brief Request the ungrouping of clips
-     * @param clipId the id of a clip belonging to the group
-     * @return true in case of success, false otherwise
-     */
-    Q_INVOKABLE bool ungroupClips(int clipId);
     Q_INVOKABLE void copyItem();
     Q_INVOKABLE bool pasteItem();
     /* @brief Request inserting a new composition in timeline (dragged from compositions list)

tests/regressions.cpp

@@ -953,3 +953,120 @@ TEST_CASE("FuzzBug6")
     }
     pCore->m_projectManager = nullptr;
 }
+
+TEST_CASE("FuzzBug7")
+{
+    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());
+        TrackModel::construct(timeline_0, -1, -1, "0", false);
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        TimelineItemModel tim_1(&reg_profile, undoStack);
+        Mock<TimelineItemModel> timMock_1(tim_1);
+        auto timeline_1 = std::shared_ptr<TimelineItemModel>(&timMock_1.get(), [](...) {});
+        TimelineItemModel::finishConstruct(timeline_1, guideModel);
+        Fake(Method(timMock_1, adjustAssetRange));
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        createProducer(reg_profile, "r5", binModel, 2, true);
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        {
+            int dummy_3;
+            bool res = timeline_0->requestClipInsertion("2", 1, 0, dummy_3, true, false, true);
+            REQUIRE(res == true);
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        {
+            int dummy_3;
+            bool res = timeline_0->requestClipInsertion("2", 1, 20, dummy_3, true, false, true);
+            REQUIRE(res == true);
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        {
+            int dummy_3;
+            bool res = timeline_0->requestClipInsertion("2", 1, 40, dummy_3, true, false, true);
+            REQUIRE(res == true);
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        {
+            int res = timeline_0->requestClipsGroup({4, 3}, true, GroupType::Selection);
+            REQUIRE(res == -1);
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        {
+            int res = timeline_0->requestClipsGroup({5, 3}, true, GroupType::Normal);
+            REQUIRE(res == 6);
+        }
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->undo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+        undoStack->redo();
+        REQUIRE(timeline_0->checkConsistency());
+        REQUIRE(timeline_1->checkConsistency());
+    }
+    pCore->m_projectManager = nullptr;
+}