From 3b186c698d5af18a49fd9cd2e22cd66ee9c8580e Mon Sep 17 00:00:00 2001 From: Robbert Krebbers Date: Sat, 9 Jun 2012 14:24:15 +0100 Subject: [PATCH] Allow playlist sorting and shuffling to be undone. Fixes issue 654. Also: - When sorting a dynamic playlist, only songs after the currently playing track are sorted. - When moving songs in a dynamic playlist, recolor them if moved across the current track. - When playing a future song in a dynamic playlist, move it to the current location. Fixes issue 1140 --- src/core/player.cpp | 2 +- src/playlist/playlist.cpp | 200 +++++++++++++++----------- src/playlist/playlist.h | 15 +- src/playlist/playlistundocommands.cpp | 33 ++++- src/playlist/playlistundocommands.h | 27 ++++ 5 files changed, 189 insertions(+), 88 deletions(-) diff --git a/src/core/player.cpp b/src/core/player.cpp index cb671bd82..50b42739c 100644 --- a/src/core/player.cpp +++ b/src/core/player.cpp @@ -302,7 +302,7 @@ void Player::PlayAt(int index, Engine::TrackChangeFlags change, bool reshuffle) } if (reshuffle) - app_->playlist_manager()->active()->set_current_row(-1); + app_->playlist_manager()->active()->ReshuffleIndices(); app_->playlist_manager()->active()->set_current_row(index); if (app_->playlist_manager()->active()->current_row() == -1) { diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 1fda7a14e..09e23e1d5 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -298,6 +298,9 @@ QVariant Playlist::data(const QModelIndex& index, int role) const { if (items_[index.row()]->HasCurrentForegroundColor()) { return QBrush(items_[index.row()]->GetCurrentForegroundColor()); } + if (index.row() < dynamic_history_length()) { + return QBrush(kDynamicHistoryColor); + } return QVariant(); case Qt::BackgroundRole: @@ -526,34 +529,29 @@ int Playlist::previous_row() const { return virtual_items_[prev_virtual_index]; } +int Playlist::dynamic_history_length() const { + return dynamic_playlist_ && last_played_item_index_.isValid() + ? last_played_item_index_.row() + 1 + : 0; +} + void Playlist::set_current_row(int i) { - QModelIndex old_current = current_item_index_; + QModelIndex old_current_item_index = current_item_index_; ClearStreamMetadata(); current_item_index_ = QPersistentModelIndex(index(i, 0, QModelIndex())); - + + // if the given item is the first in the queue, remove it from the queue if (current_item_index_.row() == queue_->PeekNext()) { queue_->TakeNext(); } - if (current_item_index_ == old_current) + if (current_item_index_ == old_current_item_index) return; - if (current_item_index_.isValid()) { - last_played_item_index_ = current_item_index_; - current_item_ = items_[current_item_index_.row()]; - Save(); - } else { - current_item_.reset(); - } - - if (old_current.isValid()) { - if (dynamic_playlist_) { - items_[old_current.row()]->SetForegroundColor(kDynamicHistoryPriority, - kDynamicHistoryColor); - } - - emit dataChanged(old_current, old_current.sibling(old_current.row(), ColumnCount-1)); + if (old_current_item_index.isValid()) { + emit dataChanged(old_current_item_index, + old_current_item_index.sibling(old_current_item_index.row(), ColumnCount-1)); } if (current_item_index_.isValid()) { @@ -561,9 +559,9 @@ void Playlist::set_current_row(int i) { } // Update the virtual index - if (i == -1) + if (i == -1) { current_virtual_index_ = -1; - else if (is_shuffled_ && current_virtual_index_ == -1) { + } else if (is_shuffled_ && current_virtual_index_ == -1) { // This is the first thing we're playing so we want to make sure the array // is shuffled ReshuffleIndices(); @@ -572,26 +570,45 @@ void Playlist::set_current_row(int i) { virtual_items_.takeAt(virtual_items_.indexOf(i)); virtual_items_.prepend(i); current_virtual_index_ = 0; - } else if (is_shuffled_) + } else if (is_shuffled_) { current_virtual_index_ = virtual_items_.indexOf(i); - else + } else { current_virtual_index_ = i; + } - if (dynamic_playlist_ && current_item_index_.isValid() && old_current.isValid()) { + // The structure of a dynamic playlist is as follows: + // history - active song - future + // We have to ensure that this invariant is maintained. + if (dynamic_playlist_ && current_item_index_.isValid()) { using smart_playlists::Generator; - // Add more dynamic playlist items - const int count = current_item_index_.row() + dynamic_playlist_->GetDynamicFuture() - items_.count(); + // Move the new item one position ahead of the last item in the history. + MoveItemWithoutUndo(current_item_index_.row(), dynamic_history_length()); + + // Compute the number of new items that have to be inserted. This is not + // necessarily 1 because the user might have added or removed items + // manually. Note that the future excludes the current item. + const int count = dynamic_history_length() + 1 + dynamic_playlist_->GetDynamicFuture() - items_.count(); if (count > 0) { InsertDynamicItems(count); } - // Remove the first item - if (current_item_index_.row() > dynamic_playlist_->GetDynamicHistory()) { - RemoveItemsWithoutUndo(0, 1); - } + // Shrink the history, again this is not necessarily by 1, because the user + // might have moved items by hand. + const int remove_count = dynamic_history_length() - dynamic_playlist_->GetDynamicHistory(); + if (0 < remove_count) + RemoveItemsWithoutUndo(0, remove_count); + + // the above actions make all commands on the undo stack invalid, so we + // better clear it. + undo_stack_->clear(); } + if (current_item_index_.isValid()) { + last_played_item_index_ = current_item_index_; + Save(); + } + UpdateScrobblePoint(); } @@ -694,8 +711,10 @@ bool Playlist::dropMimeData(const QMimeData* data, Qt::DropAction action, int ro items << source_playlist->item_at(row); if (items.count() > kUndoItemLimit) { - // Too big to keep in the undo stack + // Too big to keep in the undo stack. Also clear the stack because it + // might have been invalidated. InsertItemsWithoutUndo(items, row, false); + undo_stack_->clear(); } else { undo_stack_->push(new PlaylistUndoCommands::InsertItems(this, items, row)); } @@ -751,22 +770,31 @@ void Playlist::TurnOnDynamicPlaylist(GeneratorPtr gen) { Save(); } -void Playlist::MoveItemsWithoutUndo(const QList &source_rows, int pos) { +void Playlist::MoveItemWithoutUndo(int source, int dest) { + MoveItemsWithoutUndo(QList() << source, dest); +} + +void Playlist::MoveItemsWithoutUndo(const QList& source_rows, int pos) { layoutAboutToBeChanged(); PlaylistItemList moved_items; // Take the items out of the list first, keeping track of whether the // insertion point changes int offset = 0; + int start = pos; foreach (int source_row, source_rows) { moved_items << items_.takeAt(source_row-offset); - if (pos != -1 && pos >= source_row) - pos --; + if (pos > source_row) { + start --; + } offset++; } + if (pos < 0) { + pos = items_.count(); + } + // Put the items back in - const int start = pos == -1 ? items_.count() : pos; for (int i=start ; iRemoveForegroundColor(kDynamicHistoryPriority); items_.insert(i, moved_items[i - start]); @@ -800,13 +828,13 @@ void Playlist::MoveItemsWithoutUndo(int start, const QList& dest_rows) { layoutAboutToBeChanged(); PlaylistItemList moved_items; - if (start == -1) { + int pos = start; + foreach (int dest_row, dest_rows) { + if (dest_row < pos) start--; + } + + if (start < 0) { start = items_.count() - dest_rows.count(); - } else { - foreach (int dest_row, dest_rows) { - if (start >= dest_row) - start--; - } } // Take the items out of the list first @@ -892,8 +920,10 @@ void Playlist::InsertItems(const PlaylistItemList& itemsIn, int pos, bool play_n const int start = pos == -1 ? items_.count() : pos; if (items.count() > kUndoItemLimit) { - // Too big to keep in the undo stack + // Too big to keep in the undo stack. Also clear the stack because it + // might have been invalidated. InsertItemsWithoutUndo(items, pos, enqueue); + undo_stack_->clear(); } else { undo_stack_->push(new PlaylistUndoCommands::InsertItems(this, items, pos, enqueue)); } @@ -923,7 +953,7 @@ void Playlist::InsertItemsWithoutUndo(const PlaylistItemList& items, } } - if (item == current_item_) { + if (item == current_item()) { // It's one we removed before that got re-added through an undo current_item_index_ = index(i, 0); last_played_item_index_ = current_item_index_; @@ -1184,6 +1214,18 @@ void Playlist::sort(int column, Qt::SortOrder order) { if (ignore_sorting_) return; + PlaylistItemList new_items(items_); + PlaylistItemList::iterator begin = new_items.begin(); + if(dynamic_playlist_ && current_item_index_.isValid()) + begin += current_item_index_.row() + 1; + + qStableSort(begin, new_items.end(), + boost::bind(&Playlist::CompareItems, column, order, _1, _2)); + + undo_stack_->push(new PlaylistUndoCommands::SortItems(this, column, order, new_items)); +} + +void Playlist::ReOrderWithoutUndo(const PlaylistItemList& new_items) { layoutAboutToBeChanged(); // This is a slow and nasty way to keep the persistent indices @@ -1191,10 +1233,8 @@ void Playlist::sort(int column, Qt::SortOrder order) { foreach (const QModelIndex& index, persistentIndexList()) { old_persistent_mappings[index.row()] = items_[index.row()]; } - - qStableSort(items_.begin(), items_.end(), - boost::bind(&Playlist::CompareItems, column, order, _1, _2)); - + + items_ = new_items; QMapIterator > it(old_persistent_mappings); while (it.hasNext()) { it.next(); @@ -1206,10 +1246,7 @@ void Playlist::sort(int column, Qt::SortOrder order) { } layoutChanged(); - - // TODO - undo_stack_->clear(); - + emit PlaylistChanged(); Save(); } @@ -1353,8 +1390,10 @@ bool Playlist::removeRows(int row, int count, const QModelIndex& parent) { } if (count > kUndoItemLimit) { - // Too big to keep in the undo stack + // Too big to keep in the undo stack. Also clear the stack because it + // might have been invalidated. RemoveItemsWithoutUndo(row, count); + undo_stack_->clear(); } else { undo_stack_->push(new PlaylistUndoCommands::RemoveItems(this, row, count)); } @@ -1449,28 +1488,28 @@ void Playlist::StopAfter(int row) { void Playlist::SetStreamMetadata(const QUrl& url, const Song& song) { qLog(Debug) << "Setting metadata for" << url << "to" << song.artist() << song.title(); - if (!current_item_) + if (!current_item()) return; - if (current_item_->Url() != url) + if (current_item()->Url() != url) return; // Don't update the metadata if it's only a minor change from before - if (current_item_->Metadata().artist() == song.artist() && - current_item_->Metadata().title() == song.title()) + if (current_item()->Metadata().artist() == song.artist() && + current_item()->Metadata().title() == song.title()) return; - current_item_->SetTemporaryMetadata(song); + current_item()->SetTemporaryMetadata(song); UpdateScrobblePoint(); InformOfCurrentSongChange(); } void Playlist::ClearStreamMetadata() { - if (!current_item_) + if (!current_item()) return; - current_item_->ClearTemporaryMetadata(); + current_item()->ClearTemporaryMetadata(); UpdateScrobblePoint(); emit dataChanged(index(current_item_index_.row(), 0), index(current_item_index_.row(), ColumnCount-1)); @@ -1481,18 +1520,25 @@ bool Playlist::stop_after_current() const { stop_after_.row() == current_item_index_.row(); } +PlaylistItemPtr Playlist::current_item() const { + // QList[] runs in constant time, so no need to cache current_item + if (current_item_index_.isValid() && current_item_index_.row() <= items_.length()) + return items_[current_item_index_.row()]; + return PlaylistItemPtr(); +} + PlaylistItem::Options Playlist::current_item_options() const { - if (!current_item_) + if (!current_item()) return PlaylistItem::Default; - return current_item_->options(); + return current_item()->options(); } Song Playlist::current_item_metadata() const { - if (!current_item_) + if (!current_item()) return Song(); - return current_item_->Metadata(); + return current_item()->Metadata(); } void Playlist::UpdateScrobblePoint() { @@ -1514,8 +1560,10 @@ void Playlist::Clear() { const int count = items_.count(); if (count > kUndoItemLimit) { - // Too big to keep in the undo stack + // Too big to keep in the undo stack. Also clear the stack because it + // might have been invalidated. RemoveItemsWithoutUndo(0, count); + undo_stack_->clear(); } else { undo_stack_->push(new PlaylistUndoCommands::RemoveItems(this, 0, count)); } @@ -1626,30 +1674,20 @@ void Playlist::SongInsertVetoListenerDestroyed() { } void Playlist::Shuffle() { - layoutAboutToBeChanged(); + PlaylistItemList new_items(items_); + + int begin = 0; + if(dynamic_playlist_ && current_item_index_.isValid()) + begin += current_item_index_.row() + 1; const int count = items_.count(); - for (int i=0 ; i < count; ++i) { + for (int i=begin; i < count; ++i) { int new_pos = i + (rand() % (count - i)); - std::swap(items_[i], items_[new_pos]); - - foreach (const QModelIndex& pidx, persistentIndexList()) { - if (pidx.row() == i) - changePersistentIndex(pidx, index(new_pos, pidx.column(), QModelIndex())); - else if (pidx.row() == new_pos) - changePersistentIndex(pidx, index(i, pidx.column(), QModelIndex())); - } + std::swap(new_items[i], new_items[new_pos]); } - current_virtual_index_ = virtual_items_.indexOf(current_row()); - layoutChanged(); - - // TODO - undo_stack_->clear(); - - emit PlaylistChanged(); - Save(); + undo_stack_->push(new PlaylistUndoCommands::ShuffleItems(this, new_items)); } namespace { diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index 5a7324cb4..075a5f860 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -44,6 +44,9 @@ namespace PlaylistUndoCommands { class InsertItems; class RemoveItems; class MoveItems; + class ReOrderItems; + class SortItems; + class ShuffleItems; } typedef QMap ColumnAlignmentMap; @@ -69,6 +72,7 @@ class Playlist : public QAbstractListModel { friend class PlaylistUndoCommands::InsertItems; friend class PlaylistUndoCommands::RemoveItems; friend class PlaylistUndoCommands::MoveItems; + friend class PlaylistUndoCommands::ReOrderItems; public: Playlist(PlaylistBackend* backend, @@ -176,6 +180,7 @@ class Playlist : public QAbstractListModel { bool stop_after_current() const; bool is_dynamic() const { return dynamic_playlist_; } + int dynamic_history_length() const; QString special_type() const { return special_type_; } void set_special_type(const QString& v) { special_type_ = v; } @@ -183,7 +188,7 @@ class Playlist : public QAbstractListModel { const PlaylistItemPtr& item_at(int index) const { return items_[index]; } const bool has_item_at(int index) const { return index >= 0 && index < rowCount(); } - PlaylistItemPtr current_item() const { return current_item_; } + PlaylistItemPtr current_item() const; PlaylistItem::Options current_item_options() const; Song current_item_metadata() const; @@ -217,7 +222,8 @@ class Playlist : public QAbstractListModel { const SongList& songs, int pos = -1, bool play_now = false, bool enqueue = false); // Removes items with given indices from the playlist. This operation is not undoable. void RemoveItemsWithoutUndo (const QList& indices); - + void ReshuffleIndices(); + // If this playlist contains the current item, this method will apply the "valid" flag on it. // If the "valid" flag is false, the song will be greyed out. Otherwise the grey color will // be undone. @@ -300,7 +306,6 @@ class Playlist : public QAbstractListModel { private: void SetCurrentIsPaused(bool paused); void UpdateScrobblePoint(); - void ReshuffleIndices(); int NextVirtualIndex(int i, bool ignore_repeat_track) const; int PreviousVirtualIndex(int i) const; bool FilterContainsVirtualIndex(int i) const; @@ -321,7 +326,9 @@ class Playlist : public QAbstractListModel { bool enqueue = false); PlaylistItemList RemoveItemsWithoutUndo(int pos, int count); void MoveItemsWithoutUndo(const QList& source_rows, int pos); + void MoveItemWithoutUndo(int source, int dest); void MoveItemsWithoutUndo(int start, const QList& dest_rows); + void ReOrderWithoutUndo(const PlaylistItemList& new_items); void RemoveItemsNotInQueue(); @@ -363,8 +370,6 @@ class Playlist : public QAbstractListModel { bool current_is_paused_; int current_virtual_index_; - PlaylistItemPtr current_item_; - bool is_shuffled_; qint64 scrobble_point_; diff --git a/src/playlist/playlistundocommands.cpp b/src/playlist/playlistundocommands.cpp index 828b5ad59..e505489b4 100644 --- a/src/playlist/playlistundocommands.cpp +++ b/src/playlist/playlistundocommands.cpp @@ -96,7 +96,7 @@ MoveItems::MoveItems(Playlist *playlist, const QList &source_rows, int pos) source_rows_(source_rows), pos_(pos) { - setText(tr("move songs", "", source_rows.count())); + setText(tr("move %n songs", "", source_rows.count())); } void MoveItems::redo() { @@ -107,4 +107,35 @@ void MoveItems::undo() { playlist_->MoveItemsWithoutUndo(pos_, source_rows_); } + +ReOrderItems::ReOrderItems(Playlist* playlist, const PlaylistItemList& new_items) + : Base(playlist), + old_items_(playlist->items_), + new_items_(new_items) { } + +void ReOrderItems::undo() { + playlist_->ReOrderWithoutUndo(old_items_); +} + +void ReOrderItems::redo() { + playlist_->ReOrderWithoutUndo(new_items_); +} + + +SortItems::SortItems(Playlist* playlist, int column, Qt::SortOrder order, + const PlaylistItemList& new_items) + : ReOrderItems(playlist, new_items), + column_(column), + order_(order) +{ + setText(tr("sort songs")); +} + + +ShuffleItems::ShuffleItems(Playlist* playlist, const PlaylistItemList& new_items) + : ReOrderItems(playlist, new_items) +{ + setText(tr("shuffle songs")); +} + } // namespace diff --git a/src/playlist/playlistundocommands.h b/src/playlist/playlistundocommands.h index 0a241aa83..0a0f9fe47 100644 --- a/src/playlist/playlistundocommands.h +++ b/src/playlist/playlistundocommands.h @@ -91,6 +91,33 @@ namespace PlaylistUndoCommands { QList source_rows_; int pos_; }; + + class ReOrderItems : public Base { + public: + ReOrderItems(Playlist* playlist, const PlaylistItemList& new_items); + + void undo(); + void redo(); + + private: + PlaylistItemList old_items_; + PlaylistItemList new_items_; + }; + + class SortItems : public ReOrderItems { + public: + SortItems(Playlist* playlist, int column, Qt::SortOrder order, + const PlaylistItemList& new_items); + + private: + int column_; + Qt::SortOrder order_; + }; + + class ShuffleItems : public ReOrderItems { + public: + ShuffleItems(Playlist* playlist, const PlaylistItemList& new_items); + }; } //namespace #endif // PLAYLISTUNDOCOMMANDS_H