playlist subsystem overhaul

Nikolaj Hald Nielsen nhnfreespirit at gmail.com
Mon Oct 6 22:14:30 CEST 2008


oh, and tracks without a number are all automatically given number 0. :-)

- Nikolaj

On Mon, Oct 6, 2008 at 10:11 PM, Nikolaj Hald Nielsen
<nhnfreespirit at gmail.com> wrote:
> 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
>>
>


More information about the Amarok-devel mailing list