Qt: Playlist drag and drop corrections

- Use item ids instead of pointers for drag data to prevent usage after freeing.
- The list of selected items that contitutes drag data is sorted correctly according to item position in tree. This keeps their relative order after drop operation even if they originated from different parent nodes.
- If a parent and any of it's children are selected, only the parent enters drag'n'drop operation. Children are thus not moved out of the parent when dropping.
- Copying by drag and drop performs a recursive copy of all children.

diff --git a/modules/gui/qt4/components/playlist/playlist_item.cpp b/modules/gui/qt4/components/playlist/playlist_item.cpp
index 6ad708fa7f663178627f1e3a4923f0f806b3d232..aab77281d6666a16b4d0296f78b7fefd50c418f0 100644 (file)
--- a/modules/gui/qt4/components/playlist/playlist_item.cpp
+++ b/modules/gui/qt4/components/playlist/playlist_item.cpp
@@ -112,3 +112,22 @@ int PLItem::row() const
     return 0;
 }
 
+bool PLItem::operator< ( PLItem& other )
+{
+    PLItem *item1 = this;
+    while( item1->parentItem )
+    {
+        PLItem *item2 = &other;
+        while( item2->parentItem )
+        {
+            if( item1 == item2->parentItem ) return true;
+            if( item2 == item1->parentItem ) return false;
+            if( item1->parentItem == item2->parentItem )
+                return item1->parentItem->children.indexOf( item1 ) <
+                       item1->parentItem->children.indexOf( item2 );
+            item2 = item2->parentItem;
+        }
+        item1 = item1->parentItem;
+    }
+    return false;
+}

diff --git a/modules/gui/qt4/components/playlist/playlist_item.hpp b/modules/gui/qt4/components/playlist/playlist_item.hpp
index 45f8d8c00a6e5d9de1d6d99011e95dda8ec00486..6caafcb9a776558da0a208a05410e9b60b4f3042 100644 (file)
--- a/modules/gui/qt4/components/playlist/playlist_item.hpp
+++ b/modules/gui/qt4/components/playlist/playlist_item.hpp
@@ -56,6 +56,7 @@ public:
     PLItem *parent() { return parentItem; }
     input_item_t *inputItem() { return p_input; }
     int id() { return i_id; }
+    bool operator< ( PLItem& );
 
 protected:
     QList<PLItem*> children;

diff --git a/modules/gui/qt4/components/playlist/playlist_model.cpp b/modules/gui/qt4/components/playlist/playlist_model.cpp
index 04349cd87e64fcea8493a2838387dae8160c79f7..09785fc5feff70b5a385b3a2e97be9c33e797249 100644 (file)
--- a/modules/gui/qt4/components/playlist/playlist_model.cpp
+++ b/modules/gui/qt4/components/playlist/playlist_model.cpp
@@ -141,6 +141,15 @@ QStringList PLModel::mimeTypes() const
     return types;
 }
 
+bool modelIndexLessThen( const QModelIndex &i1, const QModelIndex &i2 )
+{
+    if( !i1.isValid() || !i2.isValid() ) return false;
+    PLItem *item1 = static_cast<PLItem*>( i1.internalPointer() );
+    PLItem *item2 = static_cast<PLItem*>( i2.internalPointer() );
+    if( item1->parent() == item2->parent() ) return i1.row() < i2.row();
+    else return *item1 < *item2;
+}
+
 QMimeData *PLModel::mimeData( const QModelIndexList &indexes ) const
 {
     QMimeData *mimeData = new QMimeData();
@@ -153,11 +162,26 @@ QMimeData *PLModel::mimeData( const QModelIndexList &indexes ) const
             list.append(index);
     }
 
-    qSort(list);
+    qSort(list.begin(), list.end(), modelIndexLessThen);
 
+    PLItem *item = NULL;
     foreach( const QModelIndex &index, list ) {
-        PLItem *item = getItem( index );
-        stream.writeRawData( (char*) &item, sizeof( PLItem* ) );
+        if( item )
+        {
+            PLItem *testee = getItem( index );
+            while( testee->parent() )
+            {
+                if( testee->parent() == item ||
+                    testee->parent() == item->parent() ) break;
+                testee = testee->parent();
+            }
+            if( testee->parent() == item ) continue;
+            item = getItem( index );
+        }
+        else
+            item = getItem( index );
+
+        stream << item->id();
     }
     mimeData->setData( "vlc/qt-playlist-item", encodedData );
     return mimeData;
