[vlc-devel] [RFC 1/8] qt: playlist: refactor "move items"

Romain Vimont rom1v at videolabs.io
Tue Jul 9 18:30:25 CEST 2019


Moving a set of items to a given target is ambiguous: the target index
may be expressed either _before_ or _after_ the move is actually
applied.

                pre-target
   move +----------+                       target
        |          |                         |
    |-------|      v                         v
  A [B  C  D] E  F  G      ------>   A  E  F [B  C  D] G
  0  1  2  3  4  5  6                0  1  2  3  4  5  6

The core playlist expects the target to be the position _after_ the
move.

In the UI, we need both:
 - during a drag&drop, we naturally know the position of the "drop",
   which is the target _before_ the move;
 - during a move using key navigation, we shift the items by a given
   amount (typically 1), so we know the expected target _after_ the
   move.

To express a move request in a "natural" way, expose two separate
functions to move items in Qt, one accepting the pre-target and another
the post-target.
---
 .../qt/components/playlist/playlist_model.cpp | 75 +++++++++++++------
 .../qt/components/playlist/playlist_model.hpp |  6 +-
 .../gui/qt/qml/playlist/PlaylistListView.qml  | 58 ++++++++------
 3 files changed, 95 insertions(+), 44 deletions(-)

diff --git a/modules/gui/qt/components/playlist/playlist_model.cpp b/modules/gui/qt/components/playlist/playlist_model.cpp
index add1ff1501..12edef2296 100644
--- a/modules/gui/qt/components/playlist/playlist_model.cpp
+++ b/modules/gui/qt/components/playlist/playlist_model.cpp
@@ -300,36 +300,67 @@ void PlaylistListModel::removeItems(const QList<int>& indexes)
     }
 }
 
-void PlaylistListModel::moveItems(const QList<int> &indexes, int target)
+/**
+ * Return the target position *after* the move has been applied, knowing the
+ * index *before* the move and the list of indexes to move.
+ *
+ * The core playlist interprets the move target as the index of the (first)
+ * item *after* the move. During a drag&drop, we know the index *before* the
+ * move.
+ */
+static int
+getMovePostTarget(const QList<int> &sortedIndexesToMove, int preTarget)
+{
+    int postTarget = preTarget;
+    for (int index : sortedIndexesToMove)
+    {
+        if (index >= preTarget)
+            break;
+        postTarget--;
+    }
+    return postTarget;
+}
+
+void
+PlaylistListModel::moveItems(const QList<int> &sortedIndexes, int target,
+                             bool isPreTarget)
 {
     Q_D(PlaylistListModel);
     if (!d->m_playlist)
         return;
-    int targetPre = target;
-    int targetPost = target;
-    if (indexes.size() == 0)
+    if (sortedIndexes.size() == 0)
         return;
     QVector<vlc_playlist_item_t*> itemsToMove;
-    //std::sort(indexes.begin(), indexes.end()); //Indexes are already sorted
-    std::transform(indexes.begin(), indexes.end(),std::back_inserter(itemsToMove), [&] (int index) {
-        return d->m_items[index].raw();
-    });
-    //the target is in the referential of the list before doing the operation,
-    //the playlist expect the target to be defined as the index after the operation
-    for ( const int index : indexes ) {
-        if (index < targetPre)
-            targetPost--;
-        else
-            break;
+    std::transform(sortedIndexes.begin(), sortedIndexes.end(),
+                   std::back_inserter(itemsToMove),
+                   [&] (int index) {
+                       return d->m_items[index].raw();
+                   });
+
+    if (isPreTarget) {
+        /* convert pre-target to post-target */
+        target = getMovePostTarget(sortedIndexes, target);
     }
 
-    {
-        PlaylistLocker locker(d->m_playlist);
-        int ret = vlc_playlist_RequestMove(d->m_playlist, itemsToMove.constData(),
-                                           itemsToMove.size(), targetPost, indexes[0]);
-        if (ret != VLC_SUCCESS)
-            throw std::bad_alloc();
-    }
+    PlaylistLocker locker(d->m_playlist);
+    int ret = vlc_playlist_RequestMove(d->m_playlist, itemsToMove.constData(),
+                                       itemsToMove.size(), target,
+                                       sortedIndexes[0]);
+    if (ret != VLC_SUCCESS)
+        throw std::bad_alloc();
+}
+
+void
+PlaylistListModel::moveItemsPre(const QList<int> &sortedIndexes, int preTarget)
+{
+    return moveItems(sortedIndexes, preTarget, true);
+}
+
+void
+PlaylistListModel::moveItemsPost(const QList<int> &sortedIndexes,
+                                 int postTarget)
+{
+    return moveItems(sortedIndexes, postTarget, false);
 }
 
 int PlaylistListModel::getCurrentIndex() const
diff --git a/modules/gui/qt/components/playlist/playlist_model.hpp b/modules/gui/qt/components/playlist/playlist_model.hpp
index 6d1eafb2f4..a1215496ac 100644
--- a/modules/gui/qt/components/playlist/playlist_model.hpp
+++ b/modules/gui/qt/components/playlist/playlist_model.hpp
@@ -62,7 +62,8 @@ public:
     const PlaylistItem &itemAt(int index) const;
 
     Q_INVOKABLE virtual void removeItems(const QList<int> &indexes);
-    Q_INVOKABLE virtual void moveItems(const QList<int> &indexes, int target);
+    Q_INVOKABLE virtual void moveItemsPre(const QList<int> &indexes, int preTarget);
+    Q_INVOKABLE virtual void moveItemsPost(const QList<int> &indexes, int postTarget);
 
     int getCurrentIndex() const;
 
@@ -77,6 +78,9 @@ signals:
 
 private:
     Q_DECLARE_PRIVATE(PlaylistListModel)
+
+    void moveItems(const QList<int> &indexes, int target, bool isPreTarget);
+
     QScopedPointer<PlaylistListModelPrivate> d_ptr;
 
 };
