[PATCH weston 3/6] libweston: Introduce input-timestamps support code

[Pekka Paalanen]

On Wed, 20 Dec 2017 16:17:58 +0200
Alexandros Frantzis <alexandros.frantzis at 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.frantzis at 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.paalanen at 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_ at 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_ at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180119/01d26216/attachment.sig>