WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
16123
[GTK] Smooth scrolling support
https://bugs.webkit.org/show_bug.cgi?id=16123
Summary
[GTK] Smooth scrolling support
Alp Toker
Reported
2007-11-24 18:23:09 PST
I find smooth scrolling greatly enhances browsing in Firefox and IE. It would be good to add support for this, but perhaps not enable the feature by default since it's not conventional for GTK+ widgets (and also slightly controversial IIRC). I think we can take a clever shortcut here by plugging into the CSS animation controllers to provide a smooth (bezier, cubic?) function for scrolling. Scrolling should be smooth not only in the outermost frame, but also child frames.
Attachments
Patch
(37.99 KB, patch)
2011-06-01 10:38 PDT
,
Scott Byer
tony
: review-
Details
Formatted Diff
Diff
Patch
(10.42 KB, patch)
2011-08-01 09:47 PDT
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Patch
(18.18 KB, patch)
2012-02-17 02:30 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Patch
(17.80 KB, patch)
2012-02-18 01:09 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Patch
(7.40 KB, patch)
2012-02-21 01:31 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Patch
(7.46 KB, patch)
2012-02-22 07:33 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Updated patch
(7.48 KB, patch)
2012-03-02 02:05 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Alp Toker
Comment 1
2007-11-24 20:47:05 PST
Relevant GTK+ bug:
http://bugzilla.gnome.org/show_bug.cgi?id=444659
Bug 444659 – Add GtkTimeline to ease animations development
Robert Carr
Comment 2
2008-12-31 02:29:46 PST
Just a relevant thing, I'm currently working on a set of patches to xf86-input-synaptics, and GTK to enable smooth scrolling support for a window, however not smooth scrolling in the firefox sense, but in the sense of the scroll events being reported with pixel granularity by the mouse driver, which I find to be MUCH nicer than firefox smooth scrolling. This is also the model used by OSX. There are some videos of it being used with the Seed webkit browser at:
http://www.hortont.com/racarr/?p=20
Scott Byer
Comment 3
2011-06-01 10:38:21 PDT
Created
attachment 95622
[details]
Patch
Scott Byer
Comment 4
2011-06-01 10:46:39 PDT
This is a start; it adds the plumbing for run-time enablement and an initial implementation (that leaves lots of tuning to do). Currently doesn't affect the standalone WebKit build at all. The initial focus is Linux/Chromium OS, but eventually this code wants to be refactored with the existing but turned off Windows smooth scrolling code into something more cross platform. Some of this will also dovetail with the platform gestures work.
Vangelis Kokkevis
Comment 5
2011-06-01 10:57:56 PDT
I haven't had the chance to review the code but at the high-level there doesn't seem to be anything linux-specific here. Should the class be renamed to something more generic or move to platform/chromium and become ScrollAnimatorChromium?
Scott Byer
Comment 6
2011-06-01 11:04:58 PDT
No, there really isn't anything Linux specific, and I don't think there will be (except for initialization paramaters, possibly, especially since we want to keep the system provided Mac touchpad behavior). The move to platform/chromium does make sense to me.
Tony Chang
Comment 7
2011-06-01 11:24:21 PDT
This is the wrong bug for this patch. This bug is for smooth scrolling in WebKit GTK+ (e.g., build-webkit --gtk). Please open a new bug for adding smooth scrolling to Chromium.
Tony Chang
Comment 8
2011-06-01 11:25:55 PDT
Comment on
attachment 95622
[details]
Patch I agree with Vangelis that these files should be in platform/chromium and shouldn't have Linux in the filename if they're not Linux specific.
James Robinson
Comment 9
2011-06-03 12:07:39 PDT
This bug is for the GTK port, not chromium.
Zan Dobersek
Comment 10
2011-08-01 09:47:29 PDT
Created
attachment 102527
[details]
Patch This patch uses ScrollAnimatorNone class to provide smooth scrolling and introduces a new flag to enable it (it is disabled by default)
Gustavo Noronha (kov)
Comment 11
2012-02-16 15:12:17 PST
API looks straight forward. Any reason to keep this behind a compilation option, though? I'd just leave it on and rely on the runtime option.
Martin Robinson
Comment 12
2012-02-16 15:27:36 PST
Comment on
attachment 102527
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=102527&action=review
Looks good! Is there any compelling reason to make this a compile-time option? Why not just compile it in always. r- because of the missing documentation. We need one more GTK+ reviewer to approve the API, but I think it's pretty non-controversial. Do you think you could add the setting to WebKit2 as well?
> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:932 > + * Whether ...
You're missing the documentation here. :(
Zan Dobersek
Comment 13
2012-02-17 02:30:35 PST
Created
attachment 127560
[details]
Patch
Zan Dobersek
Comment 14
2012-02-17 02:34:39 PST
Comment on
attachment 127560
[details]
Patch This patch also adds support for enabling smooth scrolling in WebKit2. Documentation should now be properly processed in both WebKit1 and WebKit2. This also adds two additional functions to the core WebKit2 C API, along with a basic preference in WebPreferencesStore. Also settings the cq flag. Thanks for the reviews!
Martin Robinson
Comment 15
2012-02-17 09:35:23 PST
Comment on
attachment 127560
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=127560&action=review
This looks good to me. I think we just need some Mac guys to look at the proposed C API changes.
> Source/JavaScriptCore/wtf/Platform.h:490 > +#define ENABLE_SMOOTH_SCROLLING 1
Nit. This is probably something that we want to set in the GNUmakefile. We can just do it there unconditionally.
Martin Robinson
Comment 16
2012-02-17 09:37:58 PST
Anders or Sam, do you mind giving a nod to these WebKit2 C API changes?
Zan Dobersek
Comment 17
2012-02-18 01:09:16 PST
Created
attachment 127700
[details]
Patch
Zan Dobersek
Comment 18
2012-02-18 01:30:02 PST
(In reply to
comment #15
)
> > Source/JavaScriptCore/wtf/Platform.h:490 > > +#define ENABLE_SMOOTH_SCROLLING 1 > > Nit. This is probably something that we want to set in the GNUmakefile. We can just do it there unconditionally.
Created a small section near the top of Source/WebCore/GNUmakefile.am that appends such features to FEATURE_DEFINES and webcore_cppflags in the latest patch. In future if more features require the same, the comment in that section can be expanded to nicely list all such features.
Carlos Garcia Campos
Comment 19
2012-02-18 01:36:40 PST
Comment on
attachment 127700
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=127700&action=review
Please, add a test the new WEbKit2 setting in Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp
> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2053 > + return WKPreferencesGetEnableScrollAnimator(settings->priv->preferences.get());
I'm a bit confused, the scroll animator always does smooth scrolling or are there other types of scroll animations?
Zan Dobersek
Comment 20
2012-02-18 08:41:45 PST
(In reply to
comment #19
)
> (From update of
attachment 127700
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=127700&action=review
> > Please, add a test the new WEbKit2 setting in Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp >
Sure, will do.
> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2053 > > + return WKPreferencesGetEnableScrollAnimator(settings->priv->preferences.get()); > > I'm a bit confused, the scroll animator always does smooth scrolling or are there other types of scroll animations?
The pure ScrollAnimator class only does the normal scrolling (the one in use today). It is platform-independent. Smooth scrolling is provided by ScrollAnimatorWin class for Windows platform, ScrollAnimatorMac class for Mac platform, and ScrollAnimatorNone class, which provides smooth scrolling in a platform-independent way. The ScrollAnimatorNone is obviously the only option, but is only used instead of the core ScollAnimator class if the scroll animator setting is enabled.
Anders Carlsson
Comment 21
2012-02-18 08:51:43 PST
Comment on
attachment 127700
[details]
Patch Does this really need to be a setting? Isn't there some default "use animations" setting or style property in gtk? When would you ever want to disable this?
Zan Dobersek
Comment 22
2012-02-18 11:54:49 PST
(In reply to
comment #21
)
> (From update of
attachment 127700
[details]
) > Does this really need to be a setting? Isn't there some default "use animations" setting or style property in gtk? When would you ever want to disable this?
There indeed is a 'gtk-use-animations' property in GtkSettings, which defaults to true. If acceptable to reviewers, the code could just connect to the notify::gtk-use-animation signal of GtkSettings and change the WebCore setting appropriately. Though, without the WKPreferences API additions, this would require the connecting and adjusting to the 'gtk-use-animations' value to be performed in WebProcess. But ideally, the GtkSettings instance would be gathered from the specific GtkWidget (the web view), which is a part of UIProcess, and as such AFAIK not reachable from WebProcess. How about keeping the WKPreferences API additions private, in WKPreferencesPrivate.h? [1]
http://developer.gnome.org/gtk3/stable/GtkSettings.html#GtkSettings--gtk-enable-animations
Martin Robinson
Comment 23
2012-02-18 12:00:32 PST
(In reply to
comment #22
)
> (In reply to
comment #21
) > > (From update of
attachment 127700
[details]
[details]) > > Does this really need to be a setting? Isn't there some default "use animations" setting or style property in gtk? When would you ever want to disable this? > > There indeed is a 'gtk-use-animations' property in GtkSettings, which defaults to true. If acceptable to reviewers, the code could just connect to the notify::gtk-use-animation signal of GtkSettings and change the WebCore setting appropriately.
When gtk-use-animations is true though, does GTK+ use smooth scrolling? If not, then we'll need to look elsewhere to maintain consitency with GTK+.
Zan Dobersek
Comment 24
2012-02-18 12:24:57 PST
(In reply to
comment #23
)
> (In reply to
comment #22
) > > (In reply to
comment #21
) > > > (From update of
attachment 127700
[details]
[details] [details]) > > > Does this really need to be a setting? Isn't there some default "use animations" setting or style property in gtk? When would you ever want to disable this? > > > > There indeed is a 'gtk-use-animations' property in GtkSettings, which defaults to true. If acceptable to reviewers, the code could just connect to the notify::gtk-use-animation signal of GtkSettings and change the WebCore setting appropriately. > > When gtk-use-animations is true though, does GTK+ use smooth scrolling? If not, then we'll need to look elsewhere to maintain consitency with GTK+.
Oops, it's actually gtk-enable-animations. The only Gtk application I've seen to date to perform smooth scrolling is the gnome-about dialog. It doesn't respect the gtk-use-animations setting. Grepping through the gtk source, this setting is used to decide whether or not to animate smaller animations, such as expanding the tree view or sliding a toolbar item etc. Considering this, gtk-enable-animations might not be appropriate to control whether or not to enable smooth scrolling.
Carlos Garnacho
Comment 25
2012-02-19 02:23:38 PST
In the end, smooth scrolling is going to be handled through a new GDK_SMOOTH_SCROLL_MASK evmask flag, but there's not going to be a setting as such to turn it off/on, widgets just do decide which kind of scroll events fit them better
Carlos Garcia Campos
Comment 26
2012-02-19 23:54:19 PST
So, I would just enable this unconditionally. If someone asks for a way to disable it we can then discuss whether to add a new setting to enable/disable it.
Martin Robinson
Comment 27
2012-02-20 00:14:09 PST
(In reply to
comment #26
)
> So, I would just enable this unconditionally. If someone asks for a way to disable it we can then discuss whether to add a new setting to enable/disable it.
Just to clarify, you mean we should do smooth scrolling unconditionally when the GDK_SMOOTH_SCROLL_MASK flag is active? If so, I believe I agree.
Carlos Garcia Campos
Comment 28
2012-02-20 00:39:02 PST
(In reply to
comment #27
)
> (In reply to
comment #26
) > > So, I would just enable this unconditionally. If someone asks for a way to disable it we can then discuss whether to add a new setting to enable/disable it. > > Just to clarify, you mean we should do smooth scrolling unconditionally when the GDK_SMOOTH_SCROLL_MASK flag is active? If so, I believe I agree.
No, we don't need GDK_SMOOTH_SCROLL_MASK because we have our own scroll animator that does smooth scrolling. GDK_SMOOTH_SCROLL_MASK will not have any effect in WebView because we don't implement GtkScrollable. I'm not sure it's clear what Carlos said. GTK+ doesn't still support smooth scrolling. Current implementation (in smooth-scrolling branch) adds GDK_SMOOTH_SCROLL_MASK which makes that delta information is included in GdkScroll events. I will be implemented in GtkScrolledWindow, and widgets will decide whether they want smooth scrolling or not, by adding GDK_SMOOTH_SCROLL_MASK to its own events masks (typically done in realize()). So, there won't be any API nor setting to enable/disable smooth scrolling in widgets. If GTK+ devs decide that GtkTreeView does smooth scrolling, it will include the GDK_SMOOTH_SCROLL_MASK and will do smooth scrolling unconditionally. My opinion is that we should do the same and decide to use smooth scrolling unconditionally in WebView (in our case using the scroll animator, I don't need GDK_SMOOTH_SCROLL_MASK). If we eventually see that people want to be able to disable it, we can just add a setting later.
Martin Robinson
Comment 29
2012-02-20 00:44:53 PST
(In reply to
comment #28
)
> > Just to clarify, you mean we should do smooth scrolling unconditionally when the GDK_SMOOTH_SCROLL_MASK flag is active? If so, I believe I agree. >
> My opinion is that we should do the same and decide to use smooth scrolling unconditionally in WebView (in our case using the scroll animator, I don't need GDK_SMOOTH_SCROLL_MASK). If we eventually see that people want to be able to disable it, we can just add a setting later.
I would like to understand more about whether there are plans to switch all built-in GTK+ widgets to smooth scrolling or how this decision is made if it's done for some widgets and not others. Either way, adding a setting up-front seems low risk.
Carlos Garcia Campos
Comment 30
2012-02-20 01:00:35 PST
(In reply to
comment #29
)
> (In reply to
comment #28
) > > > > Just to clarify, you mean we should do smooth scrolling unconditionally when the GDK_SMOOTH_SCROLL_MASK flag is active? If so, I believe I agree. > > > > > My opinion is that we should do the same and decide to use smooth scrolling unconditionally in WebView (in our case using the scroll animator, I don't need GDK_SMOOTH_SCROLL_MASK). If we eventually see that people want to be able to disable it, we can just add a setting later. > > I would like to understand more about whether there are plans to switch all built-in GTK+ widgets to smooth scrolling or how this decision is made if it's done for some widgets and not others. Either way, adding a setting up-front seems low risk.
It will depend on every widget, I think tree view, icon view, text view will do smooth scrolling, for example. So it's up to use to decide. I don't mind if we add the setting in wk1, but for for wk2 we could try to enable it by default and add the setting later if we see we really needed.
Gustavo Noronha (kov)
Comment 31
2012-02-20 06:14:38 PST
(In reply to
comment #30
)
> It will depend on every widget, I think tree view, icon view, text view will do smooth scrolling, for example. So it's up to use to decide. I don't mind if we add the setting in wk1, but for for wk2 we could try to enable it by default and add the setting later if we see we really needed.
Sounds like a good compromise to me.
Anders Carlsson
Comment 32
2012-02-20 08:51:58 PST
(In reply to
comment #22
)
> (In reply to
comment #21
) > > (From update of
attachment 127700
[details]
[details]) > > Does this really need to be a setting? Isn't there some default "use animations" setting or style property in gtk? When would you ever want to disable this? > > There indeed is a 'gtk-use-animations' property in GtkSettings, which defaults to true. If acceptable to reviewers, the code could just connect to the notify::gtk-use-animation signal of GtkSettings and change the WebCore setting appropriately. > > Though, without the WKPreferences API additions, this would require the connecting and adjusting to the 'gtk-use-animations' value to be performed in WebProcess. But ideally, the GtkSettings instance would be gathered from the specific GtkWidget (the web view), which is a part of UIProcess, and as such AFAIK not reachable from WebProcess. >
Sure, you'd have to keep track of the state in the UI process and send it over to the web process when it changes. We do this for a number of different settings.
Zan Dobersek
Comment 33
2012-02-21 01:28:55 PST
(In reply to
comment #31
)
> (In reply to
comment #30
) > > It will depend on every widget, I think tree view, icon view, text view will do smooth scrolling, for example. So it's up to use to decide. I don't mind if we add the setting in wk1, but for for wk2 we could try to enable it by default and add the setting later if we see we really needed. > > Sounds like a good compromise to me.
Very well, this actually requires no changes to be made in any part of WebKit2. Patch incoming.
Zan Dobersek
Comment 34
2012-02-21 01:31:33 PST
Created
attachment 127937
[details]
Patch
Zan Dobersek
Comment 35
2012-02-22 06:37:45 PST
(In reply to
comment #34
)
> Created an attachment (id=127937) [details] > Patch
Contains wrong 'Since' version in signal definition, will upload the correct one.
Zan Dobersek
Comment 36
2012-02-22 07:33:34 PST
Created
attachment 128212
[details]
Patch
Martin Robinson
Comment 37
2012-02-24 13:14:08 PST
Comment on
attachment 128212
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=128212&action=review
> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:964 > + * Since: 1.7.90
This should probably be 1.9.0 now, since we've already branched for stable.
Carlos Garcia Campos
Comment 38
2012-02-25 01:52:30 PST
(In reply to
comment #37
)
> (From update of
attachment 128212
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=128212&action=review
> > > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:964 > > + * Since: 1.7.90 > > This should probably be 1.9.0 now, since we've already branched for stable.
1.10 I guess, 1.9.x will be the unstable versions of the next release cycle.
Zan Dobersek
Comment 39
2012-02-25 02:04:30 PST
(In reply to
comment #38
)
> (In reply to
comment #37
) > > (From update of
attachment 128212
[details]
[details]) > > View in context:
https://bugs.webkit.org/attachment.cgi?id=128212&action=review
> > > > > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:964 > > > + * Since: 1.7.90 > > > > This should probably be 1.9.0 now, since we've already branched for stable. > > 1.10 I guess, 1.9.x will be the unstable versions of the next release cycle.
There's plenty of places where an unstable version is given as the 'Since:' value. This will be a part of the next unstable release (1.9.0) and subsequently of the next stable release as well (1.10.0). All the 'Since:' tags should either have a value of an unstable release (1.7.5, 1.9.0, 1.9.1 etc.), stating in which release exactly API was added, or a value of a stable release (1.6, 1.8, 1.10 etc.), stating in what stable release API was first present. AFAIK WebKitGtk+ is mostly if not completely using the first approach.
Carlos Garcia Campos
Comment 40
2012-02-25 04:52:32 PST
(In reply to
comment #39
)
> (In reply to
comment #38
) > > (In reply to
comment #37
) > > > (From update of
attachment 128212
[details]
[details] [details]) > > > View in context:
https://bugs.webkit.org/attachment.cgi?id=128212&action=review
> > > > > > > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:964 > > > > + * Since: 1.7.90 > > > > > > This should probably be 1.9.0 now, since we've already branched for stable. > > > > 1.10 I guess, 1.9.x will be the unstable versions of the next release cycle. > > There's plenty of places where an unstable version is given as the 'Since:' value. This will be a part of the next unstable release (1.9.0) and subsequently of the next stable release as well (1.10.0).
I think that comes from the time were webkitgtk did follow the gnome release cycle with stable/unstable releases, I'm not sure, though.
> All the 'Since:' tags should either have a value of an unstable release (1.7.5, 1.9.0, 1.9.1 etc.), stating in which release exactly API was added, or a value of a stable release (1.6, 1.8, 1.10 etc.), stating in what stable release API was first present. > > AFAIK WebKitGtk+ is mostly if not completely using the first approach.
So, maybe we should consider changing it.
Martin Robinson
Comment 41
2012-02-25 10:46:44 PST
(In reply to
comment #40
)
> > AFAIK WebKitGtk+ is mostly if not completely using the first approach. > > So, maybe we should consider changing it.
I agree that we should consider changing it, but probably it should be handled separately from this bug. If such a change it made, we can go back and fix all existing Since: tags.
Zan Dobersek
Comment 42
2012-03-02 02:05:57 PST
Created
attachment 129852
[details]
Updated patch Since: tag updated to 1.9.0
Carlos Garcia Campos
Comment 43
2012-03-02 02:25:24 PST
(In reply to
comment #42
)
> Created an attachment (id=129852) [details] > Updated patch > > Since: tag updated to 1.9.0
I still think we shouldn't use unstable version numbers in Since: tags. If this patrch will be merged into 1.8, this should be Since: 1.8 otherwise it should be 1.10
WebKit Review Bot
Comment 44
2012-03-02 10:04:15 PST
Comment on
attachment 129852
[details]
Updated patch Clearing flags on attachment: 129852 Committed
r109584
: <
http://trac.webkit.org/changeset/109584
>
WebKit Review Bot
Comment 45
2012-03-02 10:04:23 PST
All reviewed patches have been landed. Closing bug.
Zan Dobersek
Comment 46
2012-03-02 10:07:07 PST
(In reply to
comment #43
)
> (In reply to
comment #42
) > > Created an attachment (id=129852) [details] [details] > > Updated patch > > > > Since: tag updated to 1.9.0 > > I still think we shouldn't use unstable version numbers in Since: tags. If this patrch will be merged into 1.8, this should be Since: 1.8 otherwise it should be 1.10
Should I open a new bug covering this issue?
Martin Robinson
Comment 47
2012-03-02 10:29:09 PST
(In reply to
comment #46
)
> Should I open a new bug covering this issue?
Yeah, as I suggested above, deciding what versions to use in Since tags is not a question that this bug needs to solve and shouldn't block this patch. Probably it deserves a short note to the mailing list to gather opinions. It's such a minor issue that hopefully it shouldn't be too controversial. :) Once we decide what to do, we can open a bug to fix all the existing Since tags. It's totally orthoganol to smooth scrolling though.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug