[vlc-devel] [PATCH] Qt4 Playlist fix: when moving items keep their original order...

[Jakob Leben]

>From 04317a878773c29f3b2ec0167cd72887d77ee75b Mon Sep 17 00:00:00 2001
From: Jakob Leben <jakob.leben at gmail.com>
Date: Thu, 30 Jul 2009 20:15:41 +0200
Subject: [PATCH 2/2] Qt4 Playlist fix: when moving items keep their original
order...

When moving selected items in Qt4 playlist, their new order dependes on the
order of selection. This is fixed by ordering items before the drag
operation starts (PLModel::mimeData()).

Moreover, the order of items gets reversed when they are moved to drop
position, because they are all dropped onto the same index one after
another. This is fixed by always taking the index of previously inserted
item when moving next one.

This patch also allows for moving items to the top of the playlist by
interpreting the arguments to PLModel::dropMimeData() as they are intended
by qt design, which means that for a move operation items have to be dropped
between other items and not onto them. (Another patch will follow, which
will disable dropping onto items if they are not containers themself.)

The patch makes the drop operation atomic in the sense that it calls PL_LOCK
and PL_UNLOCK only once before and after the whole operation, thus
preventing any other thread from messing with it.
---
 .../gui/qt4/components/playlist/playlist_model.cpp |   89
++++++++++++-------
 1 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/modules/gui/qt4/components/playlist/playlist_model.cpp
b/modules/gui/qt4/components/playlist/playlist_model.cpp
index 998ebb2..1226a6f 100644
--- a/modules/gui/qt4/components/playlist/playlist_model.cpp
+++ b/modules/gui/qt4/components/playlist/playlist_model.cpp
@@ -135,10 +135,17 @@ QMimeData *PLModel::mimeData( const QModelIndexList
&indexes ) const
     QMimeData *mimeData = new QMimeData();
     QByteArray encodedData;
     QDataStream stream( &encodedData, QIODevice::WriteOnly );
+    QModelIndexList list;

     foreach( const QModelIndex &index, indexes ) {
         if( index.isValid() && index.column() == 0 )
-            stream << itemId( index );
+            list.append(index);
+    }
+
+    qSort(list);
+
+    foreach( const QModelIndex &index, list ) {
+        stream << itemId( index );
     }
     mimeData->setData( "vlc/playlist-item-id", encodedData );
     return mimeData;
@@ -153,58 +160,74 @@ bool PLModel::dropMimeData( const QMimeData *data,
Qt::DropAction action,
         if( action == Qt::IgnoreAction )
             return true;

-        if( !target.isValid() )
-            /* We don't want to move on an invalid position */
-            return true;
+        PL_LOCK;

-        PLItem *targetItem = static_cast<PLItem*>( target.internalPointer()
);
+        playlist_item_t *p_parent;
+
+        if( !parent.isValid())
+        {
+            if( row > -1)
+            {
+                // dropped into top node
+                p_parent = playlist_ItemGetById( p_playlist, rootItem->i_id
);
+            }
+            else
+            {
+                // dropped outside any item
+                PL_UNLOCK;
+                return true;
+            }
+        }
+        else
+        {
+            // dropped into/onto an item (depends on (row = -1) or (row >
-1))
+            PLItem *parentItem = static_cast<PLItem*>(
parent.internalPointer() );
+            p_parent = playlist_ItemGetById( p_playlist, parentItem->i_id
);
+        }
+        if( !p_parent || p_parent->i_children == -1 )
+        {
+            PL_UNLOCK;
+            return false;
+        }

         QByteArray encodedData = data->data( "vlc/playlist-item-id" );
         QDataStream stream( &encodedData, QIODevice::ReadOnly );

-        PLItem *newParentItem;
+        /* easiest way to never miss the right index to move to is to
+        track the previously moved item */
+        playlist_item_t *p_target = 0;
+
         while( !stream.atEnd() )
         {
-            int i;
-            int srcId;
-            stream >> srcId;
-
-            PL_LOCK;
-            playlist_item_t *p_target =
-                        playlist_ItemGetById( p_playlist, targetItem->i_id
);
-            playlist_item_t *p_src = playlist_ItemGetById( p_playlist,
srcId );
+            int src_id;
+            stream >> src_id;
+            playlist_item_t *p_src = playlist_ItemGetById( p_playlist,
src_id );

-            if( !p_target || !p_src )
+            if( !p_src )
             {
                 PL_UNLOCK;
                 return false;
             }
-            if( p_target->i_children == -1 ) /* A leaf */
+            if( !p_target )
             {
-                PLItem *parentItem = targetItem->parent();
-                assert( parentItem );
-                playlist_item_t *p_parent =
-                         playlist_ItemGetById( p_playlist, parentItem->i_id
);
-                if( !p_parent )
+                if(row == -1)
                 {
-                    PL_UNLOCK;
-                    return false;
+                    playlist_TreeMove( p_playlist, p_src, p_parent, 0 );
+                }
+                else {
+                    playlist_TreeMove( p_playlist, p_src, p_parent, row );
                 }
-                for( i = 0 ; i< p_parent->i_children ; i++ )
-                    if( p_parent->pp_children[i] == p_target ) break;
-                // Move the item to the element after i
-                playlist_TreeMove( p_playlist, p_src, p_parent, i + 1 );
-                newParentItem = parentItem;
             }
             else
             {
-                /* \todo: if we drop on a top-level node, use copy instead
? */
-                playlist_TreeMove( p_playlist, p_src, p_target, 0 );
-                i = 0;
-                newParentItem = targetItem;
+                int i;
+                for( i = 0 ; i< p_parent->i_children ; i++ )
+                    if( p_parent->pp_children[i] == p_target ) break;
+                playlist_TreeMove( p_playlist, p_src, p_parent, i + 1 );
             }
-            PL_UNLOCK;
+            p_target = p_src;
         }
+        PL_UNLOCK;
         /*TODO: That's not a good idea to rebuild the playlist */
         rebuild();
     }
-- 
1.6.4.msysgit.0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090730/ca327483/attachment.html>