GNOME Bugzilla – Bug 687573
Add proper smooth scrolling support
Last modified: 2013-03-03 22:53:40 UTC
See patches. This adds some smooth scrolling support to scroll views. A quick grep shows a few other places we use scrolling: * Calendar for changing months. * Alt-tab for changing apps. * When over the volume status icon for adjusting volume. * Zooming into windows in the overview. * Sliders, like in the volume menu. * Combo boxes, like in the user status menu. * Lock screen, for lifting up (already has smooth scrolling support). * Looking glass, for going to parent actors in the inspector. * Over the workspaces view for selecting workspaces. Of these, I think sliders and window zooming in the overview are the only ones that make sense. For consistency, if we make the slider respect scroll deltas, it probably makes sense for the volume icon to do the same. You can't test smooth scrolling with gnome-shell yet, as it requires XI2 for mutter, but you can test with the interactive test-suite: CLUTTER_ENABLE_XINPUT=1 ./tests/run-test.sh ./tests/interactive/scrolling.js
Created attachment 228029 [details] [review] st-scroll-view: Add proper smooth scrolling The code here before was added as dummy code to satisfy an error in the missing switch, and wasn't ever tested due to the lack of XI2 in mutter. Use the same math as GtkRange does to calculate scroll bar positions from raw XI2 deltas to allow for proper smooth scrolling.
Created attachment 228030 [details] [review] st-scroll-view: Throw away emulated pointer events These are sent by the X server and have large deltas. They really should be filtered out by Clutter (or the X server) somehow, but we don't have the means to do that yet.
Created attachment 228032 [details] [review] popupMenu: Add smooth scrolling support for sliders Allowing smooth scrolling on the Y axis to accurately adjust the value of the slider.
Created attachment 228033 [details] [review] volume: Add smooth scrolling support to adjust output volume
Created attachment 228034 [details] [review] workspace: Add smooth scrolling support to zoom windows
Created attachment 230238 [details] [review] scroll-bar: Add smooth scrolling support Do the same for StScrollBar.
*** Bug 689320 has been marked as a duplicate of this bug. ***
Review of attachment 228029 [details] [review]: Looks good.
Review of attachment 228030 [details] [review]: ::: src/st/st-scroll-view.c @@ +732,3 @@ return FALSE; + /* throw away this garbage event. we want smooth scrolling. */ Maybe file a clutter bug and add a comment here so that we don't forgot to remove it once we have a better place to put it?
Review of attachment 228032 [details] [review]: Looks good, but I could not test the sliders yet. So OK assuming you tested and it works.
Review of attachment 228033 [details] [review]: OK (same as above applies here).
Review of attachment 228034 [details] [review]: ::: js/ui/workspace.js @@ +278,3 @@ + let [dx, dy] = event.get_scroll_delta(); + delta = -dy * 10; + } Hmm ... we have this (and similar) code in multiple places now ... does it make sense to refactor and share it?
Review of attachment 230238 [details] [review]: Looks good but duplicates the code from the first patch. ::: src/st/st-scroll-bar.c @@ +477,3 @@ + + adjust_with_delta (adj, delta); +} Yet again code duplication ... move this into StAdjustment.
Review of attachment 228029 [details] [review]: Code duplication applies here as well.
Created attachment 231956 [details] [review] scroll-view: Add proper smooth scrolling The code here before was added as dummy code to satisfy an error in the missing switch, and wasn't ever tested due to the lack of XI2 in mutter. Use the same math as GtkRange does to calculate scroll bar positions from raw XI2 deltas to allow for proper smooth scrolling. OK, so this is my attempt at cleaning up the code. To address feedback, it turns out that having X/Clutter throw away emulated pointer events globally is a bad idea as there still may be some widgets that don't care about smooth scrolling. I listed a few in the bug description. There still is a small amount of duplicated code, but I don't think it's very big -- I can't merge more between scroll-bar and scroll-view because scroll-view is 2D, while scroll-bar (and adjustment) are 1D, so it's somewhat unclear how each component should handle each direction. If you think adjust_with_delta() is bad to be copy/pasted, we make a _st_get_delta_for_direction, but I'm not sure it's worth it. It's not very big a function.
Created attachment 231957 [details] [review] scroll-bar: Add smooth scrolling support Do the same for StScrollBar.
Created attachment 231958 [details] [review] popupMenu: Add smooth scrolling support for sliders Allowing smooth scrolling on the Y axis to accurately adjust the value of the slider. I didn't merge the " / 10" code, but I'm curious what you would want. Some form of utility that takes a step, a value, and an event and figured out how to interpret the scroll event?
Created attachment 231959 [details] [review] volume: Add smooth scrolling to adjust output volume Allow users to smoothly scroll on the volume indicator icon to adjust the volume. Do this by simply passing the scroll event to the slider inside the menu.
Created attachment 231960 [details] [review] workspace: Add smooth scrolling support to zoom windows
Review of attachment 231956 [details] [review]: OK that's fine this way.
Review of attachment 231957 [details] [review]: OK.
Review of attachment 231958 [details] [review]: I didn't think of anything specific ... I just noticed that we have this part multiple times. But lets leave it as is for now.
Review of attachment 231959 [details] [review]: OK.
Review of attachment 231960 [details] [review]: OK.
Attachment 231956 [details] pushed as 06dc12e - scroll-view: Add proper smooth scrolling Attachment 231957 [details] pushed as f162dd7 - scroll-bar: Add smooth scrolling support Attachment 231958 [details] pushed as 7d4e14f - popupMenu: Add smooth scrolling support for sliders Attachment 231959 [details] pushed as 8d4855f - volume: Add smooth scrolling to adjust output volume Attachment 231960 [details] pushed as 770ff19 - workspace: Add smooth scrolling support to zoom windows
*** Bug 665164 has been marked as a duplicate of this bug. ***