[PATCH] Implement move surface key bindings.
Pekka Paalanen
ppaalanen at gmail.com
Fri Feb 3 00:29:54 PST 2012
On Thu, 2 Feb 2012 11:46:17 -0700
Scott Moreau <oreaus at gmail.com> wrote:
> Super+F7 to initiate, arrow keys to move, Enter to accept and Esc to terminate. We need a way to move the cursor to an abitrary position such as a warp_pointer function, instead of just moving the pointer image. We also need a uniform way to set a pointer image outside of toytoolkit.
> ---
Instead of "outside of toytoolkit" I would say "in the compositor",
since that is where we need it, all clients and toolkits are already
covered.
Just thinking out loud, wonder if the compositor should load the cursor
images, or should we invent something wacky, like the desktop-shell
loading the cursor images into surfaces and setting them to the
compositor via special desktop-shell protocol...
How could we animate such cursors... oh that's obvious, the frame
callback would run only when such cursor is used, so desktop-shell
client could drive the animation. After we have converted cursor
images from wl_buffers to wl_surfaces in general. :-P
> src/compositor.c | 4 +-
> src/shell.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 152 insertions(+), 3 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index ab184ac..ab92bd6 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1411,9 +1411,7 @@ notify_key(struct wl_input_device *device,
> *k = key;
> }
>
> - if (device->keyboard_focus_resource)
> - wl_resource_post_event(device->keyboard_focus_resource,
> - WL_INPUT_DEVICE_KEY, time, key, state);
> + device->grab->interface->key(device->grab, time, key, state);
> }
>
> WL_EXPORT void
> diff --git a/src/shell.c b/src/shell.c
> index 168443b..c7e277e 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -114,6 +114,7 @@ struct weston_move_grab {
> struct wl_grab grab;
> struct weston_surface *surface;
> int32_t dx, dy;
> + int32_t origin_x, origin_y;
> };
>
> struct rotate_grab {
> @@ -183,10 +184,73 @@ move_grab_button(struct wl_grab *grab,
> }
> }
>
> +static void
> +move_grab_key(struct wl_grab *grab,
> + uint32_t time, int32_t key, int32_t state)
> +{
> + if (state == 0)
> + return;
> +
> + struct wl_input_device *device = grab->input_device;
> + struct weston_move_grab *move = (struct weston_move_grab *) grab;
> + struct weston_surface *es = move->surface;
> + int32_t x_offset = 0;
> + int32_t y_offset = 0;
> + int increment = 20;
> +
> + switch (key) {
> + case KEY_UP:
> + y_offset -= increment;
> + break;
> + case KEY_LEFT:
> + x_offset -= increment;
> + break;
> + case KEY_RIGHT:
> + x_offset += increment;
> + break;
> + case KEY_DOWN:
> + y_offset += increment;
> + break;
> + case KEY_ENTER:
> + wl_input_device_end_grab(device, time);
> + free(grab);
> + return;
> + case KEY_ESC:
> + weston_surface_configure(es,
> + move->origin_x,
> + move->origin_y,
> + es->geometry.width, es->geometry.height);
> + /* FIXME: Actually warp pointer instead
> + * of just moving the cursor image */
> + notify_motion(device, time,
> + abs(move->dx) + es->geometry.x,
> + abs(move->dy) + es->geometry.y);
abs()? That looks odd. Why abs()?
> + wl_input_device_end_grab(device, time);
> + free(grab);
> + return;
> + default:
> + return;
> + }
> +
> + if (x_offset || y_offset) {
Cannot get here with both zero, so no need to test.
> + weston_surface_configure(es,
> + x_offset + es->geometry.x,
> + y_offset + es->geometry.y,
> + es->geometry.width, es->geometry.height);
> + /* FIXME: Actually warp pointer instead
> + * of just moving the cursor image.
> + * Also, need a way to set cursor image. */
> + notify_motion(device, time,
> + abs(move->dx) + es->geometry.x,
> + abs(move->dy) + es->geometry.y);
Another strange abs().
> + }
> +}
> +
> static const struct wl_grab_interface move_grab_interface = {
> noop_grab_focus,
> move_grab_motion,
> move_grab_button,
> + move_grab_key,
> };
What happens, if you start a move by dragging the window title bar with
mouse, and press enter or esc?
What if you click a mouse button, while doing a keyboard move?
Is the behaviour as intended then?
> static int
> @@ -212,6 +276,37 @@ weston_surface_move(struct weston_surface *es,
> return 0;
> }
>
> +static int
> +weston_surface_keyboard_move(struct weston_surface *es,
> + struct weston_input_device *wd, uint32_t time)
> +{
> + struct weston_move_grab *move;
> +
> + move = malloc(sizeof *move);
> + if (!move)
> + return -1;
> +
> + move->grab.interface = &move_grab_interface;
> + move->dx = es->geometry.x - ((es->geometry.width / 2) + es->geometry.x);
This simplifies to move->dx = -es->geometry.width / 2. And since you
seem to take abs() of it everywhere, you could just write
move->dx = es->geometry.width / 2;
and drop the abs() everywhere. Or is this because the pointer move
wants it negative? In that case, just use a minus instead of abs().
> + move->dy = es->geometry.y - ((es->geometry.height / 2) + es->geometry.y);
> + move->origin_x = es->geometry.x;
> + move->origin_y = es->geometry.y;
> + move->surface = es;
> +
> + /* FIXME: Actually warp pointer instead
> + * of just moving the cursor image */
> + notify_motion(&wd->input_device, time,
> + abs(move->dx) + es->geometry.x,
> + abs(move->dy) + es->geometry.y);
If the purpose here is to move the pointer (in global coordinates) to
the center of the surface, you'd better use
weston_surface_to_global(es, width/2, height/2, &x, &y).
weston_surface_{to,from}_global() are the only way to properly convert
between surface-local and global coordinates.
Maybe you would need to use this to compute move->dx,dy?
Move->dx,dy are in global coordinate system, width and height are in
surface-local coordinate system, so you need a conversion somewhere.
> +
> + wl_input_device_start_grab(&wd->input_device, &move->grab, time);
> +
> + wl_input_device_set_keyboard_focus(&wd->input_device,
> + &es->surface, time);
> +
> + return 0;
> +}
> +
> static void
> shell_surface_move(struct wl_client *client, struct wl_resource *resource,
> struct wl_resource *input_resource, uint32_t time)
> @@ -276,10 +371,17 @@ resize_grab_button(struct wl_grab *grab,
> }
> }
>
> +static void
> +resize_grab_key(struct wl_grab *grab,
> + uint32_t time, int32_t key, int32_t state)
> +{
> +}
> +
> static const struct wl_grab_interface resize_grab_interface = {
> noop_grab_focus,
> resize_grab_motion,
> resize_grab_button,
> + resize_grab_key,
> };
>
> static int
> @@ -502,10 +604,17 @@ popup_grab_button(struct wl_grab *grab,
> shsurf->popup.initial_up = 1;
> }
>
> +static void
> +popup_grab_key(struct wl_grab *grab,
> + uint32_t time, int32_t key, int32_t state)
> +{
> +}
> +
> static const struct wl_grab_interface popup_grab_interface = {
> popup_grab_focus,
> popup_grab_motion,
> popup_grab_button,
> + popup_grab_key,
> };
>
> static void
> @@ -917,6 +1026,39 @@ move_binding(struct wl_input_device *device, uint32_t time,
> }
>
> static void
> +move_binding_key(struct wl_input_device *device, uint32_t time,
> + uint32_t key, uint32_t button, uint32_t state, void *data)
> +{
> + if (!device->keyboard_focus_resource)
> + return;
> +
> + struct weston_surface *surface =
> + (struct weston_surface *) device->keyboard_focus;
> + struct shell_surface *shsurf;
> +
> + if (surface == NULL)
> + return;
> +
> + shsurf = get_shell_surface(surface);
> + if (!shsurf)
> + return;
> +
> + switch (shsurf->type) {
> + case SHELL_SURFACE_NONE:
> + case SHELL_SURFACE_PANEL:
> + case SHELL_SURFACE_BACKGROUND:
> + case SHELL_SURFACE_LOCK:
> + case SHELL_SURFACE_SCREENSAVER:
> + return;
> + default:
> + break;
> + }
Just a thought... maybe we should make a helper for detecting
non-manipulable surfaces, so we are guaranteed to get it the same in
every function.
> +
> + weston_surface_keyboard_move(surface,
> + (struct weston_input_device *) device, time);
> +}
> +
> +static void
> resize_binding(struct wl_input_device *device, uint32_t time,
> uint32_t key, uint32_t button, uint32_t state, void *data)
> {
> @@ -1040,10 +1182,17 @@ rotate_grab_button(struct wl_grab *grab,
> }
> }
>
> +static void
> +rotate_grab_key(struct wl_grab *grab,
> + uint32_t time, int32_t key, int32_t state)
> +{
> +}
> +
> static const struct wl_grab_interface rotate_grab_interface = {
> noop_grab_focus,
> rotate_grab_motion,
> rotate_grab_button,
> + rotate_grab_key,
> };
>
> static void
> @@ -1638,6 +1787,8 @@ shell_init(struct weston_compositor *ec)
>
> weston_compositor_add_binding(ec, 0, BTN_LEFT, MODIFIER_SUPER,
> move_binding, shell);
> + weston_compositor_add_binding(ec, KEY_F7, 0, MODIFIER_SUPER,
> + move_binding_key, shell);
> weston_compositor_add_binding(ec, 0, BTN_MIDDLE, MODIFIER_SUPER,
> resize_binding, shell);
> weston_compositor_add_binding(ec, KEY_BACKSPACE, 0,
We already discussed this somewhat in irc, but I will reiterate for the
record.
By adding the key function to the grab interface, we stop sending key
events during grabs. If any changes happen in the key state during a
grab, the client having keyboard focus will go out of sync. For
example, press key 'A' down, press the key binding to start move,
release all keys, press and release Esc to end move. The client will
still see 'A' pressed, I assume.
This is a general problem with keyboard grabs, and requires a solution.
Thanks,
pq
More information about the wayland-devel
mailing list