@@ -172,11 +196,9 @@ bool PLModel::dropMimeData( const QMimeData *data, Qt::DropAction action,
         if( action == Qt::IgnoreAction )
             return true;
 
-        PLItem *parentItem = parent.isValid() ? getItem( parent ) : rootItem;
-
         PL_LOCK;
         playlist_item_t *p_parent =
-            playlist_ItemGetById( p_playlist, parentItem->i_id );
+            playlist_ItemGetById( p_playlist, itemId( parent ) );
         if( !p_parent || p_parent->i_children == -1 )
         {
             PL_UNLOCK;
@@ -197,9 +219,9 @@ bool PLModel::dropMimeData( const QMimeData *data, Qt::DropAction action,
 
         QByteArray encodedData = data->data( "vlc/qt-playlist-item" );
         if( copy )
-            dropAppendCopy( encodedData, parentItem );
+            dropAppendCopy( encodedData, getItem( parent ) );
         else
-            dropMove( encodedData, parentItem, row );
+            dropMove( encodedData, getItem( parent ), row );
     }
     return true;
 }
@@ -211,78 +233,109 @@ void PLModel::dropAppendCopy( QByteArray& data, PLItem *target )
     PL_LOCK;
     playlist_item_t *p_parent =
             playlist_ItemGetById( p_playlist, target->i_id );
+    if( !p_parent ) return;
+
+    bool b_flat = p_parent == p_playlist->p_playing &&
+                  !var_InheritBool( p_intf, "playlist-tree" );
+
     while( !stream.atEnd() )
     {
-        PLItem *item;
-        stream.readRawData( (char*)&item, sizeof(PLItem*) );
-        playlist_item_t *p_item = playlist_ItemGetById( p_playlist, item->i_id );
+        int i_id;
+        stream >> i_id;
+        playlist_item_t *p_item = playlist_ItemGetById( p_playlist, i_id );
         if( !p_item ) continue;
-        input_item_t *p_input = p_item->p_input;
-        playlist_AddExt ( p_playlist,
-            p_input->psz_uri, p_input->psz_name,
-            PLAYLIST_APPEND | PLAYLIST_SPREPARSE, PLAYLIST_END,
-            p_input->i_duration,
-            p_input->i_options, p_input->ppsz_options, p_input->optflagc,
-            p_parent == p_playlist->p_playing,
-            true );
+
+        recursiveAppendCopy( p_playlist, p_item, p_parent, b_flat );
     }
     PL_UNLOCK;
 }
 
+/* Must be entered WITH playlist lock! */
+void PLModel::recursiveAppendCopy( playlist_t *p_playlist, playlist_item_t *source,
+                                   playlist_item_t *target, bool b_flat )
+{
+    input_item_t *srcInput = source->p_input;
+
+    if( !(source->i_children != -1 && b_flat) )
+    {
+        vlc_mutex_lock( &srcInput->lock );
+        input_item_t *newInput =
+            input_item_NewWithType( VLC_OBJECT(p_playlist),
+                                    srcInput->psz_uri, srcInput->psz_name,
+                                    srcInput->i_options, srcInput->ppsz_options,
+                                    srcInput->optflagc, srcInput->i_duration,
+                                    srcInput->i_type );
+        vlc_mutex_unlock( &srcInput->lock );
+
+        if( source->i_children != -1 )
+            target = playlist_NodeCreate( p_playlist, newInput->psz_name, target, 0, newInput );
+        else
+            playlist_NodeAddInput( p_playlist, newInput, target,
+                                   PLAYLIST_APPEND | PLAYLIST_SPREPARSE,
+                                   PLAYLIST_END, pl_Locked );
+    }
+    for( int i = 0; i < source->i_children; i++ )
+        recursiveAppendCopy( p_playlist, source->pp_children[i], target, b_flat );
+}
+
 void PLModel::dropMove( QByteArray& data, PLItem *target, int row )
 {
     QDataStream stream( &data, QIODevice::ReadOnly );
     QList<PLItem*> model_items;
-    QList<int> ids;
     int new_pos = row == -1 ? target->children.size() : row;
     int model_pos = new_pos;
+
     while( !stream.atEnd() )
     {
-        PLItem *item;
-        stream.readRawData( (char*)&item, sizeof(PLItem*) );
-
-        /* better not try to move a node into itself: */
+        int i_id;
+        stream >> i_id;
+        PLItem *item = findById( rootItem, i_id );
+        if( !item ) continue;
+
+        /* Better not try to move a node into itself.
+           Abort the whole operation in that case,
+           because it is ambiguous. */
         PLItem *climber = target;
         while( climber )
         {
-            if( climber == item ) break;
+            if( climber == item ) return;
             climber = climber->parentItem;
         }
-        if( climber ) continue;
 
         if( item->parentItem == target &&
-            target->children.indexOf( item ) < model_pos )
+            target->children.indexOf( item ) < new_pos )
                 model_pos--;
 
-        ids.append( item->i_id );
         model_items.append( item );
+    }
 
