odd behaviour of browsers/CollectionTreeItemModelBase

[Maximilian Kossick]

On Sat, Jun 19, 2010 at 10:46 AM, Nikhil Marathe <nsm.nikhil at gmail.com> wrote:
> Hi,
>
> CollectionTreeItemModelBase.cpp - populateChildren() is responsible
> for populating collection entries like Artist/Album listings when a
> collection is expanded. It clears the list every time (except for
> Various Artists) some change happens, or the underlying QueryMaker
> emits new results.

That is not correct. It compares the current state of the
CollectionTreeItem hierarchy with the new state that was delivered by
the QueryMaker. In doing that it first removes all CollectionTreeItems
for Meta::Data objects that were not returned by the query, refreshes
the remaining ones, and that adds the new Meta::Data objects as
CollectionTreeItems.

>
> In the current Amarok code, where most collections are local, this is
> fine, because even after the list is completely repopulated, it is
> still quite fast.

see above.

>
> But two projects, Upnp and Playdar integration have a problem with
> this style of doing it. On a network, it makes much more sense to emit
> results as they come. For example, if I emit new artists I receive
> from a Upnp server, I can start producing entries in 2 seconds. This
> way doesn't work currently because the next time I emit new entries,
> all my old artists have been cleared away! The solution is to cache
> the artists in the collection and then send all of them once I have
> received all of them. But this can take 30 or more seconds depending
> on the size of the collection, leading to a HUGE difference in the
> speed the user perceives.

Not quite the correct solution. As stated in the QueryMaker
documentation, newResultReady may be emitted mutiple times (there is a
method that gives the actual number so clients know what to expect).
Unfortunately it has not been defined whether each additional
newResultReady signal will contain all Meta objects (i.e. including
the ones that have been emitted previously) or only a delta. This is
two different behaviours are not handled consistently in Amarok. You
would have to decide for one of the above, and fix all the places
where this would cause issues. in my opinion the latter approach makes
more sense. Off the top of my head you would have to change
ProxyQueryMaker and CollectionTreeItemModelBase (keep a list of
Meta::Data ptrs per QueryMaker and append that list to the new result
before calling populateChildren if the QueryMaker says that it will
emit more than one newResultReady signal). Creating a convenience
CachingQueryMaker class that simply wraps another QueryMaker, waits
for all newResultReady signals of the wrapped QM to complete and then
emits a single newResultReady signal might be helpful, similar to what
MetaQueryMaker does.

> My solution is that the list gets cleared BEFORE the query maker is
> run(). But while a query is running, the list is NOT cleared, assuming
> that the query maker won't give duplicates. But I'm not the author of
> the code and might be overlooking some things. Or perhaps this may not
> be odd behaviour at all due to certain reasons I'm not aware off.

I'm not quite sure what you are talking about here.

> So if what I am suggesting, is the solution, I'm willing to take a
> stab at fixing it. But I would just like to get comments from more
> experienced developers ( especially Maximilian who wrote the code ).
>
> Regards,
> Nikhil
>