Git commit 5659f0f8ad73a2894ebe8d5da3f7925e75ce6901 by Mat?j Laitl, on behalf of Konrad Zemek. Committed on 31/05/2013 at 14:21. Pushed by laitl into branch 'master'.
Playlist sort widget: reimplement Shuffle "sort" as an action. Remove now-unnecessary Shuffle handlers in sort-related functions. Additionally, PlaylistController::moveRows() has been modified to help with big move actions (validation complexity reduced). Playlist::BreadcrumbItemMenu introduced to deduplicate logic between Playlist::BreadcrumbItem and Playlist::BreadcrumbAddMenuButton. GUI: In the playlist sort widget, the Shuffle menu entry is now separated from other entries. Activating the entry no longer results in a "Shuffle" sort level being added. BUG: 320129 FIXED-IN: 2.8 REVIEW: 110658 CCMAIL: amarok-promo at kde.org M +1 -0 ChangeLog M +21 -0 src/playlist/PlaylistActions.cpp M +5 -0 src/playlist/PlaylistActions.h M +60 -64 src/playlist/PlaylistBreadcrumbItem.cpp M +56 -26 src/playlist/PlaylistBreadcrumbItem.h M +32 -47 src/playlist/PlaylistBreadcrumbItemSortButton.cpp M +1 -2 src/playlist/PlaylistBreadcrumbItemSortButton.h M +2 -27 src/playlist/PlaylistBreadcrumbLevel.cpp M +0 -1 src/playlist/PlaylistBreadcrumbLevel.h M +9 -7 src/playlist/PlaylistController.cpp M +12 -2 src/playlist/PlaylistController.h M +0 -4 src/playlist/PlaylistDefines.h M +8 -4 src/playlist/PlaylistModel.cpp M +20 -15 src/playlist/PlaylistSortWidget.cpp M +6 -1 src/playlist/PlaylistSortWidget.h M +4 -69 src/playlist/proxymodels/SortAlgorithms.cpp M +0 -22 src/playlist/proxymodels/SortAlgorithms.h M +0 -2 src/playlist/proxymodels/SortScheme.cpp M +24 -24 tests/playlist/TestPlaylistModels.cpp M +3 -1 tests/playlist/TestPlaylistModels.h http://commits.kde.org/amarok/5659f0f8ad73a2894ebe8d5da3f7925e75ce6901 diff --git a/ChangeLog b/ChangeLog index 9dd0c3d..d71b418 100644 --- a/ChangeLog +++ b/ChangeLog @@ -61,6 +61,7 @@ VERSION 2.8-Beta 1 not to fool users. BUGFIXES: + * Fix tracks not able to be dragged around when playlist is shuffled. (BR 320129) * Prevent Last.fm scrobbles not being submitted until restart due to change in liblastfm 1.0.7. (BR 320219) * Resume Playback on Start will correctly restore paused state, instead diff --git a/src/playlist/PlaylistActions.cpp b/src/playlist/PlaylistActions.cpp index 1decce7..a303f31 100644 --- a/src/playlist/PlaylistActions.cpp +++ b/src/playlist/PlaylistActions.cpp @@ -349,6 +349,27 @@ Playlist::Actions::repopulateDynamicPlaylist() } } +void +Playlist::Actions::shuffle() +{ + QList<int> fromRows, toRows; + + { + const int rowCount = The::playlist()->qaim()->rowCount(); + fromRows.reserve( rowCount ); + + QMultiMap<int, int> shuffleToRows; + for( int row = 0; row < rowCount; ++row ) + { + fromRows.append( row ); + shuffleToRows.insert( qrand(), row ); + } + toRows = shuffleToRows.values(); + } + + The::playlistController()->reorderRows( fromRows, toRows ); +} + int Playlist::Actions::queuePosition( quint64 id ) { diff --git a/src/playlist/PlaylistActions.h b/src/playlist/PlaylistActions.h index dee8793..96df16d 100644 --- a/src/playlist/PlaylistActions.h +++ b/src/playlist/PlaylistActions.h @@ -123,6 +123,11 @@ public slots: void repopulateDynamicPlaylist(); /** + * Shuffles tracks (that are visible in the top model) at the bottom model level + */ + void shuffle(); + + /** * Adds a list of top playlist model rows to the queue. */ void queue( const QList<int> &rows ); diff --git a/src/playlist/PlaylistBreadcrumbItem.cpp b/src/playlist/PlaylistBreadcrumbItem.cpp index 59fc6d2..0d93026 100644 --- a/src/playlist/PlaylistBreadcrumbItem.cpp +++ b/src/playlist/PlaylistBreadcrumbItem.cpp @@ -17,56 +17,76 @@ #include "PlaylistBreadcrumbItem.h" -#include "PlaylistDefines.h" #include "PlaylistSortWidget.h" #include <KIcon> #include <KLocale> -#include <QMenu> - namespace Playlist { +BreadcrumbItemMenu::BreadcrumbItemMenu( Column currentColumn, QWidget *parent ) + : QMenu( parent ) +{ + for( Column col = Column( 0 ); col != NUM_COLUMNS; col = Column( col + 1 ) ) + { + if( !isSortableColumn( col ) || currentColumn == col ) + continue; + + QAction *action = addAction( KIcon( iconName( col ) ), + QString( columnName( col ) ) ); + action->setData( internalColumnName( col ) ); + } + + addSeparator(); + QAction *shuffleAction = addAction( KIcon( "media-playlist-shuffle" ), + QString( i18n( "Shuffle" ) ) ); + shuffleAction->setData( QString( "Shuffle" ) ); + + connect( this, SIGNAL(triggered(QAction*)), this, SLOT(actionTriggered(QAction*)) ); +} + +BreadcrumbItemMenu::~BreadcrumbItemMenu() +{} + +void +BreadcrumbItemMenu::actionTriggered( QAction *action ) +{ + const QString actionName( action->data().toString() ); + if( actionName == "Shuffle" ) + emit shuffleActionClicked(); + else + emit actionClicked( actionName ); +} + +/////// BreadcrumbItem methods begin here + BreadcrumbItem::BreadcrumbItem( BreadcrumbLevel *level, QWidget *parent ) : KHBox( parent ) + , m_name( level->name() ) + , m_prettyName( level->prettyName() ) { - m_name = level->name(); - m_prettyName = level->prettyName(); - - //Let's set up the "siblings" button first... + // Let's set up the "siblings" button first... m_menuButton = new BreadcrumbItemMenuButton( this ); - QMenu *menu = new QMenu( this ); - QStringList usedBreadcrumbLevels = qobject_cast< SortWidget * >( parent )->levels(); + m_menu = new BreadcrumbItemMenu( columnForName( m_name ), this ); - QMap< QString, QPair< KIcon, QString > > siblings = level->siblings(); - for( QMap< QString, QPair< KIcon, QString > >::const_iterator - i = siblings.constBegin(), end = siblings.constEnd(); i != end; ++i ) - { - QAction *action = menu->addAction( i.value().first, i.value().second ); - action->setData( i.key() ); - if( usedBreadcrumbLevels.contains( i.key() ) ) + // Disable used levels + QStringList usedBreadcrumbLevels = qobject_cast< SortWidget * >( parent )->levels(); + foreach( QAction *action, m_menu->actions() ) + if( usedBreadcrumbLevels.contains( action->data().toString() ) ) action->setEnabled( false ); - } - m_menuButton->setMenu( menu ); - const int offset = 6; - menu->setContentsMargins( offset, 1, 1, 2 ); - connect( menu, SIGNAL(triggered(QAction*)), this, SLOT(siblingTriggered(QAction*)) ); - //And then the main breadcrumb button... - bool noArrow = false; - if( m_name == "Shuffle" ) - noArrow = true; - m_mainButton = new BreadcrumbItemSortButton( level->icon(), level->prettyName(), noArrow, this ); + m_menuButton->setMenu( m_menu ); + m_menu->setContentsMargins( /*offset*/ 6, 1, 1, 2 ); + // And then the main breadcrumb button... + m_mainButton = new BreadcrumbItemSortButton( level->icon(), level->prettyName(), this ); connect( m_mainButton, SIGNAL(clicked()), this, SIGNAL(clicked()) ); connect( m_mainButton, SIGNAL(arrowToggled(Qt::SortOrder)), this, SIGNAL(orderInverted()) ); - connect( m_mainButton, SIGNAL(sizePolicyChanged()), this, SLOT(updateSizePolicy()) ); - menu->hide(); + m_menu->hide(); updateSizePolicy(); - } BreadcrumbItem::~BreadcrumbItem() @@ -96,13 +116,12 @@ BreadcrumbItem::updateSizePolicy() setSizePolicy( m_mainButton->sizePolicy() ); } -void -BreadcrumbItem::siblingTriggered( QAction * action ) +const BreadcrumbItemMenu* +BreadcrumbItem::menu() { - emit siblingClicked( action ); + return m_menu; } - /////// BreadcrumbAddMenuButton methods begin here BreadcrumbAddMenuButton::BreadcrumbAddMenuButton( QWidget *parent ) @@ -110,50 +129,27 @@ BreadcrumbAddMenuButton::BreadcrumbAddMenuButton( QWidget *parent ) { setToolTip( i18n( "Add a sorting level to the playlist." ) ); - m_menu = new QMenu( this ); - for( int i = 0; i < NUM_COLUMNS; ++i ) //might be faster if it used a const_iterator - { - Playlist::Column col = static_cast<Playlist::Column>( i ); - - if( !isSortableColumn( col ) ) - continue; - QAction *action = m_menu->addAction( KIcon( iconName( col ) ), QString( columnName( col ) ) ); - action->setData( internalColumnName( col ) ); - //FIXME: this menu should have the same margins as other Playlist::Breadcrumb and - // BrowserBreadcrumb menus. - } - QAction *action = m_menu->addAction( KIcon( "media-playlist-shuffle" ), QString( i18n( "Shuffle" ) ) ); - action->setData( "Shuffle" ); - - connect( m_menu, SIGNAL(triggered(QAction*)), this, SLOT(siblingTriggered(QAction*)) ); - + //FIXME: the menu should have the same margins as other Playlist::Breadcrumb and + // BrowserBreadcrumb menus. + m_menu = new BreadcrumbItemMenu( PlaceHolder, this ); setMenu( m_menu ); } BreadcrumbAddMenuButton::~BreadcrumbAddMenuButton() {} -void -BreadcrumbAddMenuButton::siblingTriggered( QAction *action ) +const BreadcrumbItemMenu* +BreadcrumbAddMenuButton::menu() { - emit siblingClicked( action->data().toString() ); + return m_menu; } void BreadcrumbAddMenuButton::updateMenu( const QStringList &usedBreadcrumbLevels ) { - if( usedBreadcrumbLevels.contains( "Shuffle" ) ) - hide(); - else - show(); + // Enable unused, disable used levels foreach( QAction *action, m_menu->actions() ) - { - if( usedBreadcrumbLevels.contains( action->data().toString() ) ) - action->setEnabled( false ); - else - action->setEnabled( true ); - } - + action->setEnabled( !usedBreadcrumbLevels.contains( action->data().toString() ) ); } } //namespace Playlist diff --git a/src/playlist/PlaylistBreadcrumbItem.h b/src/playlist/PlaylistBreadcrumbItem.h index d6ff243..a86e8e7 100644 --- a/src/playlist/PlaylistBreadcrumbItem.h +++ b/src/playlist/PlaylistBreadcrumbItem.h @@ -20,15 +20,58 @@ #include "PlaylistBreadcrumbItemSortButton.h" #include "PlaylistBreadcrumbLevel.h" +#include "PlaylistDefines.h" #include <KHBox> +#include <QMenu> #include <QStringList> namespace Playlist { /** + * A menu that is filled with elements consisting of sortable columns + * and shuffle action. + */ +class BreadcrumbItemMenu : public QMenu +{ + Q_OBJECT + +public: + /** + * Constructor. + * @param currentColumn The column corresponding to the current sort level + * @param parent The parent QWidget + */ + explicit BreadcrumbItemMenu( Column currentColumn, QWidget *parent = 0 ); + + /** + * Destructor. + */ + virtual ~BreadcrumbItemMenu(); + +signals: + /** + * Emitted when a non-Shuffle item is triggered from the menu. + * @param action the action in the menu that has been triggered. + */ + void actionClicked( QString internalColName ); + + /** + * Emmited when the Shuffle item is triggered from the menu. + */ + void shuffleActionClicked(); + +private slots: + /** + * Handles the selection of an item from the menu. + * @param action the action in the menu that has been triggered. + */ + void actionTriggered( QAction *action ); +}; + +/** * A single item that represents a level of a general-purpose breadcrumb ribbon. * @author T?o Mrnjavac <teo at kde.org> */ @@ -72,13 +115,13 @@ public: */ void invertOrder(); -signals: /** - * Emitted when a sibling of this item has been chosen from the siblings menu. - * @param action the action in the menu that has been triggered. + * Menu accessor for the purpose of connecting to menu's signals. + * @return a pointer to the constant menu object. */ - void siblingClicked( QAction *action ); + const BreadcrumbItemMenu *menu(); +signals: /** * Emitted when the item has been clicked. */ @@ -93,17 +136,11 @@ protected slots: void updateSizePolicy(); private: + BreadcrumbItemMenu *m_menu; BreadcrumbItemMenuButton *m_menuButton; BreadcrumbItemSortButton *m_mainButton; QString m_name; QString m_prettyName; - -private slots: - /** - * Handles the selection of a sibling from the siblings menu. - * @param action the action in the menu that has been triggered. - */ - void siblingTriggered( QAction *action ); }; /** @@ -113,6 +150,7 @@ private slots: class BreadcrumbAddMenuButton : public BreadcrumbItemMenuButton { Q_OBJECT + public: /** * Constructor. @@ -125,27 +163,19 @@ public: virtual ~BreadcrumbAddMenuButton(); /** - * Updates the menu when the breadcrumb path changes. - * @param usedBreadcrumbLevels the levels used in the path. - */ - void updateMenu( const QStringList &usedBreadcrumbLevels ); - -signals: - /** - * Emitted when a sibling is triggered from the menu. - * @param sibling the name of the sibling. + * Menu accessor for the purpose of connecting to menu's signals. + * @return a pointer to the constant menu object. */ - void siblingClicked( QString sibling ); + const BreadcrumbItemMenu *menu(); -private slots: /** - * Handles the selection of an item from the menu. - * @param action the action in the menu that has been triggered. + * Updates the menu when the breadcrumb path changes. + * @param usedBreadcrumbLevels the levels used in the path. */ - void siblingTriggered( QAction *action ); + void updateMenu( const QStringList &usedBreadcrumbLevels ); private: - QMenu *m_menu; + BreadcrumbItemMenu *m_menu; }; } //namespace Playlist diff --git a/src/playlist/PlaylistBreadcrumbItemSortButton.cpp b/src/playlist/PlaylistBreadcrumbItemSortButton.cpp index 8bf861c..9b2f882 100644 --- a/src/playlist/PlaylistBreadcrumbItemSortButton.cpp +++ b/src/playlist/PlaylistBreadcrumbItemSortButton.cpp @@ -28,7 +28,6 @@ namespace Playlist BreadcrumbItemSortButton::BreadcrumbItemSortButton( QWidget *parent ) : BreadcrumbItemButton( parent ) , m_order( Qt::AscendingOrder ) - , m_noArrows( false ) , m_arrowPressed( false ) , m_arrowHovered( false ) , m_arrowWidth( 11 ) @@ -37,10 +36,9 @@ BreadcrumbItemSortButton::BreadcrumbItemSortButton( QWidget *parent ) init(); } -BreadcrumbItemSortButton::BreadcrumbItemSortButton( const QIcon &icon, const QString &text, bool noArrows, QWidget *parent ) +BreadcrumbItemSortButton::BreadcrumbItemSortButton(const QIcon &icon, const QString &text, QWidget *parent ) : BreadcrumbItemButton( icon, text, parent ) , m_order( Qt::AscendingOrder ) - , m_noArrows( noArrows ) , m_arrowPressed( false ) , m_arrowHovered( false ) , m_arrowWidth( 11 ) @@ -63,48 +61,40 @@ BreadcrumbItemSortButton::init() void BreadcrumbItemSortButton::paintEvent( QPaintEvent *event ) { - if( !m_noArrows ) - { - QPainter painter(this); + Q_UNUSED( event ) + QPainter painter(this); - const int buttonHeight = height(); - int buttonWidth = width(); - int preferredWidth = sizeHint().width(); - if (preferredWidth < minimumWidth()) - preferredWidth = minimumWidth(); - if (buttonWidth > preferredWidth) - buttonWidth = preferredWidth; + const int buttonHeight = height(); + const int preferredWidth = qMax( minimumWidth(), sizeHint().width() ); + const int buttonWidth = qMin( preferredWidth, width() ); - int left, top, right, bottom; - getContentsMargins ( &left, &top, &right, &bottom ); - const int padding = 2; + int left, top, right, bottom; + getContentsMargins ( &left, &top, &right, &bottom ); + const int padding = 2; - const int arrowLeft = buttonWidth - m_arrowWidth - padding; - const int arrowTop = ( ( buttonHeight - top - bottom) - m_arrowHeight )/2; - m_arrowRect = QRect( arrowLeft, arrowTop, m_arrowWidth, m_arrowHeight ); + const int arrowLeft = buttonWidth - m_arrowWidth - padding; + const int arrowTop = ( ( buttonHeight - top - bottom) - m_arrowHeight )/2; + m_arrowRect = QRect( arrowLeft, arrowTop, m_arrowWidth, m_arrowHeight ); - drawHoverBackground( &painter ); + drawHoverBackground( &painter ); - const QColor fgColor = foregroundColor(); - QStyleOption option; - option.initFrom(this); - option.rect = m_arrowRect; - option.palette = palette(); - option.palette.setColor(QPalette::Text, fgColor); - option.palette.setColor(QPalette::WindowText, fgColor); - option.palette.setColor(QPalette::ButtonText, fgColor); - - if( m_order == Qt::DescendingOrder ) - style()->drawPrimitive( QStyle::PE_IndicatorArrowDown, &option, &painter, this ); - else - style()->drawPrimitive( QStyle::PE_IndicatorArrowUp, &option, &painter, this ); - - QRect newPaintRect( 0, 0, buttonWidth - m_arrowWidth - padding, buttonHeight ); - QPaintEvent newEvent( newPaintRect ); - BreadcrumbItemButton::paintEvent( &newEvent ); - } + const QColor fgColor = foregroundColor(); + QStyleOption option; + option.initFrom(this); + option.rect = m_arrowRect; + option.palette = palette(); + option.palette.setColor(QPalette::Text, fgColor); + option.palette.setColor(QPalette::WindowText, fgColor); + option.palette.setColor(QPalette::ButtonText, fgColor); + + if( m_order == Qt::DescendingOrder ) + style()->drawPrimitive( QStyle::PE_IndicatorArrowDown, &option, &painter, this ); else - BreadcrumbItemButton::paintEvent( event ); + style()->drawPrimitive( QStyle::PE_IndicatorArrowUp, &option, &painter, this ); + + QRect newPaintRect( 0, 0, buttonWidth - m_arrowWidth - padding, buttonHeight ); + QPaintEvent newEvent( newPaintRect ); + BreadcrumbItemButton::paintEvent( &newEvent ); } void @@ -172,14 +162,9 @@ BreadcrumbItemSortButton::invertOrder() QSize BreadcrumbItemSortButton::sizeHint() const { - if( !m_noArrows ) - { - QSize size = BreadcrumbItemButton::sizeHint(); - size.setWidth( size.width() + m_arrowWidth ); - return size; - } - else - return BreadcrumbItemButton::sizeHint(); + QSize size = BreadcrumbItemButton::sizeHint(); + size.setWidth( size.width() + m_arrowWidth ); + return size; } Qt::SortOrder diff --git a/src/playlist/PlaylistBreadcrumbItemSortButton.h b/src/playlist/PlaylistBreadcrumbItemSortButton.h index c141597..a189116 100644 --- a/src/playlist/PlaylistBreadcrumbItemSortButton.h +++ b/src/playlist/PlaylistBreadcrumbItemSortButton.h @@ -47,7 +47,7 @@ public: * otherwise false. * @param parent the parent QWidget. */ - BreadcrumbItemSortButton( const QIcon &icon, const QString &text, bool noArrows, QWidget *parent ); + BreadcrumbItemSortButton( const QIcon &icon, const QString &text, QWidget *parent ); /** * Destructor. @@ -116,7 +116,6 @@ private: */ void init(); Qt::SortOrder m_order; - bool m_noArrows; QRect m_arrowRect; //!< the QRect that contains the order inversion arrow primitive. QPoint m_pressedPos; //!< the position of the last mousePressEvent, for handling clicks. bool m_arrowPressed; diff --git a/src/playlist/PlaylistBreadcrumbLevel.cpp b/src/playlist/PlaylistBreadcrumbLevel.cpp index 185ebfb..02ba8e6 100644 --- a/src/playlist/PlaylistBreadcrumbLevel.cpp +++ b/src/playlist/PlaylistBreadcrumbLevel.cpp @@ -28,27 +28,8 @@ BreadcrumbLevel::BreadcrumbLevel( QString internalColumnName ) { Column col = columnForName( internalColumnName ); - if( col == Shuffle ) - { - m_icon = KIcon( "media-playlist-shuffle" ); - m_prettyName = i18n( "Shuffle" ); - } - else - { - m_icon = KIcon( iconName( col ) ); - m_prettyName = columnName( col ); - } - - for( int i = 0; i < NUM_COLUMNS; ++i ) //might be faster if it used a const_iterator - { - Column currentCol = static_cast<Column>(i); - if( !isSortableColumn( currentCol ) || currentCol == col ) - continue; - m_siblings.insert( Playlist::internalColumnName( currentCol ), - QPair< KIcon, QString>( KIcon( iconName( currentCol ) ), columnName( currentCol ) ) ); - } - if( col != Shuffle ) - m_siblings.insert( "Shuffle", QPair< KIcon, QString>( KIcon( "media-playlist-shuffle" ), i18n("Shuffle" ) ) ); + m_icon = KIcon( iconName( col ) ); + m_prettyName = columnName( col ); } BreadcrumbLevel::~BreadcrumbLevel() @@ -72,10 +53,4 @@ BreadcrumbLevel::icon() return m_icon; } -const QMap< QString, QPair< KIcon, QString > > -BreadcrumbLevel::siblings() -{ - return m_siblings; -} - } //namespace Playlist diff --git a/src/playlist/PlaylistBreadcrumbLevel.h b/src/playlist/PlaylistBreadcrumbLevel.h index d87c88d..0627d43 100644 --- a/src/playlist/PlaylistBreadcrumbLevel.h +++ b/src/playlist/PlaylistBreadcrumbLevel.h @@ -55,7 +55,6 @@ protected: QString m_name; //!< the name of this item. QString m_prettyName; KIcon m_icon; - QMap< QString, QPair< KIcon, QString > > m_siblings; //!< internalColumnName, icon, prettyName }; } //namespace Playlist diff --git a/src/playlist/PlaylistController.cpp b/src/playlist/PlaylistController.cpp index df293a9..f76e2a7 100644 --- a/src/playlist/PlaylistController.cpp +++ b/src/playlist/PlaylistController.cpp @@ -392,7 +392,7 @@ Controller::moveRow( int from, int to ) } } - moveRows( source, target ); + reorderRows( source, target ); } int @@ -443,22 +443,24 @@ Controller::moveRows( QList<int>& from, int to ) source.insert( ( to - min ), *f_iter ); } - moveRows( source, target ); + reorderRows( source, target ); return to; } void -Controller::moveRows( QList<int>& from, QList<int>& to ) +Controller::reorderRows( const QList<int> &from, const QList<int> &to ) { DEBUG_BLOCK if( from.size() != to.size() ) return; - // validity check - foreach( int i, from ) + // validity check: each item should appear exactly once in both lists { - if(( from.count( i ) != 1 ) || ( to.count( i ) != 1 ) ) + QSet<int> fromItems( from.toSet() ); + QSet<int> toItems( to.toSet() ); + + if( fromItems.size() != from.size() || toItems.size() != to.size() || fromItems != toItems ) { error() << "row move lists malformed:"; error() << from; @@ -470,7 +472,7 @@ Controller::moveRows( QList<int>& from, QList<int>& to ) MoveCmdList bottomModelCmds; for( int i = 0; i < from.size(); i++ ) { - debug() << "moving rows:" << from.at( i ) << to.at( i ); + debug() << "moving rows:" << from.at( i ) << "->" << to.at( i ); if( ( from.at( i ) >= 0 ) && ( from.at( i ) < m_topModel->qaim()->rowCount() ) ) if( from.at( i ) != to.at( i ) ) bottomModelCmds.append( MoveCmd( m_topModel->rowToBottomModel( from.at( i ) ), m_topModel->rowToBottomModel( to.at( i ) ) ) ); diff --git a/src/playlist/PlaylistController.h b/src/playlist/PlaylistController.h index 5047e47..db71e2b 100644 --- a/src/playlist/PlaylistController.h +++ b/src/playlist/PlaylistController.h @@ -177,8 +177,18 @@ public slots: * @param to the target row where the tracks should be moved. * @return the first row where the tracks ended up in the new list. */ - int moveRows( QList<int>& topModelFrom, int topModelTo ); - void moveRows( QList<int>& topModelFrom, QList<int>& topModelTo ); + int moveRows( QList<int>& topModelFrom, int topModelTo ); + + /** + * Reorders tracks in the playlist. For each i, track at position + * topModelFrom[i] is moved to the position topModelTo[i]. Note that when track + * on position A is moved to the position B, the track from position B needs to + * be moved as well. As a consequence, every track position appearing + * in topModelFrom needs to appear in topModelTo. + * @param topModelFrom the list containing positions of tracks to be moved + * @param topModelTo the list containing positions the tracks should be moved to + */ + void reorderRows( const QList<int> &topModelFrom, const QList<int> &topModelTo ); void undo(); void redo(); diff --git a/src/playlist/PlaylistDefines.h b/src/playlist/PlaylistDefines.h index 3e10b55..c03edda 100644 --- a/src/playlist/PlaylistDefines.h +++ b/src/playlist/PlaylistDefines.h @@ -30,7 +30,6 @@ namespace Playlist */ enum Column { - Shuffle = -1, // TODO: having shuffle at -1 is causing a lot of effort. PlaceHolder = 0, Album, AlbumArtist, @@ -117,9 +116,6 @@ class PlaylistColumnInfos inline Column columnForName( const QString &internalName ) { - if( internalName == QLatin1String( "Shuffle" ) ) - return Shuffle; - return static_cast<Column>(Playlist::PlaylistColumnInfos::internalNames(). indexOf( internalName )); } diff --git a/src/playlist/PlaylistModel.cpp b/src/playlist/PlaylistModel.cpp index a562bdb..e93bd06 100644 --- a/src/playlist/PlaylistModel.cpp +++ b/src/playlist/PlaylistModel.cpp @@ -1147,14 +1147,18 @@ Playlist::Model::moveTracksCommand( const MoveCmdList& cmds, bool reverse ) if ( cmds.size() < 1 ) return; - int min = m_items.size() + cmds.size(); - int max = 0; + int min = INT_MAX; + int max = INT_MIN; foreach( const MoveCmd &rc, cmds ) { min = qMin( min, rc.first ); - min = qMin( min, rc.second ); max = qMax( max, rc.first ); - max = qMax( max, rc.second ); + } + + if( min < 0 || max >= m_items.size() ) + { + error() << "Wrong row numbers given"; + return; } int newActiveRow = m_activeRow; diff --git a/src/playlist/PlaylistSortWidget.cpp b/src/playlist/PlaylistSortWidget.cpp index 62f760c..384e4a0 100644 --- a/src/playlist/PlaylistSortWidget.cpp +++ b/src/playlist/PlaylistSortWidget.cpp @@ -17,6 +17,7 @@ #include "PlaylistSortWidget.h" #include "core/support/Debug.h" +#include "PlaylistActions.h" #include "PlaylistModelStack.h" #include "proxymodels/SortScheme.h" @@ -57,8 +58,8 @@ SortWidget::SortWidget( QWidget *parent ) m_urlButton = new BreadcrumbUrlMenuButton( "playlist", this ); m_layout->addWidget( m_urlButton ); - connect( m_addButton, SIGNAL(siblingClicked(QString)), this, SLOT(addLevel(QString)) ); - + connect( m_addButton->menu(), SIGNAL(actionClicked(QString)), this, SLOT(addLevel(QString)) ); + connect( m_addButton->menu(), SIGNAL(shuffleActionClicked()), The::playlistActions(), SLOT(shuffle()) ); QString sortPath = Amarok::config( "Playlist Sorting" ).readEntry( "SortPath", QString() ); if( !sortPath.isEmpty() ) @@ -66,11 +67,6 @@ SortWidget::SortWidget( QWidget *parent ) QStringList levels = sortPath.split( '-' ); foreach( const QString &level, levels ) { - if( level == QString( "Shuffle" ) || level == QString( "Random" ) ) //we keep "Random" for compatibility - { - addLevel( QString( "Shuffle" ) ); - break; - } QStringList levelParts = level.split( '_' ); if( levelParts.count() > 2 ) warning() << "Playlist sorting load error: Invalid sort level " << level; @@ -94,7 +90,8 @@ SortWidget::addLevel( QString internalColumnName, Qt::SortOrder sortOrder ) //p BreadcrumbItem *item = new BreadcrumbItem( bLevel, this ); m_ribbon->addWidget( item ); connect( item, SIGNAL(clicked()), this, SLOT(onItemClicked()) ); - connect( item, SIGNAL(siblingClicked(QAction*)), this, SLOT(onItemSiblingClicked(QAction*)) ); + connect( item->menu(), SIGNAL(actionClicked(QString)), this, SLOT(onItemSiblingClicked(QString)) ); + connect( item->menu(), SIGNAL(shuffleActionClicked()), this, SLOT(onShuffleSiblingClicked()) ); connect( item, SIGNAL(orderInverted()), this, SLOT(updateSortScheme()) ); if( sortOrder != item->sortOrder() ) item->invertOrder(); @@ -128,16 +125,24 @@ SortWidget::levels() const void SortWidget::onItemClicked() { - const int level = m_ribbon->indexOf( qobject_cast< QWidget * >( sender() ) ); + const int level = m_ribbon->indexOf( qobject_cast< QWidget * >( sender()->parent() ) ); trimToLevel( level ); } void -SortWidget::onItemSiblingClicked( QAction *action ) +SortWidget::onItemSiblingClicked( QString internalColumnName ) +{ + const int level = m_ribbon->indexOf( qobject_cast< QWidget * >( sender()->parent() ) ); + trimToLevel( level - 1 ); + addLevel( internalColumnName ); +} + +void +SortWidget::onShuffleSiblingClicked() { - const int level = m_ribbon->indexOf( qobject_cast< QWidget * >( sender() ) ); - trimToLevel( level -1 ); - addLevel( action->data().toString() ); + const int level = m_ribbon->indexOf( qobject_cast< QWidget * >( sender()->parent() ) ); + trimToLevel( level - 1 ); + The::playlistActions()->shuffle(); } void @@ -165,7 +170,7 @@ SortWidget::sortPath() const { QString name( qobject_cast< BreadcrumbItem * >( m_ribbon->itemAt( i )->widget() )->name() ); Qt::SortOrder sortOrder = qobject_cast< BreadcrumbItem * >( m_ribbon->itemAt( i )->widget() )->sortOrder(); - QString level = ( name == "Shuffle" ) ? name : ( name + "_" + ( sortOrder ? "des" : "asc" ) ); + QString level = name + "_" + ( sortOrder ? "des" : "asc" ); path.append( ( i == m_ribbon->count() - 1 ) ? level : ( level + '-' ) ); } return path; @@ -180,7 +185,7 @@ SortWidget::prettySortPath() const QString name( qobject_cast< BreadcrumbItem * >( m_ribbon->itemAt( i )->widget() )->name() ); QString prettyName( qobject_cast< BreadcrumbItem * >( m_ribbon->itemAt( i )->widget() )->prettyName() ); Qt::SortOrder sortOrder = qobject_cast< BreadcrumbItem * >( m_ribbon->itemAt( i )->widget() )->sortOrder(); - QString prettyLevel = ( name == "Shuffle" ) ? prettyName : ( prettyName + ( sortOrder ? "?" : "?" ) ); + QString prettyLevel = prettyName + ( sortOrder ? "?" : "?" ); prettyPath.append( ( i == m_ribbon->count() - 1 ) ? prettyLevel : ( prettyLevel + " > " ) ); //TODO: see how this behaves on RTL systems } diff --git a/src/playlist/PlaylistSortWidget.h b/src/playlist/PlaylistSortWidget.h index 71aefe5..7d117b3 100644 --- a/src/playlist/PlaylistSortWidget.h +++ b/src/playlist/PlaylistSortWidget.h @@ -97,7 +97,12 @@ private slots: * Handles the rearrangement of the breadcrumb path when a sibling of an item is clicked. * @param action the action in the menu. */ - void onItemSiblingClicked( QAction *action ); + void onItemSiblingClicked( QString internalColumnName ); + + /** + * Handles the rearrangement of the breadcrumb path when a Shuffle action is clicked. + */ + void onShuffleSiblingClicked(); }; } //namespace Playlist diff --git a/src/playlist/proxymodels/SortAlgorithms.cpp b/src/playlist/proxymodels/SortAlgorithms.cpp index 47b468c..5265825 100644 --- a/src/playlist/proxymodels/SortAlgorithms.cpp +++ b/src/playlist/proxymodels/SortAlgorithms.cpp @@ -52,17 +52,6 @@ multilevelLessThan::operator()( const QAbstractItemModel* sourceModel, const bool inverted = ( level.order() == Qt::DescendingOrder ); const Playlist::Column currentCategory = level.category(); - if( currentCategory == Playlist::Shuffle ) - { - long randomSeqnumA = constantRandomSeqnumForRow( sourceModel, sourceModelRowA ); - long randomSeqnumB = constantRandomSeqnumForRow( sourceModel, sourceModelRowB ); - - // '!=' is the XOR operation; it simply negates the result if 'inverted' - // is true. It isn't necessarry to do it this way, although later on it will - // ease figuring out what's actually being returned. - return ( randomSeqnumA < randomSeqnumB ) != inverted; - } - const QModelIndex indexA = sourceModel->index( sourceModelRowA, currentCategory ); const QModelIndex indexB = sourceModel->index( sourceModelRowB, currentCategory ); @@ -79,6 +68,10 @@ multilevelLessThan::operator()( const QAbstractItemModel* sourceModel, const QDateTime lastPlayedB = trackB->statistics()->lastPlayed(); // The track with higher lastPlayed value was played more recently + // + // '!=' is the XOR operation; it simply negates the result if 'inverted' + // is true. It isn't necessarry to do it this way, although later on it will + // ease figuring out what's actually being returned. if( lastPlayedA != lastPlayedB ) return ( lastPlayedA > lastPlayedB ) != inverted; @@ -176,62 +169,4 @@ multilevelLessThan::compareBySortableName( const KSharedPtr<T> &left, return 0; } -// If the 'qrand()' save+restore ever turns out to be a performance bottleneck: try -// switching to 'jrand48()', which has no common random pool and therefore doesn't have -// to be saved+restored. -// -// I chose qrand() because I'm not sure about 'jrand48()' portability. -typedef uint randomSeedType; // For multilevelLessThan::constantRandomSeqnumForRow() 'qsrand()' - -long -multilevelLessThan::constantRandomSeqnumForRow( const QAbstractItemModel* sourceModel, int sourceModelRow ) const -{ - randomSeedType randomSeedForItem; - unsigned char *randomSeedForItem_bytes = (unsigned char*)( &randomSeedForItem ); - memset( randomSeedForItem_bytes, 0x00, sizeof(randomSeedType) ); - - - // Use the playlist item id as the fixed key for the random generator. - // This has all the properties we need: - // - unique - // - stable - // - // Note that we do *NOT* assume the playlist item ids to be random. That happens - // to be the case in Amarok v2.3, but we work just as well if an item id is e.g. - // a C pointer or a linear number. - // - QModelIndex index = sourceModel->index( sourceModelRow, 0 ); - quint64 id = index.data( UniqueIdRole ).value<quint64>(); - - const unsigned char *key = (const unsigned char *)( &id ); - unsigned int keyLen = sizeof(id); - - - // Don't make any assumptions about the structure of the item key: treat it as bytes. - for ( unsigned int i = 0; i < keyLen; i++ ) - randomSeedForItem_bytes[ i % sizeof(randomSeedType) ] ^= key[ i ]; - - - // Mix in our salt, to get a different sort order from run to run. - const unsigned char *salt = (const unsigned char *)( &m_randomSalt ); - for ( unsigned int i = 0; i < sizeof(m_randomSalt); i++ ) - randomSeedForItem_bytes[ i % sizeof(randomSeedType) ] ^= salt[ i ]; - - - // Generate the fixed sequence number based on the fixed key: - - // 1. Save current non-predictable randomness. - int globalSeed = qrand(); - - // 2. (Ab)use the random generator as a hash function. - qsrand( randomSeedForItem ); // Ensure we get the same random number for a given item every time - long randomSeqnum = qrand(); // qrand() actually returns 'int'; use 'long' to allow switch to 'jrand48()'. - - // 3. Restore non-predictability for the rest of Amarok. - qsrand( globalSeed ); - - - return randomSeqnum; -} - } //namespace Playlist diff --git a/src/playlist/proxymodels/SortAlgorithms.h b/src/playlist/proxymodels/SortAlgorithms.h index 2eba6a7..2900930 100644 --- a/src/playlist/proxymodels/SortAlgorithms.h +++ b/src/playlist/proxymodels/SortAlgorithms.h @@ -57,28 +57,6 @@ struct multilevelLessThan */ bool operator()( const QAbstractItemModel* sourceModel, int sourceModelRowA, int sourceModelRowB ) const; - - protected: - /** - * For random sort, we want to assign a random sequence number to each item in - * the source model. - * - * However, the sequence number must stay constant for any given item. - * The QSortFilterProxyModel sort code can ask us about the same row twice, and - * we need to return consistent answers. - * - * The sequence number must also stay constant across item insert/remove, so the - * row number itself should not be used as a persistent key. - * - * The returned sequence numbers don't need to be contiguous. - * - * The returned sequence numbers don't need to be unique; a few collisions are no - * problem. (fallback tiebreaker will be the source row numbers, as always) - * - * @return a sequence number that is random, but constant for the item at 'sourceModelRow'. - */ - long constantRandomSeqnumForRow( const QAbstractItemModel* sourceModel, int sourceModelRow ) const; - private: template<typename T> int compareBySortableName( const KSharedPtr<T> &left, const KSharedPtr<T> &right ) const; diff --git a/src/playlist/proxymodels/SortScheme.cpp b/src/playlist/proxymodels/SortScheme.cpp index d29f9de..6c925ec 100644 --- a/src/playlist/proxymodels/SortScheme.cpp +++ b/src/playlist/proxymodels/SortScheme.cpp @@ -89,8 +89,6 @@ SortLevel::isFloat() const QString SortLevel::prettyName() const { - if( m_category == -1 ) - return i18n( "Shuffle" ); return columnName( m_category ); } diff --git a/tests/playlist/TestPlaylistModels.cpp b/tests/playlist/TestPlaylistModels.cpp index ec8adb8..8a60c82 100644 --- a/tests/playlist/TestPlaylistModels.cpp +++ b/tests/playlist/TestPlaylistModels.cpp @@ -20,6 +20,7 @@ #include "EngineController.h" #include "playlist/PlaylistActions.h" +#include "playlist/PlaylistController.h" #include "playlist/PlaylistModelStack.h" #include "playlist/PlaylistModel.h" #include "playlist/UndoCommands.h" @@ -45,11 +46,14 @@ TestPlaylistModels::TestPlaylistModels() void TestPlaylistModels::initTestCase() { - //apparently the engine controller is needed somewhere, or we will get a crash... EngineController *controller = new EngineController(); Amarok::Components::setEngineController( controller ); + // Initialize playlistAction before we set the playlist, lest our playlist be overwritten with Art Of Nations + The::playlistActions(); + The::playlistController()->removeRow( 0 ); + //we want to add a few tracks to the playlist so we can test sorting, filtering and so on. So first create a bunch of dummy tracks we can use. Meta::TrackList tracks; @@ -102,32 +106,20 @@ void TestPlaylistModels::initTestCase() // note: no artist, no album! tracks << Meta::TrackPtr( metaMock ); - InsertCmdList insertCmds; - int row = 0; - foreach( Meta::TrackPtr t, tracks ) - { - insertCmds.append( InsertCmd( t, row ) ); - row++; - } + The::playlistController()->insertTracks( 0, tracks ); + + QCOMPARE( The::playlist()->trackAt( 3 )->prettyName(), QString( "Zlick" ) ); +} - //make sure sort mode is reset +void TestPlaylistModels::cleanup() +{ SortScheme scheme = SortScheme(); ModelStack::instance()->sortProxy()->updateSortMap( scheme ); ModelStack::instance()->filterProxy()->clearSearchTerm(); - - Model * model = ModelStack::instance()->bottom(); - model->insertTracksCommand( insertCmds ); - - AbstractModel * topModel = The::playlist(); - - QCOMPARE( topModel->trackAt( 3 )->prettyName(), QString( "Zlick" ) ); } void TestPlaylistModels::testSorting() { - - ModelStack::instance()->filterProxy()->clearSearchTerm(); - //simple sort by title //******************************// @@ -210,11 +202,6 @@ void TestPlaylistModels::testSorting() void TestPlaylistModels::testFiltering() { - - //make sure sort mode is reset - SortScheme scheme = SortScheme(); - ModelStack::instance()->sortProxy()->updateSortMap( scheme ); - ModelStack::instance()->filterProxy()->showOnlyMatches( true ); ModelStack::instance()->filterProxy()->find( "ou" ); ModelStack::instance()->filterProxy()->filterUpdated(); @@ -233,3 +220,16 @@ void TestPlaylistModels::testFiltering() void TestPlaylistModels::testSearching() { } + +void TestPlaylistModels::testShuffling() +{ + Meta::TrackList oldTrackList = The::playlist()->tracks(); + + The::playlistActions()->shuffle(); + + QVERIFY( oldTrackList != The::playlist()->tracks() ); + + The::playlistController()->undo(); + + QCOMPARE( oldTrackList, The::playlist()->tracks() ); +} diff --git a/tests/playlist/TestPlaylistModels.h b/tests/playlist/TestPlaylistModels.h index 3751e6a..a501135 100644 --- a/tests/playlist/TestPlaylistModels.h +++ b/tests/playlist/TestPlaylistModels.h @@ -24,12 +24,14 @@ class TestPlaylistModels : public QObject Q_OBJECT public: TestPlaylistModels(); - + private slots: void initTestCase(); + void cleanup(); void testSorting(); void testFiltering(); void testSearching(); + void testShuffling(); }; #endif // TESTPLAYLISTMODELS_H
