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 729608 - Physically based kinetic scrolling
Physically based kinetic scrolling
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkScrolledWindow
unspecified
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-05-05 22:50 UTC by Lieven van der Heide
Modified: 2014-06-05 14:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (48.78 KB, patch)
2014-05-05 22:50 UTC, Lieven van der Heide
none Details | Review
Physically based kinetic scrolling (50.90 KB, patch)
2014-05-17 22:26 UTC, Lieven van der Heide
none Details | Review
gtk: Add kinetic scrolling helper (10.58 KB, patch)
2014-05-26 16:24 UTC, Carlos Garnacho
committed Details | Review
scrolledwindow: Replace kinetic scrolling with GtkKineticScrolling (10.01 KB, patch)
2014-05-26 16:24 UTC, Carlos Garnacho
committed Details | Review

Description Lieven van der Heide 2014-05-05 22:50:18 UTC
I modified the GtkScrolledWindow's kinetic scrolling to use the actual physics laws for friction and springs. IMHO, this gives a much more pleasing result. 

I created a new gtkkineticscrolling.h/.c, which implements the functionality to scroll in a single direction. When decelerating, it applies just friction, when overshooting, it applies both friction and a spring force. It can be configured using the following two parameters

#define DECELERATION_FRICTION 5
#define OVERSHOOT_FRICTION 20

For overshoot, a spring stiffness is also needed, but this one is computed from OVERSHOOT_FRICTION, such that it returns to its equilibrium position as quick as possible, without oscillating (critically damped).

In gtkscrollwindow.c, I added two instances of GtkKineticSCrolling to KineticScrollData, one for horizontal and one for vertical scrolling.

With with this new overshoot, it can happen that the position exceeds the MAX_OVERSHOOT_DISTANCE range, and because of that I couldn't use _gtk_scrolled_window_set_adjustment_valu any more. Instead I know modify the priv->unclamped_?adj_value's directly, and then call gtk_adjustment_set_value.

I have attached a patch with these changes.
Comment 1 Lieven van der Heide 2014-05-05 22:50:56 UTC
Created attachment 275926 [details] [review]
Patch
Comment 2 Matthias Clasen 2014-05-11 13:44:30 UTC
Carlos was talking about actually going the opposite direction, and removing the overshooting - it is causing problems with things like popovers, touch selection handles, etc.
Comment 3 Lieven van der Heide 2014-05-11 16:30:54 UTC
What are those problems? Many other toolkits do have both features, so it doesn't seem to me it can be an inherent incompatibility between the two.
Comment 4 Carlos Garnacho 2014-05-14 14:20:29 UTC
I need to test this a bit further, but the patch(es) looks mostly fine to me, with the following comments:

* Explanatory blurbs should IMO go to the .c file, below the license comment. Even if this is not picked by gtk-doc, docs in gtk+ are most usually in .c files instead of .h ones
* gtkkineticscrolling.h should go to $(gtk_private_h_sources) in Makefile.am
* Indentation mishap in:

@@ -2378,11 +2377,10 @@ gtk_scrolled_window_start_deceleration (GtkScrolledWindow *scrolled_window)
       gdouble lower,upper;
       GtkAdjustment *vadjustment;
       
+		vadjustment = gtk_range_get_adjustment (GTK_RANGE (priv->vscrollbar));
       lower = gtk_adjustment_get_lower(vadjustment);

As for the "problems" with overshooting, what I intend to is replacing the visual overshoot animation with a gradient one when limits are hit. The way the overshoot_window (and its contents) are relocated involves quite some trickery (eg. overshooting down/right involves many queue_resize() calls on the child), and also fool the child widget coordinates, so popovers and text handles may appear misplaced during overshooting.

But I don't think this has much to do with this patch, the "overshooting" phase indeed exists, but visually it is represented differently.
Comment 5 Lieven van der Heide 2014-05-17 22:26:38 UTC
Created attachment 276709 [details] [review]
Physically based kinetic scrolling

I've attached a patch with the fixes you suggested.
Comment 6 Carlos Garnacho 2014-05-26 16:24:27 UTC
Created attachment 277222 [details] [review]
gtk: Add kinetic scrolling helper

GtkKineticScrolling implements the actual physics laws for friction
and springs. When created, position/velocity/boundaries/constants are
given, so at every gtk_kinetic_scrolling_tick() it returns the current
position, and whether the system is in rest.
Comment 7 Carlos Garnacho 2014-05-26 16:24:38 UTC
Created attachment 277223 [details] [review]
scrolledwindow: Replace kinetic scrolling with GtkKineticScrolling

Two GtkKineticScrolling helpers are used, one per axis direction.
Comment 8 Carlos Garnacho 2014-05-26 16:35:14 UTC
(In reply to comment #5)
> Created an attachment (id=276709) [details] [review]
> Physically based kinetic scrolling
> 
> I've attached a patch with the fixes you suggested.

Thanks Lieven for that last update and your work on this, I've taken the liberty to update/merge your patches (with your authorship), against current master (there's been additional GtkScrolledWindow changes there that made these patches not apply). I've done other minor changes:

- Replaced init(&kinetic_data,...) with new/free functions, that helped me hide the struct and enum. GtkScrolledWindow is mainly interested in knowing the current position and whether rest was found, so made that retrievable from gtk_kinetic_scrolling_tick()

- Made it re-reset overshoot at velocity=0 when the overshoot extra border edge was hit, in your current patches scrolling would be performed past that limit

- Other minor code style issues I found

But the core of the patch is largely unchanged, and works great IMO, nicely done!
Comment 9 Lieven van der Heide 2014-05-30 15:01:18 UTC
Thanks Carlos, your changes look fine to me. So can it be submitted to git?
Comment 10 Carlos Garnacho 2014-06-05 14:58:07 UTC
Attachment 277222 [details] pushed as c726226 - gtk: Add kinetic scrolling helper
Attachment 277223 [details] pushed as 828594d - scrolledwindow: Replace kinetic scrolling with GtkKineticScrolling