GNOME Bugzilla – Bug 635565
When scrolling through artist UI feels unresponsive
Last modified: 2010-12-15 21:23:42 UTC
If you're in the browser mode and you select an artist then you hold down the down arrow key the UI will start to stutter.
That is awfully slow on my dual core atom as well. Could be related to: https://bugzilla.gnome.org/show_bug.cgi?id=613723
I think this has more to do with pounding the db. If you run banshee with --debug-sql and do this, you see it's making a veritable buttload of sql requests. I think the best solution would be to modify the search behavior to just scroll the list on mouse down, and not make the db request until mouse up.
To clarify Alex's solution, don't filter the album and track lists on the artist selection until ButtonReleased. That could work -- hopefully wouldn't feel wrong/jarring.
Created attachment 175219 [details] [review] Changes the keyboard navigation behavior to only fire Changed when KeyReleased is received
Note: the visual behavior is identical to current Banshee, the only difference I noticed was that the DB doesn't get hammered, so Banshee's UI does not appear to hang while it finishes executing the backlog of queries.
Created attachment 175222 [details] [review] Add myself to headers for these
I applied the patch to git master and the difference is very noticeable, the visual experience is the same but the stalls are gone. Great work Alex.
Created attachment 175239 [details] [review] Refactored to remove the copied and pasted Selection code
Review of attachment 175239 [details] [review]: Hrm, I'm not happy with how complicated this ended up being -- I'd hoped it could be a relatively simple change. I need to think about it more. ::: Hyena/Hyena.Collections/Selection.cs @@ +97,3 @@ } + public void FauxSelect (int index) Instead of 'Faux', should probably do what we already do -- just add another method w/ the same name that also takes a "bool notify". Or maybe we should change the API to add Freeze()/Unfreeze (bool notifyIfChanged) to avoid this issue.
I don't like the freeze/unfreeze solution. That's too fragile. The bool option is essentially the same, I'm fine with that. It's probably cleaner- specially if that's the pattern we use in other places.
Created attachment 175670 [details] [review] Changes Faux methods to bool notify pattern
Created attachment 175671 [details] [review] Changes Faux methods to bool notify pattern Forgot to git add my changes last time
Review of attachment 175671 [details] [review]: Just a couple tiny nitpicks, including 1st line of commit msg is too long ::: Hyena.Gui/Hyena.Data.Gui/ListView/ListView_Interaction.cs @@ +101,3 @@ } + private int RowIndex { get; set; } 'RowIndex' isn't descriptive enough -- 'ScrollTargetItemIndex' maybe? 'Row' isn't really accurate either I don't think since it could be an index into a grid ::: Hyena/Hyena.Collections/Selection.cs @@ +176,3 @@ + { + SelectRange (a, b, true); + remove blank line
Created attachment 176495 [details] [review] Fix for review
Created attachment 176500 [details] [review] Change private member to stack variable
Review of attachment 176500 [details] [review]: Looks good, please fix the commit msg and push