After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 635565 - When scrolling through artist UI feels unresponsive
When scrolling through artist UI feels unresponsive
Status: RESOLVED FIXED
Product: banshee
Classification: Other
Component: User Interface
unspecified
Other Linux
: Normal normal
: 1.x
Assigned To: Alex Launi
Banshee Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-11-22 22:14 UTC by Ted Gould
Modified: 2010-12-15 21:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Changes the keyboard navigation behavior to only fire Changed when KeyReleased is received (17.20 KB, patch)
2010-11-25 07:58 UTC, Alex Launi
none Details | Review
Add myself to headers for these (18.00 KB, patch)
2010-11-25 08:15 UTC, Alex Launi
none Details | Review
Refactored to remove the copied and pasted Selection code (18.45 KB, patch)
2010-11-25 13:52 UTC, Alex Launi
reviewed Details | Review
Changes Faux methods to bool notify pattern (18.44 KB, patch)
2010-12-01 21:26 UTC, Alex Launi
none Details | Review
Changes Faux methods to bool notify pattern (18.83 KB, patch)
2010-12-01 21:28 UTC, Alex Launi
needs-work Details | Review
Fix for review (18.91 KB, patch)
2010-12-15 20:07 UTC, Alex Launi
none Details | Review
Change private member to stack variable (18.85 KB, patch)
2010-12-15 20:21 UTC, Alex Launi
committed Details | Review

Description Ted Gould 2010-11-22 22:14:36 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.
Comment 1 David Nielsen 2010-11-23 02:07:25 UTC
That is awfully slow on my dual core atom as well.

Could be related to:
https://bugzilla.gnome.org/show_bug.cgi?id=613723
Comment 2 Alex Launi 2010-11-23 15:56:18 UTC
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.
Comment 3 Gabriel Burt 2010-11-23 19:06:10 UTC
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.
Comment 4 Alex Launi 2010-11-25 07:58:13 UTC
Created attachment 175219 [details] [review]
Changes the keyboard navigation behavior to only fire Changed when KeyReleased is received
Comment 5 Alex Launi 2010-11-25 07:59:31 UTC
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.
Comment 6 Alex Launi 2010-11-25 08:15:35 UTC
Created attachment 175222 [details] [review]
Add myself to headers for these
Comment 7 David Nielsen 2010-11-25 11:58:50 UTC
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.
Comment 8 Alex Launi 2010-11-25 13:52:43 UTC
Created attachment 175239 [details] [review]
Refactored to remove the copied and pasted Selection code
Comment 9 Gabriel Burt 2010-11-30 18:58:26 UTC
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.
Comment 10 Alex Launi 2010-11-30 19:04:38 UTC
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.
Comment 11 Alex Launi 2010-12-01 21:26:23 UTC
Created attachment 175670 [details] [review]
Changes Faux methods to bool notify pattern
Comment 12 Alex Launi 2010-12-01 21:28:01 UTC
Created attachment 175671 [details] [review]
Changes Faux methods to bool notify pattern

Forgot to git add my changes last time
Comment 13 Gabriel Burt 2010-12-15 19:38:43 UTC
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
Comment 14 Alex Launi 2010-12-15 20:07:51 UTC
Created attachment 176495 [details] [review]
Fix for review
Comment 15 Alex Launi 2010-12-15 20:21:22 UTC
Created attachment 176500 [details] [review]
Change private member to stack variable
Comment 16 Gabriel Burt 2010-12-15 20:30:30 UTC
Review of attachment 176500 [details] [review]:

Looks good, please fix the commit msg and push