[kde-doc-english] [amarok] /: Playlist sort widget: reimplement Shuffle "sort" as an action.
Matěj Laitl
matej at laitl.cz
Fri May 31 12:23:54 UTC 2013
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
More information about the kde-doc-english
mailing list