On Wed, 20 Dec 2017 16:17:58 +0200 Alexandros Frantzis <alexandros.frant...@collabora.com> wrote:
> Introduce code to support the implementation of the > input_timestamps_unstable_v1 protocol in libweston. This commit does not > implement the actual timestamp subscriptions, but lays the foundation so > timestamp subscriptions for keyboard/pointer/touch can be added cleanly > in upcoming commits. > > Signed-off-by: Alexandros Frantzis <alexandros.frant...@collabora.com> > --- > Makefile.am | 4 +- > libweston/input.c | 115 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 118 insertions(+), 1 deletion(-) > Hi, the patch split is very nice for review, but a little less so for merging. We usually try to avoid introducing code that is not referenced in the patch. This one introduces a couple of unused function warnings, fixed in the very next patch. How about squashing this patch with the keyboard patch? The squash would have: Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> > diff --git a/Makefile.am b/Makefile.am > index 0b616c11..823e9845 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -181,7 +181,9 @@ nodist_libweston_@LIBWESTON_MAJOR@_la_SOURCES = > \ > protocol/relative-pointer-unstable-v1-protocol.c \ > protocol/relative-pointer-unstable-v1-server-protocol.h \ > protocol/pointer-constraints-unstable-v1-protocol.c \ > - protocol/pointer-constraints-unstable-v1-server-protocol.h > + protocol/pointer-constraints-unstable-v1-server-protocol.h \ > + protocol/input-timestamps-unstable-v1-protocol.c \ > + protocol/input-timestamps-unstable-v1-server-protocol.h > > BUILT_SOURCES += $(nodist_libweston_@LIBWESTON_MAJOR@_la_SOURCES) > > diff --git a/libweston/input.c b/libweston/input.c > index 94a3423a..a682fa94 100644 > --- a/libweston/input.c > +++ b/libweston/input.c > @@ -42,6 +42,7 @@ > #include "compositor.h" > #include "relative-pointer-unstable-v1-server-protocol.h" > #include "pointer-constraints-unstable-v1-server-protocol.h" > +#include "input-timestamps-unstable-v1-server-protocol.h" > > enum pointer_constraint_type { > POINTER_CONSTRAINT_TYPE_LOCK, > @@ -86,6 +87,42 @@ region_init_infinite(pixman_region32_t *region) > UINT32_MAX, UINT32_MAX); > } > > +static void > +send_timestamp(struct wl_resource *resource, > + const struct timespec *time) > +{ > + uint32_t tv_sec_hi, tv_sec_lo, tv_nsec; > + > + timespec_to_proto(time, &tv_sec_hi, &tv_sec_lo, &tv_nsec); > + zwp_input_timestamps_v1_send_timestamp(resource, tv_sec_hi, tv_sec_lo, > + tv_nsec); > +} > + > +static void > +send_timestamps_for_input_resource(struct wl_resource *input_resource, > + struct wl_list *list, > + const struct timespec *time) > +{ > + struct wl_resource *resource; > + > + wl_resource_for_each(resource, list) { > + if (wl_resource_get_user_data(resource) == input_resource) > + send_timestamp(resource, time); I see this as a trade-off between efficiency and code simplicity, avoiding possibly quite many lines of code since none of wl_keyboard, wl_pointer or wl_touch have matching weston structures yet. I agree on not doing premature optimization here and favouring simplicity. OTOH, mouse motion events can go at 1000 Hz rate, I wonder if there is any measurable overhead. I don't think there is any need to worry about that for now though. > + } > +} > + > +static void > +remove_input_resource_from_timestamps(struct wl_resource *input_resource, > + struct wl_list *list) > +{ > + struct wl_resource *resource; > + > + wl_resource_for_each(resource, list) { > + if (wl_resource_get_user_data(resource) == input_resource) > + wl_resource_set_user_data(resource, NULL); > + } > +} > + Thanks, pq
pgp2IyBzf7VtK.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel