Hi Martin, Apart some style issues this seems good to me, but i'ts the first time i look into evdev.c, so i might have missed something. I could not reproduce the two issues you describe, with 20 mpv instances running a full hd movie in the background.
2013/3/27 Martin Minarik <[email protected]>: > When the kernel event queue overruns, the evdev.c will: > 1. Skip events until and up to the next SYN_REPORT > 2. Count depressed grab buttons on seat devices. > 3. If the depressed grab button count is different, update > it atomically. If it changed to zero, call grab cancelation. > > Potential pitfalls: > a) During the testing, button isn't counted properly on one device. > This causes grab resets when the system is under heavy load. Under > normal operation it's ok. > b) A mysterious notify_button() call that didn't arrive via evdev. (?) > > The evdev_notify_keyboard_focus() has been reworked to make the > similiarity obvious. (still works exactly as before). The functions > are now almost the same: > > evdev_keys_states_count() > evdev_keys_states_query() > --- > src/compositor.c | 21 ++++++-- > src/compositor.h | 6 ++- > src/evdev.c | 145 > +++++++++++++++++++++++++++++++++++++++++++++--------- > src/evdev.h | 15 ++++++ > 4 files changed, 158 insertions(+), 29 deletions(-) > > diff --git a/src/compositor.c b/src/compositor.c > index d6a45ba..580366c 100644 > --- a/src/compositor.c > +++ b/src/compositor.c > @@ -1715,7 +1715,15 @@ weston_compositor_idle_inhibit(struct > weston_compositor *compositor) > static void > weston_compositor_idle_release(struct weston_compositor *compositor) > { > - compositor->idle_inhibit--; > + if (compositor->idle_inhibit) I'd do "if (compositor->idle_inhibit > 0)" here, to be more consistent with places where you do "if (some_var == $n)", and to make the purpose more evident. There is another occurrence below. > + compositor->idle_inhibit--; > + weston_compositor_wake(compositor); > +} > + > +WL_EXPORT void > +weston_compositor_idle_reset(struct weston_compositor *compositor) > +{ > + compositor->idle_inhibit = 0; > weston_compositor_wake(compositor); > } > > @@ -1867,7 +1875,7 @@ weston_surface_activate(struct weston_surface *surface, > } > > WL_EXPORT void > -notify_button(struct weston_seat *seat, uint32_t time, int32_t button, > +notify_button(struct weston_seat *seat, uint32_t time, uint32_t button, > enum wl_pointer_button_state state) > { > struct weston_compositor *compositor = seat->compositor; > @@ -1886,10 +1894,12 @@ notify_button(struct weston_seat *seat, uint32_t > time, int32_t button, > pointer->grab_x = pointer->x; > pointer->grab_y = pointer->y; > } > - pointer->button_count++; > + if (button == pointer->grab_button) > + pointer->button_count++; > } else { > weston_compositor_idle_release(compositor); > - pointer->button_count--; > + if ((button == pointer->grab_button) && pointer->button_count) > + pointer->button_count--; > } > > weston_compositor_run_button_binding(compositor, seat, time, button, > @@ -1897,7 +1907,8 @@ notify_button(struct weston_seat *seat, uint32_t time, > int32_t button, > > pointer->grab->interface->button(pointer->grab, time, button, state); > > - if (pointer->button_count == 1) > + if ((pointer->button_count == 1) && > + (state == WL_POINTER_BUTTON_STATE_PRESSED)) > pointer->grab_serial = > wl_display_get_serial(compositor->wl_display); > } > diff --git a/src/compositor.h b/src/compositor.h > index 48ec57c..777596c 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -570,7 +570,9 @@ void > notify_motion(struct weston_seat *seat, uint32_t time, > wl_fixed_t x, wl_fixed_t y); > void > -notify_button(struct weston_seat *seat, uint32_t time, int32_t button, > +notify_lag(struct weston_seat *seat, uint32_t time); > +void > +notify_button(struct weston_seat *seat, uint32_t time, uint32_t button, > enum wl_pointer_button_state state); > void > notify_axis(struct weston_seat *seat, uint32_t time, uint32_t axis, > @@ -693,6 +695,8 @@ weston_compositor_run_debug_binding(struct > weston_compositor *compositor, > struct weston_seat *seat, uint32_t time, > uint32_t key, > enum wl_keyboard_key_state state); > +void > +weston_compositor_idle_reset(struct weston_compositor *compositor); > > int > weston_environment_get_fd(const char *env); > diff --git a/src/evdev.c b/src/evdev.c > index d2954b5..7a4966c 100644 > --- a/src/evdev.c > +++ b/src/evdev.c > @@ -60,6 +60,24 @@ evdev_led_update(struct evdev_device *device, enum > weston_led leds) > (void)i; /* no, we really don't care about the return value */ > } > > +struct evdev_dispatch_interface fallback_interface; > +struct evdev_dispatch_interface syn_drop_interface; > + > +static inline void > +evdev_process_syn(struct evdev_device *device, struct input_event *e, int > time) > +{ > + switch (e->code) { > + case SYN_DROPPED: > + if (device->dispatch->interface == &fallback_interface) > + device->dispatch->interface = &syn_drop_interface; > + break; > + case SYN_REPORT: > + default: > + device->pending_events |= EVDEV_SYN; > + break; > + } > +} > + > static inline void > evdev_process_key(struct evdev_device *device, struct input_event *e, int > time) > { > @@ -80,7 +98,6 @@ evdev_process_key(struct evdev_device *device, struct > input_event *e, int time) > e->value ? WL_POINTER_BUTTON_STATE_PRESSED : > WL_POINTER_BUTTON_STATE_RELEASED); > break; > - > default: > notify_key(device->seat, > time, e->code, > @@ -308,7 +325,7 @@ fallback_process(struct evdev_dispatch *dispatch, > evdev_process_key(device, event, time); > break; > case EV_SYN: > - device->pending_events |= EVDEV_SYN; > + evdev_process_syn(device, event, time); > break; > } > } > @@ -324,6 +341,50 @@ struct evdev_dispatch_interface fallback_interface = { > fallback_destroy > }; > > +static void > +syn_drop_process(struct evdev_dispatch *dispatch, > + struct evdev_device *device, > + struct input_event *event, > + uint32_t time) > +{ > + if ((event->code != EV_SYN) || (event->code != SYN_REPORT)) > + return; > + > + if (device->dispatch->interface == &syn_drop_interface) > + device->dispatch->interface = &fallback_interface; > + > + struct weston_seat *seat = device->seat; > + struct wl_pointer *pointer = seat->seat.pointer; > + struct wl_list *devices_list = &device->link; > + > + uint8_t button_count_new = 0; > + evdev_keys_states_count(devices_list, &button_count_new, > + pointer->grab_button, > + pointer->grab_button+1); > + > + if (button_count_new == pointer->button_count) > + return; > + > + int stop_grab = (button_count_new == 0) && (pointer->button_count > > 0); The declaration of stop_grab should go at the top, with the others. > + > + weston_compositor_idle_reset(seat->compositor); > + pointer->button_count = button_count_new; > + if (stop_grab) > + pointer->grab->interface->button(pointer->grab, time, > + pointer->grab_button, > WL_POINTER_BUTTON_STATE_RELEASED); > +} > + > +static void > +syn_drop_destroy(struct evdev_dispatch *dispatch) > +{ > + free(dispatch); > +} > + > +struct evdev_dispatch_interface syn_drop_interface = { > + syn_drop_process, > + syn_drop_destroy > +}; > + > static struct evdev_dispatch * > fallback_dispatch_create(void) > { > @@ -607,44 +668,82 @@ evdev_device_destroy(struct evdev_device *device) > } > > void > -evdev_notify_keyboard_focus(struct weston_seat *seat, > - struct wl_list *evdev_devices) > +evdev_keys_states_count(struct wl_list *evdev_devices, uint8_t *keys_counts, > + uint32_t key_min, uint32_t key_max) > { > struct evdev_device *device; > - struct wl_array keys; > - unsigned int i, set; > - char evdev_keys[(KEY_CNT + 7) / 8]; > - char all_keys[(KEY_CNT + 7) / 8]; > - uint32_t *k; > - int ret; > + uint32_t key_range = key_max - key_min; > > - if (!seat->seat.keyboard) > + if (key_range > KEY_MAX || key_min > KEY_CNT || key_max > KEY_MAX) > return; > > - memset(all_keys, 0, sizeof all_keys); > wl_list_for_each(device, evdev_devices, link) { > - memset(evdev_keys, 0, sizeof evdev_keys); > - ret = ioctl(device->fd, > - EVIOCGKEY(sizeof evdev_keys), evdev_keys); > - if (ret < 0) { > - weston_log("failed to get keys for device %s\n", > - device->devnode); > + unsigned long evdev_keys[NBITS(KEY_CNT)]; > + uint32_t i; > + int ret; These should be outside of the loop, at the top of the function. > + memset(evdev_keys, 0, sizeof(evdev_keys)); > + > + ret = ioctl(device->fd, EVIOCGKEY(KEY_CNT), evdev_keys); > + > + if (ret < 0) > continue; > + > + for (i = 0; i < key_range; i++) { > + int value = TEST_BIT(evdev_keys, (i + key_min)); > + if (value) > + keys_counts[i]++; > } > + } > +} > + > +void > +evdev_keys_states_query(struct wl_list *evdev_devices, > + unsigned long all_keys[NBITS(KEY_CNT)]) > +{ > + struct evdev_device *device; > + > + wl_list_for_each(device, evdev_devices, link) { > + unsigned long evdev_keys[NBITS(KEY_CNT)]; > + uint32_t i; > + int ret; Same here. > + memset(evdev_keys, 0, sizeof(evdev_keys)); > + > + ret = ioctl(device->fd, EVIOCGKEY(KEY_CNT), evdev_keys); > + > + if (ret < 0) > + continue; > + > for (i = 0; i < ARRAY_LENGTH(evdev_keys); i++) > all_keys[i] |= evdev_keys[i]; > } > +} > + > +void > +evdev_keys_depressed_array(struct wl_list *evdev_devices, struct wl_array > *keys) > +{ > + unsigned long all_keys[NBITS(KEY_CNT)]; > + uint32_t i; > + uint32_t *k; > + > + memset(all_keys, 0, sizeof(all_keys)); > + evdev_keys_states_query(evdev_devices, all_keys); > > - wl_array_init(&keys); > for (i = 0; i < KEY_CNT; i++) { > - set = all_keys[i >> 3] & (1 << (i & 7)); > - if (set) { > - k = wl_array_add(&keys, sizeof *k); > + if (TEST_BIT(all_keys,i)) { > + k = wl_array_add(keys, sizeof *k); > *k = i; > } > } > +} > > - notify_keyboard_focus_in(seat, &keys, STATE_UPDATE_AUTOMATIC); > +void > +evdev_notify_keyboard_focus(struct weston_seat *seat, > + struct wl_list *evdev_devices) > +{ > + struct wl_array keys; > > + wl_array_init(&keys); > + evdev_keys_depressed_array(evdev_devices, &keys); > + notify_keyboard_focus_in(seat, &keys, STATE_UPDATE_AUTOMATIC); > wl_array_release(&keys); > } > diff --git a/src/evdev.h b/src/evdev.h > index eb5c868..ad68081 100644 > --- a/src/evdev.h > +++ b/src/evdev.h > @@ -121,7 +121,22 @@ void > evdev_device_destroy(struct evdev_device *device); > > void > +evdev_keys_states_count(struct wl_list *evdev_devices, uint8_t *keys_counts, > + uint32_t key_min, uint32_t key_max); > +void > +evdev_keys_states_query(struct wl_list *evdev_devices, > + unsigned long all_keys[NBITS(KEY_CNT)]); > +void > +evdev_keys_depressed_array(struct wl_list *evdev_devices,struct wl_array > *keys); > + > +void > evdev_notify_keyboard_focus(struct weston_seat *seat, > struct wl_list *evdev_devices); > > +void > +evdev_notify_key_status(struct weston_seat *seat, > + struct wl_list *evdev_devices, > + uint32_t key_min, uint32_t key_max); > + > + > #endif /* EVDEV_H */ > -- > 1.7.10.4 > > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