+    if( model_items.isEmpty() ) return;
+
+    foreach( PLItem *item, model_items )
         takeItem( item );
-    }
-    int count = ids.size();
-    if( count )
-    {
-        playlist_item_t *pp_items[count];
 
-        PL_LOCK;
-        for( int i = 0; i < count; i++ )
+    playlist_item_t *pp_items[model_items.size()];
+
+    PL_LOCK;
+    int i = 0;
+    foreach( PLItem *item, model_items )
+    {
+        playlist_item_t *p_item = playlist_ItemGetById( p_playlist, item->i_id );
+        if( !p_item )
         {
-            playlist_item_t *p_item = playlist_ItemGetById( p_playlist, ids[i] );
-            if( !p_item )
-            {
-                PL_UNLOCK;
-                return;
-            }
-            pp_items[i] = p_item;
+            PL_UNLOCK;
+            return;
         }
-        playlist_item_t *p_parent =
-            playlist_ItemGetById( p_playlist, target->i_id );
-        playlist_TreeMoveMany( p_playlist, count, pp_items, p_parent,
-            new_pos );
-        PL_UNLOCK;
-
-        insertChildren( target, model_items, model_pos );
+        pp_items[i] = p_item;
+        i++;
     }
+    playlist_item_t *p_parent =
+        playlist_ItemGetById( p_playlist, target->i_id );
+    playlist_TreeMoveMany( p_playlist, i, pp_items, p_parent,
+        new_pos );
+    PL_UNLOCK;
+
+    insertChildren( target, model_items, model_pos );
 }
 
 /* remove item with its id */

diff --git a/modules/gui/qt4/components/playlist/playlist_model.hpp b/modules/gui/qt4/components/playlist/playlist_model.hpp
index 4b31f1f29e8f6d2363da55836a0dd6a16866773c..4db07d6b51bef83e27c63cef5e6cf417219a1cef 100644 (file)
--- a/modules/gui/qt4/components/playlist/playlist_model.hpp
+++ b/modules/gui/qt4/components/playlist/playlist_model.hpp
@@ -46,12 +46,14 @@
 #include <QAction>
 
 class PLItem;
+class PLSelector;
 
 class PLModel : public QAbstractItemModel
 {
     Q_OBJECT
 
 friend class PLItem;
+friend class PLSelector;
 
 public:
     enum {
@@ -139,6 +141,7 @@ private:
     void updateChildren( playlist_item_t *, PLItem * );
 
     /* Deep actions (affect core playlist) */
+    static void recursiveAppendCopy( playlist_t *, playlist_item_t *, playlist_item_t *, bool );
     void dropAppendCopy( QByteArray& data, PLItem *target );
     void dropMove( QByteArray& data, PLItem *target, int new_pos );
 

diff --git a/modules/gui/qt4/components/playlist/selector.cpp b/modules/gui/qt4/components/playlist/selector.cpp
index 2e616800a364be627156cac8cda0e0f495dc4d09..9c5256508e0062e66e7d5642e8f0f3b453922c93 100644 (file)
--- a/modules/gui/qt4/components/playlist/selector.cpp
+++ b/modules/gui/qt4/components/playlist/selector.cpp
@@ -29,7 +29,7 @@
 #include <assert.h>
 
 #include "components/playlist/selector.hpp"
-#include "playlist_item.hpp"
+#include "playlist_model.hpp"
 #include "qt4.hpp"
 #include "../../dialogs_provider.hpp"
 #include "playlist.hpp"
@@ -357,15 +357,15 @@ bool PLSelector::dropMimeData ( QTreeWidgetItem * parent, int index,
         playlist_Lock( THEPL );
         while( !stream.atEnd() )
         {
-            PLItem *item;
-            stream.readRawData( (char*)&item, sizeof(PLItem*) );
-            input_item_t *pl_input =item->inputItem();
-            playlist_AddExt ( THEPL,
-                pl_input->psz_uri, pl_input->psz_name,
-                PLAYLIST_APPEND | PLAYLIST_SPREPARSE, PLAYLIST_END,
-                pl_input->i_duration,
-                pl_input->i_options, pl_input->ppsz_options, pl_input->optflagc,
-                to_pl, true );
+            int i_id;
+            stream >> i_id;
+
+            playlist_item_t *p_item = playlist_ItemGetById( THEPL, i_id );
+            if( !p_item ) continue;
+
+            PLModel::recursiveAppendCopy( THEPL, p_item,
+                                          to_pl ? THEPL->p_playing : THEPL->p_media_library,
+                                          to_pl && !var_InheritBool( p_intf, "playlist-tree" ) );
         }
         playlist_Unlock( THEPL );
     }