GNOME Bugzilla – Bug 746127
Scroll to zoom doesn't work
Last modified: 2015-03-18 11:02:24 UTC
Scroll to zoom stopped working recently. My intuition tells me it's related to that workaround for Clutter / GDK events.
Hi, thanks! This one is tricker. I do not think we do anything at all for this to work in Maps(?) it is all libchamplain, maybe champlain-gtk needs work-around, or this needs to be fixed in Clutter. If this is clutter at all! I do not have time to try this for a couple of days. But if someone could try running with a clutter pre/post the switch to GDK backend to verify that would be appreciated!
As a said in bug 746128, a stop-gap measure would be to call clutter_set_windowing_backend() before initializing Clutter.
This bug belongs on Champlain.
Created attachment 299532 [details] [review] ChamplainView: Handle CLUTTER_SCROLL_SMOOTH Since Clutter changed to the Gdk backend the mouse-scroll to zoom functionality stopped working for Maps. It seems that we now get CLUTTER_SCROLL_SMOOTH for direction in the scroll callback. So lets handle that.
Created attachment 299533 [details] [review] ChamplainView: Handle CLUTTER_SCROLL_SMOOTH Since Clutter changed to the Gdk backend the mouse-scroll to zoom functionality stopped working for Maps. It seems that we now get CLUTTER_SCROLL_SMOOTH for direction in the scroll callback. So lets handle that.
Any chance of a libchamplain release soon?
Jonas, thanks for looking into this. I probably don't have recent-enough version of clutter to test (which one introduces this behaviour?) but the patch looks good. After applying the patch, does it still behave the same way as before (i.e. if you scroll the mouse wheel fast, will it zoom by several zoom levels)? I can prepare the release today evening.
Review of attachment 299533 [details] [review]: ::: champlain/champlain-view.c @@ +394,3 @@ + /* Use a threshhold value to avoid jitter */ + if (fabs(dy) < 0.75) + return FALSE; Regarding this threshold value I have no way of testing it currently as my touchpad doesn't work nicely in F22. @@ +399,3 @@ + zoom_level = priv->zoom_level + 1; + else + zoom_level = priv->zoom_level - 1; These should be swapped I think. With this patch, when I scroll down I zoom in instead of out (as it was previously).
Review of attachment 299533 [details] [review]: ::: champlain/champlain-view.c @@ +387,3 @@ else if (event->direction == CLUTTER_SCROLL_DOWN) zoom_level = priv->zoom_level - 1; + else if (event->direction == CLUTTER_SCROLL_SMOOTH) { Also, libchamplain uses GNU style for block indenting.
Created attachment 299621 [details] [review] ChamplainView: Handle CLUTTER_SCROLL_SMOOTH In latest Clutter more devices reports scroll using the CLUTTER_SCROLL_SMOOTH direction. This means that the scroll to zoom functionality stopped working for Maps. Not handling the CLUTTER_SCROLL_SMOOTH in Champlain has always been a buglet. It gets more serious now that we get CLUTTER_SCROLL_SMOOTH more often.
(In reply to Jiri Techet from comment #7) > Jonas, thanks for looking into this. I probably don't have recent-enough > version of clutter to test (which one introduces this behaviour?) but the > patch looks good. > I am not sure which Clutter introduced this behaviour, might also be libinputs doing. Or it might be switching to the Gdk backend caused more devices capable of giving smooth values. But I guess 1.21.4ish. Maybe you could try forcing the Gdk backend for clutter? By calling: clutter_set_windowing_backend (CLUTTER_WINDOWING_GDK); Before calling clutter_init() or gtk_clutter_init(). > After applying the patch, does it still behave the same way as before (i.e. > if you scroll the mouse wheel fast, will it zoom by several zoom levels)? > Yes that is still the case! > I can prepare the release today evening. Awesome.
Review of attachment 299533 [details] [review]: ::: champlain/champlain-view.c @@ +387,3 @@ else if (event->direction == CLUTTER_SCROLL_DOWN) zoom_level = priv->zoom_level - 1; + else if (event->direction == CLUTTER_SCROLL_SMOOTH) { Yep, thanks! @@ +399,3 @@ + zoom_level = priv->zoom_level + 1; + else + zoom_level = priv->zoom_level - 1; Ah, thanks! Will fix.
(In reply to Jonas Danielsson from comment #11) > (In reply to Jiri Techet from comment #7) > > Jonas, thanks for looking into this. I probably don't have recent-enough > > version of clutter to test (which one introduces this behaviour?) but the > > patch looks good. > > > > I am not sure which Clutter introduced this behaviour, might also be > libinputs doing. Or it might be switching to the Gdk backend caused more > devices capable of giving smooth values. But I guess 1.21.4ish. Strictly speaking, the smooth device handling has been introduced by Clutter 1.10, released three years ago in March 2012; code handling scroll events should have been ported to handle smooth scrolling after that. In the future every platform will likely get smooth scrolling events, since the directional, discrete scrolling events are considered an X11-specific legacy. > Maybe you could try forcing the Gdk backend for clutter? By calling: > > clutter_set_windowing_backend (CLUTTER_WINDOWING_GDK); > > Before calling clutter_init() or gtk_clutter_init(). That is not necessary: the GDK windowing backend is the default, if Clutter has been compiled with it. If you have an older version of Clutter that will help, but you'll probably have other issues with the GDK backend — the reason why we made it the default in 1.21 was that we could finally ensure that there wouldn't be regressions.
Review of attachment 299621 [details] [review]: I've just fixed a warning in a separate commit about using ClutterScrollEvent instead of ClutterEvent in clutter_event_get_scroll_delta(), otherwise the patch looks good (untested on my side though). Thanks.