From 04317a878773c29f3b2ec0167cd72887d77ee75b Mon Sep 17 00:00:00 2001<br>From: Jakob Leben <<a href="mailto:jakob.leben@gmail.com">jakob.leben@gmail.com</a>><br>Date: Thu, 30 Jul 2009 20:15:41 +0200<br>Subject: [PATCH 2/2] Qt4 Playlist fix: when moving items keep their original order...<br>
<br>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()).<br><br>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.<br>
<br>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.)<br>
<br>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.<br>---<br> .../gui/qt4/components/playlist/playlist_model.cpp | 89 ++++++++++++-------<br>
1 files changed, 56 insertions(+), 33 deletions(-)<br><br>diff --git a/modules/gui/qt4/components/playlist/playlist_model.cpp b/modules/gui/qt4/components/playlist/playlist_model.cpp<br>index 998ebb2..1226a6f 100644<br>--- a/modules/gui/qt4/components/playlist/playlist_model.cpp<br>
+++ b/modules/gui/qt4/components/playlist/playlist_model.cpp<br>@@ -135,10 +135,17 @@ QMimeData *PLModel::mimeData( const QModelIndexList &indexes ) const<br> QMimeData *mimeData = new QMimeData();<br> QByteArray encodedData;<br>
QDataStream stream( &encodedData, QIODevice::WriteOnly );<br>+ QModelIndexList list;<br> <br> foreach( const QModelIndex &index, indexes ) {<br> if( index.isValid() && index.column() == 0 )<br>
- stream << itemId( index );<br>+ list.append(index);<br>+ }<br>+<br>+ qSort(list);<br>+<br>+ foreach( const QModelIndex &index, list ) {<br>+ stream << itemId( index );<br>
}<br> mimeData->setData( "vlc/playlist-item-id", encodedData );<br> return mimeData;<br>@@ -153,58 +160,74 @@ bool PLModel::dropMimeData( const QMimeData *data, Qt::DropAction action,<br> if( action == Qt::IgnoreAction )<br>
return true;<br> <br>- if( !target.isValid() )<br>- /* We don't want to move on an invalid position */<br>- return true;<br>+ PL_LOCK;<br> <br>- PLItem *targetItem = static_cast<PLItem*>( target.internalPointer() );<br>
+ playlist_item_t *p_parent;<br>+<br>+ if( !parent.isValid())<br>+ {<br>+ if( row > -1)<br>+ {<br>+ // dropped into top node<br>+ p_parent = playlist_ItemGetById( p_playlist, rootItem->i_id );<br>
+ }<br>+ else<br>+ {<br>+ // dropped outside any item<br>+ PL_UNLOCK;<br>+ return true;<br>+ }<br>+ }<br>+ else<br>+ {<br>
+ // dropped into/onto an item (depends on (row = -1) or (row > -1))<br>+ PLItem *parentItem = static_cast<PLItem*>( parent.internalPointer() );<br>+ p_parent = playlist_ItemGetById( p_playlist, parentItem->i_id );<br>
+ }<br>+ if( !p_parent || p_parent->i_children == -1 )<br>+ {<br>+ PL_UNLOCK;<br>+ return false;<br>+ }<br> <br> QByteArray encodedData = data->data( "vlc/playlist-item-id" );<br>
QDataStream stream( &encodedData, QIODevice::ReadOnly );<br> <br>- PLItem *newParentItem;<br>+ /* easiest way to never miss the right index to move to is to<br>+ track the previously moved item */<br>
+ playlist_item_t *p_target = 0;<br>+<br> while( !stream.atEnd() )<br> {<br>- int i;<br>- int srcId;<br>- stream >> srcId;<br>-<br>- PL_LOCK;<br>- playlist_item_t *p_target =<br>
- playlist_ItemGetById( p_playlist, targetItem->i_id );<br>- playlist_item_t *p_src = playlist_ItemGetById( p_playlist, srcId );<br>+ int src_id;<br>+ stream >> src_id;<br>
+ playlist_item_t *p_src = playlist_ItemGetById( p_playlist, src_id );<br> <br>- if( !p_target || !p_src )<br>+ if( !p_src )<br> {<br> PL_UNLOCK;<br> return false;<br>
}<br>- if( p_target->i_children == -1 ) /* A leaf */<br>+ if( !p_target )<br> {<br>- PLItem *parentItem = targetItem->parent();<br>- assert( parentItem );<br>
- playlist_item_t *p_parent =<br>- playlist_ItemGetById( p_playlist, parentItem->i_id );<br>- if( !p_parent )<br>+ if(row == -1)<br> {<br>
- PL_UNLOCK;<br>- return false;<br>+ playlist_TreeMove( p_playlist, p_src, p_parent, 0 );<br>+ }<br>+ else {<br>+ playlist_TreeMove( p_playlist, p_src, p_parent, row );<br>
}<br>- for( i = 0 ; i< p_parent->i_children ; i++ )<br>- if( p_parent->pp_children[i] == p_target ) break;<br>- // Move the item to the element after i<br>
- playlist_TreeMove( p_playlist, p_src, p_parent, i + 1 );<br>- newParentItem = parentItem;<br> }<br> else<br> {<br>- /* \todo: if we drop on a top-level node, use copy instead ? */<br>
- playlist_TreeMove( p_playlist, p_src, p_target, 0 );<br>- i = 0;<br>- newParentItem = targetItem;<br>+ int i;<br>+ for( i = 0 ; i< p_parent->i_children ; i++ )<br>
+ if( p_parent->pp_children[i] == p_target ) break;<br>+ playlist_TreeMove( p_playlist, p_src, p_parent, i + 1 );<br> }<br>- PL_UNLOCK;<br>+ p_target = p_src;<br>
}<br>+ PL_UNLOCK;<br> /*TODO: That's not a good idea to rebuild the playlist */<br> rebuild();<br> }<br>-- <br>1.6.4.msysgit.0<br><br><br>