[PATCH] Implement move surface key bindings.
Scott Moreau
oreaus at gmail.com
Fri Feb 3 02:10:22 PST 2012
From: Scott Moreau <oreaus at gmail.com>
Date: Fri, Feb 3, 2012 at 2:58 AM
Subject: Re: [PATCH] Implement move surface key bindings.
To: Pekka Paalanen <ppaalanen at gmail.com>
On Fri, Feb 3, 2012 at 1:29 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 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()?
>
Because move->dx is a negative offset. (see below)
>
> > + 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.
>
Indeed.
>
> > + 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().
>
See comments below.
>
> > + }
> > +}
> > +
> > 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?
>
> Regarding the latter, it is intended behavior. So far as the former is
concerned, I did not think about it but it does indeed have the same effect
as a key grab. I assume this is not desired behavior.
> 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().
>
Yes, you are right. This will have to be adjusted appropriately.
>
> > + 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.
>
I was unaware of this function and it seems to make more sense.
>
> 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.
>
More information about the wayland-devel
mailing list