[PATCH weston v3 3/3] Introduce wl_relative_pointer interface
Derek Foreman
derekf at osg.samsung.com
Wed Oct 7 11:32:35 PDT 2015
On 29/07/15 01:39 AM, Jonas Ådahl wrote:
> A wl_relative_pointer object is an extension to the wl_pointer interface
> only used for emitting relative pointer events. It will only emit events
> when the parent pointer has focus.
>
> To get a relative pointer object, use the get_relative_pointer request
> of the global wl_relative_pointer_manager object. When stabilizing it
> might make more sense to just add it to wl_seat instead of having a
> single use global interface.
>
> All interface names are currently prefixed with underscore in order to
> avoid any future conflicts with stable protocol.
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
Some comments below, and this series doesn't merge anymore - do you have
time to rebase it?
As far as I'm concerned, the first 2 patches in the series are good to
land (once they compile again...)
This one's
Acked-by: Derek Foreman <derekf at osg.samsung.com>
Would really like to see Daniel's review, time permitting.
I think it would be nice to land this in the same release cycle (ie:
this one) as pointer confinement, because I think the two features
really go hand in hand.
> ---
>
> Changes since v2:
>
> Time stamps are now 64 bit and in microseconds. Since Wayland can't
> represent 64 bit unsigned integers, it is split into two 32 bit unsigned
> integers, one for each half.
>
> The other change is that there is no more manual resource moving around
> as focus resources are now tracked with weston_pointer_client.
>
>
> Makefile.am | 7 +-
> protocol/relative-pointer.xml | 129 +++++++++++++++++++++++++++
> src/compositor.c | 3 +
> src/compositor.h | 4 +
> src/input.c | 197 +++++++++++++++++++++++++++++++++++++-----
> 5 files changed, 317 insertions(+), 23 deletions(-)
> create mode 100644 protocol/relative-pointer.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index 52c736c..af17e7e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -109,7 +109,9 @@ nodist_weston_SOURCES = \
> protocol/presentation_timing-protocol.c \
> protocol/presentation_timing-server-protocol.h \
> protocol/scaler-protocol.c \
> - protocol/scaler-server-protocol.h
> + protocol/scaler-server-protocol.h \
> + protocol/relative-pointer-protocol.c \
> + protocol/relative-pointer-server-protocol.h
>
> BUILT_SOURCES += $(nodist_weston_SOURCES)
>
> @@ -1316,7 +1318,8 @@ EXTRA_DIST += \
> protocol/presentation_timing.xml \
> protocol/scaler.xml \
> protocol/ivi-application.xml \
> - protocol/ivi-hmi-controller.xml
> + protocol/ivi-hmi-controller.xml \
> + protocol/relative-pointer.xml
>
> #
> # manual test modules in tests subdirectory
> diff --git a/protocol/relative-pointer.xml b/protocol/relative-pointer.xml
> new file mode 100644
> index 0000000..dd993e4
> --- /dev/null
> +++ b/protocol/relative-pointer.xml
> @@ -0,0 +1,129 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="relative_pointer">
> +
> + <copyright>
> + Copyright © 2014 Jonas Ådahl
> + Copyright © 2015 Red Hat Inc.
> +
> + Permission is hereby granted, free of charge, to any person obtaining a
> + copy of this software and associated documentation files (the "Software"),
> + to deal in the Software without restriction, including without limitation
> + the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + and/or sell copies of the Software, and to permit persons to whom the
> + Software is furnished to do so, subject to the following conditions:
> +
> + The above copyright notice and this permission notice (including the next
> + paragraph) shall be included in all copies or substantial portions of the
> + Software.
> +
> + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + DEALINGS IN THE SOFTWARE.
> + </copyright>
> +
> + <interface name="_wl_relative_pointer_manager" version="1">
Should there be a comment somewhere in here that this should probably be
part of wl_seat?
> + <description summary="get relative pointer objects">
> + A global interface used for getting the relative pointer object for a
> + given seat.
> +
> + Warning! The protocol described in this file is experimental. Each version
> + of this protocol should be considered incompatible with any other version,
> + and a client binding to a version different to the one advertised will be
> + terminated. Once the protocol is declared stable, backward compatibility
> + is guaranteed, the '_' prefix will be removed from the name and the
> + version will be reset to 1.
> + </description>
> +
> + <request name="get_relative_pointer">
> + <description summary="get a relative pointer object">
> + Create a relative pointer interface given a wl_pointer object. See
> + the wl_relative_pointer interface for more details.
> + </description>
> +
> + <arg name="id" type="new_id" interface="_wl_relative_pointer"/>
> + <arg name="pointer" type="object" interface="wl_pointer"/>
Really this is the only reason I acked instead of r-b, and it's probably
just due to my own misunderstandings, but:
Why are we passing a pointer object here instead of the seat? If a game
wants relative pointer data it'll likely not want absolute pointer data,
so why does it have to bind an absolute pointer (and receive events on
it?) before it can get the relative stuff it's actually going to use?
Can the "parent" wl_pointer be safely released once the relative one is
acquired?
> + </request>
> + </interface>
> +
> + <interface name="_wl_relative_pointer" version="1">
> + <description summary="relative pointer object">
> + A wl_relative_pointer object is an extension to the wl_pointer interface
> + used for emitting relative pointer events. It shares the same focus as
> + wl_pointer objects of the same seat and will only emit events when it
> + has focus.
> + </description>
> +
> + <request name="release" type="destructor">
> + <description summary="release the relative pointer object"/>
> + </request>
> +
> + <event name="relative_motion">
> + <description summary="relative pointer motion">
> + Relative x/y pointer motion in from the pointer of the seat associated
> + with this object.
> +
> + A relative motion is in the same dimension as regular wl_pointer motion
> + events, except they do not represent an absolute position. For example,
> + moving a pointer from (x, y) to (x', y') would have the equivalent
> + relative motion (x' - x, y' - y). If a pointer motion caused the
> + absolute pointer position to be clipped by for example the edge of the
> + monitor, the relative motion is unaffected by the clipping and will
> + represent the unclipped motion.
> +
> + This event also contains non-accelerated motion deltas. The
> + non-accelerated delta is, when applicable, the regular pointer motion
> + delta as it was before having applied motion acceleration
> + transformations. The compositor will have applied the same processing
> + (such as normalization) meaning the events will have roughly the same
> + magnitude as accelerated motion events.
> +
> + Note that the non-accelerated delta does not represent 'raw' events as
> + they were read from some device. Pointer motion acceleration is device-
> + and configuration-specific and non-accelerated deltas and accelerated
> + deltas may have the same value on some devices.
> +
> + Relative motions are not coupled to wl_pointer.motion events, and can
> + be sent in combination with such events, but also independently. There
> + may also be scenarious where wl_pointer.motion is sent, but there is no
> + relative motion. The order of an absolute and relative motion event
> + originating from the same physical motion is not guaranteed.
> +
> + The motion vectors are encoded as double fixed point values.
> +
> + A double fixed point value is a 64 bit data type encoded as two separate
> + signed 32 bit integers. The integral part of the value is stored in one
> + of the integers and the fractional part in the other.
> +
> + If the client needs button events, it can receive them from a wl_pointer
> + object of the same seat that the wl_relative_pointer object is
> + associated with.
> + </description>
> +
> + <arg name="utime_most" type="uint"
> + summary="32 most significant bits of a 64 bit timestamp with microsecond granularity"/>
> + <arg name="utime_least" type="uint"
The presentation timing extension also splits a 64-bit value up like
this, but there it's "tv_sec_hi" and "tv_sec_lo"
Personally (and deep inside I realize this is pedantic bike shed
painting...) I'd like to see consistency in how we name split 64 bit
types...
> + summary="32 least significant bits of a 64 bit timestamp with microsecond granularity"/>
> + <arg name="dx_int" type="int"
> + summary="integral part of the x component of the motion vector"/>
> + <arg name="dx_frac" type="int"
> + summary="fractional part of the x component of the motion vector"/>
> + <arg name="dy_int" type="int"
> + summary="integral part of the y component of the motion vector"/>
> + <arg name="dy_frac" type="int"
> + summary="fractional part of the y component of the motion vector"/>
> + <arg name="dx_unaccel_int" type="int"
> + summary="integral part of the x component of the unaccelerated motion vector"/>
> + <arg name="dx_unaccel_frac" type="int"
> + summary="fractional part of the x component of the unaccelerated motion vector"/>
> + <arg name="dy_unaccel_int" type="int"
> + summary="integral part of the y component of the unaccelerated motion vector"/>
> + <arg name="dy_unaccel_frac" type="int"
> + summary="fractional part of the y component of the unaccelerated motion vector"/>
> + </event>
> + </interface>
> +
> +</protocol>
> diff --git a/src/compositor.c b/src/compositor.c
> index ce1dacd..8ffa976 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -4503,6 +4503,9 @@ weston_compositor_create(struct wl_display *display, void *user_data)
> ec, bind_presentation))
> goto fail;
>
> + if (weston_input_init(ec) != 0)
> + goto fail;
> +
> wl_list_init(&ec->view_list);
> wl_list_init(&ec->plane_list);
> wl_list_init(&ec->layer_list);
> diff --git a/src/compositor.h b/src/compositor.h
> index 20c1dd3..faece84 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -334,6 +334,7 @@ struct weston_pointer_client {
> struct wl_list link;
> struct wl_client *client;
> struct wl_list pointer_resources;
> + struct wl_list relative_pointer_resources;
> };
>
> struct weston_pointer {
> @@ -1603,6 +1604,9 @@ int
> noop_renderer_init(struct weston_compositor *ec);
>
> int
> +weston_input_init(struct weston_compositor *compositor);
> +
> +int
> backend_init(struct weston_compositor *c,
> int *argc, char *argv[],
> struct weston_config *config);
> diff --git a/src/input.c b/src/input.c
> index 3338fdb..c19528d 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -36,7 +36,9 @@
>
> #include "shared/helpers.h"
> #include "shared/os-compatibility.h"
> +#include "shared/util.h"
> #include "compositor.h"
> +#include "protocol/relative-pointer-server-protocol.h"
>
> static void
> empty_region(pixman_region32_t *region)
> @@ -56,6 +58,7 @@ weston_pointer_client_create(struct wl_client *client)
>
> pointer_client->client = client;
> wl_list_init(&pointer_client->pointer_resources);
> + wl_list_init(&pointer_client->relative_pointer_resources);
>
> return pointer_client;
> }
> @@ -69,7 +72,9 @@ weston_pointer_client_destroy(struct weston_pointer_client *pointer_client)
> static bool
> weston_pointer_client_is_empty(struct weston_pointer_client *pointer_client)
> {
> - return wl_list_empty(&pointer_client->pointer_resources);
> + return
> + wl_list_empty(&pointer_client->pointer_resources) &&
> + wl_list_empty(&pointer_client->relative_pointer_resources);
> }
>
> static struct weston_pointer_client *
> @@ -140,6 +145,49 @@ static void unbind_resource(struct wl_resource *resource)
> }
>
> WL_EXPORT void
> +weston_pointer_motion_to_abs(struct weston_pointer *pointer,
> + struct weston_pointer_motion_event *event,
> + wl_fixed_t *x, wl_fixed_t *y)
> +{
> + if (event->mask & WESTON_POINTER_MOTION_ABS) {
> + *x = wl_fixed_from_double(event->x);
> + *y = wl_fixed_from_double(event->y);
> + } else if (event->mask & WESTON_POINTER_MOTION_REL) {
> + *x = pointer->x + wl_fixed_from_double(event->dx);
> + *y = pointer->y + wl_fixed_from_double(event->dy);
> + } else {
> + assert(!"invalid motion event");
> + *x = *y = 0;
> + }
> +}
> +
> +static bool
> +weston_pointer_motion_to_rel(struct weston_pointer *pointer,
> + struct weston_pointer_motion_event *event,
> + double *dx, double *dy,
> + double *dx_unaccel, double *dy_unaccel)
> +{
> + if (event->mask & WESTON_POINTER_MOTION_REL &&
> + event->mask & WESTON_POINTER_MOTION_REL_NOACCEL) {
> + *dx = event->dx;
> + *dy = event->dy;
> + *dx_unaccel = event->dx_unaccel;
> + *dy_unaccel = event->dy_unaccel;
> + return true;
> + } else if (event->mask & WESTON_POINTER_MOTION_REL) {
> + *dx_unaccel = *dx = event->dx;
> + *dy_unaccel = *dy = event->dy;
> + return true;
> + } else if (event->mask & WESTON_POINTER_MOTION_REL_NOACCEL) {
> + *dx_unaccel = *dx = event->dx_unaccel;
> + *dy_unaccel = *dy = event->dy_unaccel;
> + return true;
> + } else {
> + return false;
> + }
> +}
> +
> +WL_EXPORT void
> weston_seat_repick(struct weston_seat *seat)
> {
> const struct weston_pointer *pointer = seat->pointer;
> @@ -255,6 +303,52 @@ default_grab_pointer_focus(struct weston_pointer_grab *grab)
> }
>
> static void
> +weston_pointer_send_relative_motion(struct weston_pointer *pointer,
> + uint32_t time,
> + struct weston_pointer_motion_event *event)
> +{
> + uint64_t time_usec;
> + double dx, dy, dx_unaccel, dy_unaccel;
> + int32_t dx_int, dx_frac;
> + int32_t dy_int, dy_frac;
> + int32_t dx_unaccel_int, dx_unaccel_frac;
> + int32_t dy_unaccel_int, dy_unaccel_frac;
> + struct wl_list *resource_list;
> + struct wl_resource *resource;
> +
> + if (!pointer->focus_client)
> + return;
> +
> + if (!weston_pointer_motion_to_rel(pointer, event,
> + &dx, &dy,
> + &dx_unaccel, &dy_unaccel))
> + return;
> +
> + resource_list = &pointer->focus_client->relative_pointer_resources;
> + time_usec = event->time_usec;
> + if (time_usec == 0)
> + time_usec = time * 1000ULL;
> + wl_double_fixed_from_double(dx, &dx_int, &dx_frac);
> + wl_double_fixed_from_double(dy, &dy_int, &dy_frac);
Do you have a patch somewhere for wl_double_fixed_from_double?
> + wl_double_fixed_from_double(dx_unaccel,
> + &dx_unaccel_int,
> + &dx_unaccel_frac);
> + wl_double_fixed_from_double(dy_unaccel,
> + &dy_unaccel_int,
> + &dy_unaccel_frac);
> + wl_resource_for_each(resource, resource_list) {
> + _wl_relative_pointer_send_relative_motion(
> + resource,
> + (uint32_t) (time_usec >> 32),
> + (uint32_t) time_usec,
> + dx_int, dx_frac,
> + dy_int, dy_frac,
> + dx_unaccel_int, dx_unaccel_frac,
> + dy_unaccel_int, dy_unaccel_frac);
> + }
> +}
> +
> +static void
> default_grab_pointer_motion(struct weston_pointer_grab *grab, uint32_t time,
> struct weston_pointer_motion_event *event)
> {
> @@ -281,6 +375,8 @@ default_grab_pointer_motion(struct weston_pointer_grab *grab, uint32_t time,
> pointer->sx, pointer->sy);
> }
> }
> +
> + weston_pointer_send_relative_motion(pointer, time, event);
> }
>
> static void
> @@ -789,7 +885,6 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
> pointer->sx != sx || pointer->sy != sy)
> refocus = 1;
>
> -
Unrelated, but no big deal. :)
> if (pointer->focus_client) {
> focus_resource_list = &pointer->focus_client->pointer_resources;
> if (!wl_list_empty(focus_resource_list)) {
> @@ -1050,23 +1145,6 @@ weston_pointer_move_to(struct weston_pointer *pointer,
> }
>
> WL_EXPORT void
> -weston_pointer_motion_to_abs(struct weston_pointer *pointer,
> - struct weston_pointer_motion_event *event,
> - wl_fixed_t *x, wl_fixed_t *y)
> -{
> - if (event->mask & WESTON_POINTER_MOTION_ABS) {
> - *x = wl_fixed_from_double(event->x);
> - *y = wl_fixed_from_double(event->y);
> - } else if (event->mask & WESTON_POINTER_MOTION_REL) {
> - *x = pointer->x + wl_fixed_from_double(event->dx);
> - *y = pointer->y + wl_fixed_from_double(event->dy);
> - } else {
> - assert(!"invalid motion event");
> - *x = *y = 0;
> - }
> -}
> -
> -WL_EXPORT void
> weston_pointer_move(struct weston_pointer *pointer,
> struct weston_pointer_motion_event *event)
> {
> @@ -1932,12 +2010,17 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
> wl_client_post_no_memory(client);
> return;
> }
> - wl_resource_set_implementation(cr, &pointer_interface, seat->pointer,
> - unbind_pointer_client_resource);
>
> pointer_client = weston_pointer_ensure_pointer_client(pointer, client);
> + if (!pointer_client) {
> + wl_client_post_no_memory(client);
> + return;
> + }
> +
> wl_list_insert(&pointer_client->pointer_resources,
> wl_resource_get_link(cr));
> + wl_resource_set_implementation(cr, &pointer_interface, seat->pointer,
> + unbind_pointer_client_resource);
Is this an unrelated bug fix?
>
> if (seat->pointer->focus && seat->pointer->focus->surface->resource &&
> wl_resource_get_client(seat->pointer->focus->surface->resource) == client) {
> @@ -2124,6 +2207,67 @@ bind_seat(struct wl_client *client, void *data, uint32_t version, uint32_t id)
> wl_seat_send_name(resource, seat->seat_name);
> }
>
> +static void
> +relative_pointer_release(struct wl_client *client,
> + struct wl_resource *resource)
> +{
> + wl_resource_destroy(resource);
> +}
> +
> +static const struct _wl_relative_pointer_interface relative_pointer_interface = {
> + relative_pointer_release
> +};
> +
> +static void
> +relative_pointer_manager_get_relative_pointer(struct wl_client *client,
> + struct wl_resource *resource,
> + uint32_t id,
> + struct wl_resource *pointer_resource)
> +{
> + struct weston_pointer *pointer =
> + wl_resource_get_user_data(pointer_resource);
> + struct weston_pointer_client *pointer_client;
> + struct wl_resource *cr;
> +
> + cr = wl_resource_create(client, &_wl_relative_pointer_interface,
> + wl_resource_get_version(resource), id);
> + if (cr == NULL) {
> + wl_client_post_no_memory(client);
> + return;
> + }
> +
> + pointer_client = weston_pointer_ensure_pointer_client(pointer, client);
> + if (!pointer_client) {
> + wl_client_post_no_memory(client);
> + return;
> + }
> +
> + wl_list_insert(&pointer_client->relative_pointer_resources,
> + wl_resource_get_link(cr));
> + wl_resource_set_implementation(cr, &relative_pointer_interface,
> + pointer,
> + unbind_pointer_client_resource);
> +}
> +
> +static const struct _wl_relative_pointer_manager_interface relative_pointer_manager = {
> + relative_pointer_manager_get_relative_pointer,
> +};
> +
> +static void
> +bind_relative_pointer_manager(struct wl_client *client, void *data,
> + uint32_t version, uint32_t id)
> +{
> + struct weston_compositor *compositor = data;
> + struct wl_resource *resource;
> +
> + resource = wl_resource_create(client,
> + &_wl_relative_pointer_manager_interface,
> + 1, id);
> + wl_resource_set_implementation(resource, &relative_pointer_manager,
> + compositor,
> + NULL);
> +}
> +
> #ifdef ENABLE_XKBCOMMON
> int
> weston_compositor_xkb_init(struct weston_compositor *ec,
> @@ -2545,3 +2689,14 @@ weston_seat_release(struct weston_seat *seat)
>
> wl_signal_emit(&seat->destroy_signal, seat);
> }
> +
> +int
> +weston_input_init(struct weston_compositor *compositor)
> +{
> + if (!wl_global_create(compositor->wl_display,
> + &_wl_relative_pointer_manager_interface, 1,
> + compositor, bind_relative_pointer_manager))
> + return -1;
> +
> + return 0;
> +}
>
More information about the wayland-devel
mailing list