GNOME Bugzilla – Bug 707560
Event filter
Last modified: 2013-11-14 19:34:25 UTC
In the Wayland port of Mutter, we need to have a way to hook into all Clutter events so that we can forward them on to the Wayland clients. It also needs to update the cursor position because that is now drawn by Mutter rather than an external display server. We need to see the events even if there is a grab in place. Previously this was accomplished by installing a signal handler for the captured-event signal on the stage. To make it work with grabs it also needed to install an signal emission hook on the event signal. This isn't very nice so I think it would be good to have a way to hook into the events at a lower level, similar to gdk_window_add_filter in GDK.
Created attachment 254176 [details] [review] Add API to install an event filter This adds clutter_event_add/remove_filter which adds a callback function which will receive all Clutter events just before the event signal is emitted for them. The event filter will be invoked regardless of any grabs or captures. This will be used by Mutter which wants to access the events at a lower level then the event bubbling mechanism. It needs to see all mouse motion events even if there is a grab in place.
As you say in the docs, the event may not have a source actor yet, and this is bad for mutter (because we use the source actor to know which surface should receive events). How were you planning to implement that? Always force a double pick?
Review of attachment 254176 [details] [review]: ::: clutter/clutter-event.c @@ +63,3 @@ +{ + ClutterEventFilterFunc func; + gpointer user_data; missing a GDestroyNotify @@ +1685,3 @@ + * @user_data: A data pointer to pass to the function. + * + I'd add a nullable ClutterStage argument, so you can filter events for a specific stage — similarly to gdk_window_add_event_filter(). @@ +1689,3 @@ + * emitted for the event and it will take precedence over any grabs. + * Note that the event might not yet have a source actor associated + this is kind of problematic, though. if you want to get the actor, you'd have to call clutter_stage_get_actor_at_pos(). then, if you don't stop the event propagation, it means doing another pick(). granted, this is kind of a corner case, and looking at the event processing code, I don't really see a case in which the event is processed without a source actor assigned. are you being overly conservative, or is there an actual case where this can happen? @@ +1693,3 @@ + * event. + * + return TRUE; instead of TRUE/FALSE, the constants should be CLUTTER_EVENT_PROPAGATE and CLUTTER_EVENT_STOP. @@ +1699,3 @@ +void +clutter_event_add_filter (ClutterEventFilterFunc func, + return FALSE; this function should come with a GDestroyNotify, otherwise it's unbindable. @@ +1737,3 @@ + { + context->event_filters = + */ needs to call the GDestroyNotify function, if one is set. ::: clutter/clutter-event.h @@ +414,3 @@ + * Since: 1.16 + */ + * clutter_event_add_filter(). I'd probably add the stage as a parameter, here. @@ +424,3 @@ void clutter_event_put (const ClutterEvent *event); +void clutter_event_add_filter (ClutterEventFilterFunc func, missing CLUTTER_AVAILABLE_IN_1_16 annotation. @@ +426,3 @@ +void clutter_event_add_filter (ClutterEventFilterFunc func, + gpointer user_data); +void clutter_event_remove_filter (ClutterEventFilterFunc func, missing CLUTTER_AVAILABLE_IN_1_16 annotation.
Review of attachment 254176 [details] [review]: the patch also needs to update clutter.symbols with the newly added functions.
(In reply to comment #2) > As you say in the docs, the event may not have a source actor yet, and this is > bad for mutter (because we use the source actor to know which surface should > receive events). > How were you planning to implement that? Always force a double pick? Ah, oops, I actually changed the patch so that it would run the filters after the pick so the source field should always be up to date but I forgot to update the documentation. I will make a second version of the patch which removes that comment and I'll also do all of ebassi's tweaks.
Created attachment 259830 [details] [review] Add API to install an event filter This adds clutter_event_add/remove_filter which adds a callback function which will receive all Clutter events just before the event signal is emitted for them. The event filter will be invoked regardless of any grabs or captures. This will be used by Mutter which wants to access the events at a lower level then the event bubbling mechanism. It needs to see all mouse motion events even if there is a grab in place.
Created attachment 259831 [details] [review] Add API to install an event filter This adds clutter_event_add/remove_filter which adds a callback function which will receive all Clutter events just before the event signal is emitted for them. The event filter will be invoked regardless of any grabs or captures. This will be used by Mutter which wants to access the events at a lower level then the event bubbling mechanism. It needs to see all mouse motion events even if there is a grab in place.
Review of attachment 259831 [details] [review]: looks okay now, except for some documentation and annotation issues that need to be addressed before pushing to the clutter-1.18 branch. ::: clutter/clutter-event.c @@ +1767,3 @@ + * emitted for the event and it will take precedence over any grabs. + * + this should be: Return value: an identifier for the event filter, to be used with clutter_event_remove_filter() ::: clutter/clutter-event.h @@ +414,3 @@ + * clutter_event_add_filter(). + * + * @user_data: the data pointer passed to clutter_event_add_filter() TRUE should be CLUTTER_EVENT_STOP, and FALSE should be CLUTTER_EVENT_PROPAGATE. @@ +418,3 @@ + * functions and prevents the signal emission for the event. + * + * A function pointer type used by event filters that are added with Since: 1.18 @@ +430,3 @@ void clutter_event_put (const ClutterEvent *event); +CLUTTER_AVAILABLE_IN_1_16 annotation should be CLUTTER_AVAILABLE_IN_1_18 @@ +435,3 @@ + GDestroyNotify notify, + gpointer user_data); +CLUTTER_AVAILABLE_IN_1_16 same as above.
Attachment 259831 [details] pushed as 7029267 - Add API to install an event filter Pushed with suggested fixes.