diff --git a/modules/gui/qt/qml/playlist/PlaylistListView.qml b/modules/gui/qt/qml/playlist/PlaylistListView.qml
index 54e75d8eea..bd69a060ea 100644
--- a/modules/gui/qt/qml/playlist/PlaylistListView.qml
+++ b/modules/gui/qt/qml/playlist/PlaylistListView.qml
@@ -84,8 +84,14 @@ Utils.NavigableFocusScope {
                 onItemClicked : {
                     view.forceActiveFocus()
                     if (delegateModel.mode == "move") {
-                        delegateModel.onMoveSelectionAtPos(index)
-                        view.currentIndex = index
+                        var selectedIndexes = delegateModel.getSelectedIndexes()
+                        var preTarget = index
+                        /* move to _above_ the clicked item if move up, but
+                         * _below_ the clicked item if move down */
+                        if (preTarget > selectedIndexes[0])
+                            preTarget++
+                        view.currentIndex = selectedIndexes[0]
+                        root.plmodel.moveItemsPre(selectedIndexes, preTarget)
                     } else if ( delegateModel.mode == "select" ) {
                     } else {
                         delegateModel.onUpdateIndex( modifier , view.currentIndex, index)
@@ -98,22 +104,36 @@ Utils.NavigableFocusScope {
                     if (drop.hasUrls) {
                         delegateModel.onDropUrlAtPos(drop.urls, target)
                     } else {
-                        delegateModel.onMoveSelectionAtPos(target)
+                        /* on drag&drop, the target is the position _before_
+                         * the move is applied */
+                        delegateModel.moveSelectionToPreTarget(target)
                     }
                 }
             }
         }
 
-        function onMoveSelectionAtPos(target) {
+        function getSelectedIndexes() {
             var list = []
             for (var i = 0; i < delegateModel.selectedGroup.count; i++ ) {
                 list.push(delegateModel.selectedGroup.get(i).itemsIndex)
             }
-            root.plmodel.moveItems(list, target)
+            return list;
+        }
+
+        function moveSelectionToPreTarget(target) {
+            var selectedIndexes = getSelectedIndexes()
+            view.currentIndex = selectedIndexes[0]
+            root.plmodel.moveItemsPre(selectedIndexes, target)
+        }
+
+        function moveSelectionToPostTarget(target) {
+            var selectedIndexes = getSelectedIndexes()
+            view.currentIndex = selectedIndexes[0]
+            root.plmodel.moveItemsPost(selectedIndexes, target)
         }
 
         function onDropMovedAtEnd() {
-            onMoveSelectionAtPos(items.count)
+            moveSelectionToPreTarget(items.count)
         }
 
         function onDropUrlAtPos(urls, target) {
@@ -160,25 +180,21 @@ Utils.NavigableFocusScope {
                 if (delegateModel.selectedGroup.count === 0)
                     return
 
-                var list = []
-                for (var i = 0; i < delegateModel.selectedGroup.count; i++ ) {
-                    list.push(delegateModel.selectedGroup.get(i).itemsIndex)
-                }
-                var minIndex= delegateModel.selectedGroup.get(0).itemsIndex
-                var maxIndex= delegateModel.selectedGroup.get(delegateModel.selectedGroup.count - 1).itemsIndex
+                var selectedIndexes = getSelectedIndexes()
 
+                /* always move relative to the first item of the selection */
+                var target = selectedIndexes[0];
                 if (newIndex > oldIndex) {
-                    //after the next item
-                    newIndex = Math.min(maxIndex + 2, delegateModel.items.count)
-                    view.currentIndex = Math.min(maxIndex, delegateModel.items.count)
-                } else if (newIndex < oldIndex) {
-                    //before the previous item
-                    view.currentIndex = Math.max(minIndex, 0)
-                    newIndex = Math.max(minIndex - 1, 0)
+                    /* move down */
+                    target++
+                } else if (newIndex < oldIndex && target > 0) {
+                    /* move up */
+                    target--
                 }
 
-                root.plmodel.moveItems(list, newIndex)
-
+                view.currentIndex = selectedIndexes[0]
+                /* the target is the position _after_ the move is applied */
+                root.plmodel.moveItemsPost(selectedIndexes, target)
             } else  { //normal
                 updateSelection( keyModifiers, oldIndex, newIndex )
             }
-- 
2.20.1



More information about the vlc-devel mailing list