From: Jakob Leben Date: Mon, 8 Mar 2010 10:56:45 +0000 (+0100) Subject: Qt: Playlist drag and drop corrections X-Git-Tag: 1.1.0-pre1~448 X-Git-Url: https://git.sesse.net/?a=commitdiff_plain;h=4bd42ef61b011eeb2f016354df9767bd75a5e1df;p=vlc 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 6ad708fa7f..aab77281d6 100644 --- 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 45f8d8c00a..6caafcb9a7 100644 --- 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 children; diff --git a/modules/gui/qt4/components/playlist/playlist_model.cpp b/modules/gui/qt4/components/playlist/playlist_model.cpp index 04349cd87e..09785fc5fe 100644 --- 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( i1.internalPointer() ); + PLItem *item2 = static_cast( 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 model_items; - QList 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 4b31f1f29e..4db07d6b51 100644 --- a/modules/gui/qt4/components/playlist/playlist_model.hpp +++ b/modules/gui/qt4/components/playlist/playlist_model.hpp @@ -46,12 +46,14 @@ #include 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 2e616800a3..9c5256508e 100644 --- a/modules/gui/qt4/components/playlist/selector.cpp +++ b/modules/gui/qt4/components/playlist/selector.cpp @@ -29,7 +29,7 @@ #include #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 ); }