Re: Initial impressions of the gesture branch

[Carlos Garnacho <carlosg gnome org>]

Hey :),

On lun, 2014-03-31 at 23:10 -0400, Matthias Clasen wrote:
> Here are some unsorted, initial impressions from looking over the
> gesture branch. I've also tried the wip/gesture branches of eog and
> evince - they work quite nicely.

Thanks for the comments! I should add that evince is particularly a good
test bed, it demonstrates better than eog how gesture management works
across a widget hierarchy. A bunch of replies below.

> The class hierarchy I see:
>
> GtkEventController (abstract)
>   GtkGesture (abstract)
>     GtkGestureSingle (abstract)
>       GtkGestureDrag
>       GtkGestureLongPress
>       GtkGestureMultiPress
>       GtkGestureSwipe
>     GtkGestureRotate
>     GtkGestureZoom
>
> What other uses are there for GtkEventController, if the widget api is
> now using GtkGesture ? Is it worth keeping this base class around ?

That's a good point, I guess depends a bit on the future direction. I
thought GtkEventController might be also useful for keyboard
controllers, keybindings could fit as controllers hooked to the capture
phase, 

Other (likely out of GTK+ scope) cases I could think of are key combo
handlers, or something to abstract the handling of directional keys,
eating "key repeat" events, allowing several keys to be simultaneously
pressed, etc.

But the GtkWidget-level helpers in the branch are all around
GdkEventSequence (hence GtkGesture), it was a bit inconsistent to have
those added as GtkEventController, so I figured other possible uses
might need other hooks into GTK+.

Something that comes across my mind lately is that GtkEventController
might hook underneath to the widget (it's a required construct-time
argument anyway), that'd help get rid of this oddity, although maybe not
of the "why GtkEventController?" question :).

> It would be good to write a high-level explanation of how events are
> handled now, highlighting the changes. one way to do it would be to
> update the comment above gtk_main_do_event - it needs updates anyway.

Yes indeed, and probably a big chunk with diagrams in GtkGesture,
documentation in the branch is admittedly half-way...

> Doesn't it do violence to grab semantics if the capture phase runs all
> the way from the top ?

"No" for all traditional event handling (ie. bubbling), captured events
have been kept in private domains so far, that played in my favor here a
bit... 

But this is most nominally to counteract the "perform GTK+ grab while
button is pressed" trend in widgets, that largely defeats gesture
recognition on parents if none of those captured the button/touch press.

OTOH it probably makes sense to mass remove those superfluous GTK+
grabs, and document it as a possible pitfall regarding gesture
recognition.

> The case for ENTER/LEAVE looks at the sensitivity of the grab_widget
> before propagating the event - I think the logic there needs to be
> changed to always propagate, regardless of the grab_widget
> sensitivity.

Hmm, you mean only on the capture phase, right? Looks like the
gtk_widget_event() "bubble" call should still check for sensitivity.

> The long press gesture is always using the default delay - that should
> probably be a property ?

Oh indeed, I pushed some patches to have it rely on a GtkSettings
property.

> The swipe gesture is a subclass of GtkGestureSingle - aren't there
> two-finger swipes, or is that not a useful distinction ?

That's one thing I considered when making all those gestures derive from
GtkGestureSingle, in greater or lesser degree, in the end having
mouse-friendly API in a common place for those won... 

In theory those can be made multi-touch, even though they are optimized
for single touch. That is, you could have two finger swipes, but speed
calculation is only done on one of both sequences (or the bounding box?), 
this at least makes sense for swipe/drag. one thing for sure, multi-touch
GtkGesture[Multi|Long]Press and accuracy are not going to get along...

> GtkPressAndHold should probably be replaced by GtkGestureLongPress.

Oops yes, that's missing :)

> Comparing to ClutterAction, I see ClutterPanAction and
> ClutterTapAction that have no equivalent yet.

Right, would be good to have those for completeness.

> It seems slightly complicated that gtk_widget_add_gesture() takes a
> GtkPropagationPhase as argument. clutter_actor_add_action doesn't have
> that.

Yeah... maybe if I get rid of gtk_widget_add_gesture() as proposed
above, this could be moved to a less often called
gtk_event_controller_set_propagation_phase(), a "bubble" default could
make sense, at least for non-containers.

But due to the way event propagation is prone to be stopped deep in the
stack in the bubble phase, the capture phase is very likely what
containers want. 

Maybe if events propagated unstoppable in the bubble phase we could get
rid of the capture phase, but that'd require deepest widgets to hold off
interaction until they're guaranteed to be the only handlers. This might
compare in the X model to the presence/lack of XI_TouchOwnership event
in the mask.

And GTK_PHASE_NONE is mainly there for compatibility reasons, so you can
use gestures, in eg. signal handlers without external behavioral
changes.

> Should gtk_widget_add_gesture update the event mask ?

It is OR'ed internally, I was hesitant to change the real mask because
it could be hard to get it back to normal in
gtk_widget_remove_gesture(), maybe it can be stored separately, so
widgets transferring gtk_widget_get_events() to their GdkWindow still
get the right thing.

> Does the order of event controllers in a widget matter ?
> (gtk_widget_add_gesture prepends them)

On well behaved usage, it shouldn't matter. widgets doing convoluted
things with gestures may orchestrate the state of GdkEventSequences
through the GtkWidget::sequence-state-changed, and/or
gtk_widget_set_sequence_state(). Essentially, when an event is run
through a widget controllers', every gesture that has its
GdkEventSequence with state claimed/none will handle it.

> It would be good to see an example of a gesture implementation that
> provides feedback/updates while the gesture is in progress - e.g. drag
> up from a spinbutton entry and have the spinbutton tick up while I
> keep holding. Or a long press with a 'timer' animation.

Yeah :), got started on the former.

> What is the ideal end state for this work ? All mouse and touch events
> are handled by event controllers, and there are no more
> button_press/release_event handlers anywhere ?

Ideally it should get as close to that as possible, but one thing worth
noting is that gestures only cater for press/motion*/release
interactions, so there may still be a need for usual event handling in
widgets. 

Also, in practical terms, just attaching to button press may be more
convenient to widgets if they just wanted that, so realistically I would
see both hand-by-hand for long enough...

> Do we envision applications to have custom gestures ? Any plausible
> examples for that ? If yes, it would be good to have an example for a
> custom gesture implementation. If not, we can probably save quite a
> bit of API that is only relevant for subclassing.

Indeed the aim is to provide the most usual set, I guess very little
apps will want to extend that. I guess it can start off as hidden and be
made public if there is a demand?



And my own couple observations/sore points:

* GtkGesture API revolves much around GdkEventSequence, for pointer
events I use a NULL special value, which is oddly most common...

* I feel the GtkWidget::sequence-state-changed signal is a bit busy,
right now it handles sequence state changes both when it happens within
the widget and outside the widget (eg a parent), it should probably be
easier if I split these two in separate signals.


Cheers,
  Carlos