playlist subsystem overhaul

[Nikolaj Hald Nielsen]

First of all, I think it is super cool that you have taken it upon
yourself to refactor the playlist, something that has been sorely
needed for some time now. My comments here are based on actually
trying out your patch. I have not looked into the code at all just
yet.

First of all the playlist now seems to run super fast, even with an
insane amount for track added ( I have tried it with up to 40k track
so far ). I do sort of miss the animations, but I think I would be
willing to trade them for now for a playlist that has the basics
working really well.

I know most of these are nitpicking, and that we should fix these
after you commit the patch, but I just want to list the issues I have
encountered so far:

 - Clicking the album header selects all tracks in the album ( good )
but is not in itself dragable ( bad )
 - Starting a drag with a large selection is very slow ( rendering of
the image shown when dragging? )
 - Crashes when clearing a large playlist using the clear button.
 - The background of the view should be made completely transparent.
 - I do sort of miss the separator between album header and first track


Other than that it looks good. Will look into it some more when I have the time.

- Nikolaj

On Mon, Oct 6, 2008 at 8:01 PM, Soren Harward <stharward at gmail.com> wrote:
> As I mentioned on amarok-devel last week, I've been working on an overhaul of
> the playlist subsytem, and it's now ready to commit to the SVN tree.  It fixes
> a bunch of bugs, and (from my perspective) vastly cleans up and simplifies the
> subsystem.  But it's a big (about 16k lines, 6k of which are entirely new code)
> invasive patch, so I'd like to have a couple people look over it before I
> commit it.  The full patch (against r868502) is available at:
>
> http://www.mandy-and-soren.org/misc_files/playlist_overhaul-r868502.patch.gz
>
> And here's what the SVN commit log would be if I were committing it now:
>
> ---------
> Major overhaul of the playlist subsystem
>
> == Changes for end users ==
> Mouse and keyboard event handling, and the selection system have both been
> significantly improved.  The undo/redo commands now work properly in all cases.
> Album grouping works consistently.  Some of the eye candy (like animations) has
> been temporarily removed, but will hopefully return in the future.
>
> == Changes in general Amarok code ==
> The PlaylistModel has been split into four different classes:
> 1) PlaylistActions
> 2) PlaylistController
> 3) PlaylistModel
> 4) PMGroupingProxy
>
> The play, nextTrack, and previousTrack functions are now part of
> PlaylistActions.  Functions that modify the contents of the playlist (insert,
> remove, and move tracks; undo and redo) are now part of PlaylistController.
> If you want to find out about the contents of the playlist, you still use
> PlaylistModel, though you are forbidden from modifying it.  I encourage you to
> take a look at the headers for each of these classes to find out what functions
> are available.
>
> PMGroupingProxy is described in the next section, and should not be used for
> anything other than playlist views.
>
> The signals and slots for the PlaylistModel have changed substantially;
> PlaylistModel now behaves much more like a QAbstractItem model when it is
> modified.  I have also added a position-independent accessing system for
> situations where you care what is in the playlist, but not what order the
> playlist is in (eg, Random Track mode).
>
> The Model::trackNumberLessThan() function has moved to Meta::Track::lessThan()
>
> == Changes in the playlist subsystem ==
>
> The src/playlist/ directory has been rearranged.
>
> Along with the PlaylistModel changes, the PlaylistGraphicsView has been
> deprecated, though it has not yet removed from the source tree and will
> probably still work with all the other changes I've made.  Its replacement is a
> new PrettyListView, which is a subclass of QListView.  The reworking of
> PlaylistModel to behave more like QAbstractItemModel should vastly simplify
> writing new playlist views.  Even the base QListView adequately displays the
> content of the playlist and responds to model changes.
>
> The grouping functionality which used to be in PlaylistModel has now be pulled
> out into PMGroupingProxy, and the grouping algorithm has been rewritten so that
> it stays properly synchronized with the playlist contents.  PrettyListView and
> PlaylitGraphicsView both use this proxy model.
>
> --------
>
> If you have any feedback before I commit the patch, please feel free to email
> me.  If there aren't any major objections, I'll commit this in a day or two,
> and then we can start bug hunting.
>
> --
> Soren Harward
> stharward at gmail.com
> _______________________________________________
> Amarok-devel mailing list
> Amarok-devel at kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel
>