No subject


Fri Feb 3 01:21:28 PST 2012


starting at 0,0 for the upper left corner of the surface. This probably
will be refactored taking into consideration these suggestions.

>
> > +
> > +     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
>

Yes, many good points. As you suggested in irc, we probably should wait to
see what happens at fosdem before proceeding, especially since such
functionality touches on protocol design and other key assets. Thank you
for your timely review, I'll submit a second version of this patch after
getting krh's input as well.

--0015175d04bc52c67304b80c8529
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<br><div class=3D"gmail_quote">From: <b class=3D"gmail_sendername">Scott Mo=
reau</b> <span dir=3D"ltr">&lt;<a href=3D"mailto:oreaus at gmail.com">oreaus at g=
mail.com</a>&gt;</span><br>Date: Fri, Feb 3, 2012 at 2:58 AM<br>Subject: Re=
: [PATCH] Implement move surface key bindings.<br>
To: Pekka Paalanen &lt;<a href=3D"mailto:ppaalanen at gmail.com">ppaalanen at gma=
il.com</a>&gt;<br><br><br><br><br><div class=3D"gmail_quote"><div><div clas=
s=3D"h5">On Fri, Feb 3, 2012 at 1:29 AM, Pekka Paalanen <span dir=3D"ltr">&=
lt;<a href=3D"mailto:ppaalanen at gmail.com" target=3D"_blank">ppaalanen at gmail=
.com</a>&gt;</span> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
<div>On Thu, =A02 Feb 2012 11:46:17 -0700<br>
Scott Moreau &lt;<a href=3D"mailto:oreaus at gmail.com" target=3D"_blank">orea=
us at gmail.com</a>&gt; wrote:<br>
<br>
&gt; Super+F7 to initiate, arrow keys to move, Enter to accept and Esc to t=
erminate. 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.<br>


&gt; ---<br>
<br>
</div>Instead of &quot;outside of toytoolkit&quot; I would say &quot;in the=
 compositor&quot;,<br>
since that is where we need it, all clients and toolkits are already<br>
covered.<br>
<br>
Just thinking out loud, wonder if the compositor should load the cursor<br>
images, or should we invent something wacky, like the desktop-shell<br>
loading the cursor images into surfaces and setting them to the<br>
compositor via special desktop-shell protocol...<br>
How could we animate such cursors... oh that&#39;s obvious, the frame<br>
callback would run only when such cursor is used, so desktop-shell<br>
client could drive the animation. After we have converted cursor<br>
images from wl_buffers to wl_surfaces in general. :-P<br>
<div><div><br>
&gt; =A0src/compositor.c | =A0 =A04 +-<br>
&gt; =A0src/shell.c =A0 =A0 =A0| =A0151 +++++++++++++++++++++++++++++++++++=
+++++++++++++++++++<br>
&gt; =A02 files changed, 152 insertions(+), 3 deletions(-)<br>
&gt;<br>
&gt; diff --git a/src/compositor.c b/src/compositor.c<br>
&gt; index ab184ac..ab92bd6 100644<br>
&gt; --- a/src/compositor.c<br>
&gt; +++ b/src/compositor.c<br>
&gt; @@ -1411,9 +1411,7 @@ notify_key(struct wl_input_device *device,<br>
&gt; =A0 =A0 =A0 =A0 =A0 =A0 =A0 *k =3D key;<br>
&gt; =A0 =A0 =A0 }<br>
&gt;<br>
&gt; - =A0 =A0 if (device-&gt;keyboard_focus_resource)<br>
&gt; - =A0 =A0 =A0 =A0 =A0 =A0 wl_resource_post_event(device-&gt;keyboard_f=
ocus_resource,<br>
&gt; - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0WL_INPUT_DEVICE_KEY, time, key, state);<br>
&gt; + =A0 =A0 device-&gt;grab-&gt;interface-&gt;key(device-&gt;grab, time,=
 key, state);<br>
&gt; =A0}<br>
&gt;<br>
&gt; =A0WL_EXPORT void<br>
&gt; diff --git a/src/shell.c b/src/shell.c<br>
&gt; index 168443b..c7e277e 100644<br>
&gt; --- a/src/shell.c<br>
&gt; +++ b/src/shell.c<br>
&gt; @@ -114,6 +114,7 @@ struct weston_move_grab {<br>
&gt; =A0 =A0 =A0 struct wl_grab grab;<br>
&gt; =A0 =A0 =A0 struct weston_surface *surface;<br>
&gt; =A0 =A0 =A0 int32_t dx, dy;<br>
&gt; + =A0 =A0 int32_t origin_x, origin_y;<br>
&gt; =A0};<br>
&gt;<br>
&gt; =A0struct rotate_grab {<br>
&gt; @@ -183,10 +184,73 @@ move_grab_button(struct wl_grab *grab,<br>
&gt; =A0 =A0 =A0 }<br>
&gt; =A0}<br>
&gt;<br>
&gt; +static void<br>
&gt; +move_grab_key(struct wl_grab *grab,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0uint32_t time, int32_t key, int32_t state=
)<br>
&gt; +{<br>
&gt; + =A0 =A0 if (state =3D=3D 0)<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; +<br>
&gt; + =A0 =A0 struct wl_input_device *device =3D grab-&gt;input_device;<br=
>
&gt; + =A0 =A0 struct weston_move_grab *move =3D (struct weston_move_grab *=
) grab;<br>
&gt; + =A0 =A0 struct weston_surface *es =3D move-&gt;surface;<br>
&gt; + =A0 =A0 int32_t x_offset =3D 0;<br>
&gt; + =A0 =A0 int32_t y_offset =3D 0;<br>
&gt; + =A0 =A0 int increment =3D 20;<br>
&gt; +<br>
&gt; + =A0 =A0 switch (key) {<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case KEY_UP:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 y_offset -=3D increment;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case KEY_LEFT:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 x_offset -=3D increment;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case KEY_RIGHT:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 x_offset +=3D increment;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case KEY_DOWN:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 y_offset +=3D increment;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case KEY_ENTER:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wl_input_device_end_grab(dev=
ice, time);<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 free(grab);<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case KEY_ESC:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 weston_surface_configure(es,=
<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0move-&gt;origin_x,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0move-&gt;origin_y,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0es-&gt;geometry.width, es-&gt;geometry.height);<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* FIXME: Actually warp poin=
ter instead<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* of just moving the curs=
or image */<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 notify_motion(device, time,<=
br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 abs(move-&gt;dx) + es-&gt;geometry.x,<b=
r>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 abs(move-&gt;dy) + es-&gt;geometry.y);<=
br>
<br>
</div></div>abs()? That looks odd. Why abs()?<br></blockquote></div></div><=
div><br>Because=A0 move-&gt;dx is a negative offset. (see below)<br></div><=
div class=3D"im"><blockquote class=3D"gmail_quote" style=3D"margin:0pt 0pt =
0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<div><br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wl_input_device_end_grab(dev=
ice, time);<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 free(grab);<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 default:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; + =A0 =A0 }<br>
&gt; +<br>
&gt; + =A0 =A0 if (x_offset || y_offset) {<br>
<br>
</div>Cannot get here with both zero, so no need to test.<br></blockquote><=
div>=A0</div></div><div>Indeed. <br></div><div class=3D"im"><blockquote cla=
ss=3D"gmail_quote" style=3D"margin:0pt 0pt 0pt 0.8ex;border-left:1px solid =
rgb(204,204,204);padding-left:1ex">


<div><br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 weston_surface_configure(es,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0x_offset + es-&gt;geometry.x,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0y_offset + es-&gt;geometry.y,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0es-&gt;geometry.width, es-&gt;geometry.height);<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 /* FIXME: Actually warp pointer instead<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0* of just moving the cursor image.<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0* Also, need a way to set cursor image. *=
/<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 notify_motion(device, time,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 abs(move-&gt;dx) + es-&gt;geometry.x,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 abs(move-&gt;dy) + es-&gt;geometry.y);<br>
<br>
</div>Another strange abs().<br></blockquote><div>=A0</div></div><div>See c=
omments below. <br></div><div class=3D"im"><blockquote class=3D"gmail_quote=
" style=3D"margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);=
padding-left:1ex">

<div><br>
&gt; + =A0 =A0 }<br>
&gt; +}<br>
&gt; +<br>
&gt; =A0static const struct wl_grab_interface move_grab_interface =3D {<br>
&gt; =A0 =A0 =A0 noop_grab_focus,<br>
&gt; =A0 =A0 =A0 move_grab_motion,<br>
&gt; =A0 =A0 =A0 move_grab_button,<br>
&gt; + =A0 =A0 move_grab_key,<br>
&gt; =A0};<br>
<br>
</div>What happens, if you start a move by dragging the window title bar wi=
th<br>
mouse, and press enter or esc?<br>
What if you click a mouse button, while doing a keyboard move?<br>
Is the behaviour as intended then?<br>
<div><br></div></blockquote></div><div>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 des=
ired behavior.<br>

<br></div><div class=3D"im"><blockquote class=3D"gmail_quote" style=3D"marg=
in:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1e=
x"><div>
&gt; =A0static int<br>
&gt; @@ -212,6 +276,37 @@ weston_surface_move(struct weston_surface *es,<br=
>
&gt; =A0 =A0 =A0 return 0;<br>
&gt; =A0}<br>
&gt;<br>
&gt; +static int<br>
&gt; +weston_surface_keyboard_move(struct weston_surface *es,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct weston_input_device *wd, uint=
32_t time)<br>
&gt; +{<br>
&gt; + =A0 =A0 struct weston_move_grab *move;<br>
&gt; +<br>
&gt; + =A0 =A0 move =3D malloc(sizeof *move);<br>
&gt; + =A0 =A0 if (!move)<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 return -1;<br>
&gt; +<br>
&gt; + =A0 =A0 move-&gt;grab.interface =3D &amp;move_grab_interface;<br>
&gt; + =A0 =A0 move-&gt;dx =3D es-&gt;geometry.x - ((es-&gt;geometry.width =
/ 2) + es-&gt;geometry.x);<br>
<br>
</div>This simplifies to move-&gt;dx =3D -es-&gt;geometry.width / 2. And si=
nce you<br>
seem to take abs() of it everywhere, you could just write<br>
 =A0 =A0 =A0 =A0move-&gt;dx =3D es-&gt;geometry.width / 2;<br>
and drop the abs() everywhere. Or is this because the pointer move<br>
wants it negative? In that case, just use a minus instead of abs().<br></bl=
ockquote></div><div><br>Yes, you are right. This will have to be adjusted a=
ppropriately. <br></div><div class=3D"im"><blockquote class=3D"gmail_quote"=
 style=3D"margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);p=
adding-left:1ex">


<div><br>
&gt; + =A0 =A0 move-&gt;dy =3D es-&gt;geometry.y - ((es-&gt;geometry.height=
 / 2) + es-&gt;geometry.y);<br>
&gt; + =A0 =A0 move-&gt;origin_x =3D es-&gt;geometry.x;<br>
&gt; + =A0 =A0 move-&gt;origin_y =3D es-&gt;geometry.y;<br>
&gt; + =A0 =A0 move-&gt;surface =3D es;<br>
&gt; +<br>
&gt; + =A0 =A0 /* FIXME: Actually warp pointer instead<br>
&gt; + =A0 =A0 =A0* of just moving the cursor image */<br>
&gt; + =A0 =A0 notify_motion(&amp;wd-&gt;input_device, time,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 abs(move-&gt;dx) + es-&gt;geometry.x,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 abs(move-&gt;dy) + es-&gt;geometry.y);<br>
<br>
</div>If the purpose here is to move the pointer (in global coordinates) to=
<br>
the center of the surface, you&#39;d better use<br>
weston_surface_to_global(es, width/2, height/2, &amp;x, &amp;y).<br>
<br>
weston_surface_{to,from}_global() are the only way to properly convert<br>
between surface-local and global coordinates.<br></blockquote><div>=A0</div=
></div><div>I was unaware of this function and it seems to make more sense.=
 <br></div><div class=3D"im"><blockquote class=3D"gmail_quote" style=3D"mar=
gin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1=
ex">


<br>
Maybe you would need to use this to compute move-&gt;dx,dy?<br>
Move-&gt;dx,dy are in global coordinate system, width and height are in<br>
surface-local coordinate system, so you need a conversion somewhere.<br></b=
lockquote></div><div><br>From my tests as it stands currently, move-&gt;dx,=
dy are negative offsets starting at 0,0 for the upper left corner of the su=
rface. This probably will be refactored taking into consideration these sug=
gestions.<br>

</div><div><div class=3D"h5"><blockquote class=3D"gmail_quote" style=3D"mar=
gin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1=
ex">
<div><div><br>
&gt; +<br>
&gt; + =A0 =A0 wl_input_device_start_grab(&amp;wd-&gt;input_device, &amp;mo=
ve-&gt;grab, time);<br>
&gt; +<br>
&gt; + =A0 =A0 wl_input_device_set_keyboard_focus(&amp;wd-&gt;input_device,=
<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 &amp;es-&gt;surface, time);<br>
&gt; +<br>
&gt; + =A0 =A0 return 0;<br>
&gt; +}<br>
&gt; +<br>
&gt; =A0static void<br>
&gt; =A0shell_surface_move(struct wl_client *client, struct wl_resource *re=
source,<br>
&gt; =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct wl_resource *input_resource,=
 uint32_t time)<br>
&gt; @@ -276,10 +371,17 @@ resize_grab_button(struct wl_grab *grab,<br>
&gt; =A0 =A0 =A0 }<br>
&gt; =A0}<br>
&gt;<br>
&gt; +static void<br>
&gt; +resize_grab_key(struct wl_grab *grab,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0uint32_t time, int32_t key, int32_t s=
tate)<br>
&gt; +{<br>
&gt; +}<br>
&gt; +<br>
&gt; =A0static const struct wl_grab_interface resize_grab_interface =3D {<b=
r>
&gt; =A0 =A0 =A0 noop_grab_focus,<br>
&gt; =A0 =A0 =A0 resize_grab_motion,<br>
&gt; =A0 =A0 =A0 resize_grab_button,<br>
&gt; + =A0 =A0 resize_grab_key,<br>
&gt; =A0};<br>
&gt;<br>
&gt; =A0static int<br>
&gt; @@ -502,10 +604,17 @@ popup_grab_button(struct wl_grab *grab,<br>
&gt; =A0 =A0 =A0 =A0 =A0 =A0 =A0 shsurf-&gt;popup.initial_up =3D 1;<br>
&gt; =A0}<br>
&gt;<br>
&gt; +static void<br>
&gt; +popup_grab_key(struct wl_grab *grab,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0uint32_t time, int32_t key, int32_t s=
tate)<br>
&gt; +{<br>
&gt; +}<br>
&gt; +<br>
&gt; =A0static const struct wl_grab_interface popup_grab_interface =3D {<br=
>
&gt; =A0 =A0 =A0 popup_grab_focus,<br>
&gt; =A0 =A0 =A0 popup_grab_motion,<br>
&gt; =A0 =A0 =A0 popup_grab_button,<br>
&gt; + =A0 =A0 popup_grab_key,<br>
&gt; =A0};<br>
&gt;<br>
&gt; =A0static void<br>
&gt; @@ -917,6 +1026,39 @@ move_binding(struct wl_input_device *device, uin=
t32_t time,<br>
&gt; =A0}<br>
&gt;<br>
&gt; =A0static void<br>
&gt; +move_binding_key(struct wl_input_device *device, uint32_t time,<br>
&gt; + =A0 =A0 =A0 =A0 =A0uint32_t key, uint32_t button, uint32_t state, vo=
id *data)<br>
&gt; +{<br>
&gt; + =A0 =A0 if (!device-&gt;keyboard_focus_resource)<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; +<br>
&gt; + =A0 =A0 struct weston_surface *surface =3D<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 (struct weston_surface *) device-&gt;keyboar=
d_focus;<br>
&gt; + =A0 =A0 struct shell_surface *shsurf;<br>
&gt; +<br>
&gt; + =A0 =A0 if (surface =3D=3D NULL)<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; +<br>
&gt; + =A0 =A0 shsurf =3D get_shell_surface(surface);<br>
&gt; + =A0 =A0 if (!shsurf)<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; +<br>
&gt; + =A0 =A0 switch (shsurf-&gt;type) {<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case SHELL_SURFACE_NONE:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case SHELL_SURFACE_PANEL:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case SHELL_SURFACE_BACKGROUND:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case SHELL_SURFACE_LOCK:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 case SHELL_SURFACE_SCREENSAVER:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 default:<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;<br>
&gt; + =A0 =A0 }<br>
<br>
</div></div>Just a thought... maybe we should make a helper for detecting<b=
r>
non-manipulable surfaces, so we are guaranteed to get it the same in<br>
every function.<br>
<div><div><br>
&gt; +<br>
&gt; + =A0 =A0 weston_surface_keyboard_move(surface,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 (struct weston_input_device *) device, time);<br>
&gt; +}<br>
&gt; +<br>
&gt; +static void<br>
&gt; =A0resize_binding(struct wl_input_device *device, uint32_t time,<br>
&gt; =A0 =A0 =A0 =A0 =A0 =A0 =A0uint32_t key, uint32_t button, uint32_t sta=
te, void *data)<br>
&gt; =A0{<br>
&gt; @@ -1040,10 +1182,17 @@ rotate_grab_button(struct wl_grab *grab,<br>
&gt; =A0 =A0 =A0 }<br>
&gt; =A0}<br>
&gt;<br>
&gt; +static void<br>
&gt; +rotate_grab_key(struct wl_grab *grab,<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0uint32_t time, int32_t key, int32_t s=
tate)<br>
&gt; +{<br>
&gt; +}<br>
&gt; +<br>
&gt; =A0static const struct wl_grab_interface rotate_grab_interface =3D {<b=
r>
&gt; =A0 =A0 =A0 noop_grab_focus,<br>
&gt; =A0 =A0 =A0 rotate_grab_motion,<br>
&gt; =A0 =A0 =A0 rotate_grab_button,<br>
&gt; + =A0 =A0 rotate_grab_key,<br>
&gt; =A0};<br>
&gt;<br>
&gt; =A0static void<br>
&gt; @@ -1638,6 +1787,8 @@ shell_init(struct weston_compositor *ec)<br>
&gt;<br>
&gt; =A0 =A0 =A0 weston_compositor_add_binding(ec, 0, BTN_LEFT, MODIFIER_SU=
PER,<br>
&gt; =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mo=
ve_binding, shell);<br>
&gt; + =A0 =A0 weston_compositor_add_binding(ec, KEY_F7, 0, MODIFIER_SUPER,=
<br>
&gt; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 move=
_binding_key, shell);<br>
&gt; =A0 =A0 =A0 weston_compositor_add_binding(ec, 0, BTN_MIDDLE, MODIFIER_=
SUPER,<br>
&gt; =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 re=
size_binding, shell);<br>
&gt; =A0 =A0 =A0 weston_compositor_add_binding(ec, KEY_BACKSPACE, 0,<br>
<br>
</div></div>We already discussed this somewhat in irc, but I will reiterate=
 for the<br>
record.<br>
<br>
By adding the key function to the grab interface, we stop sending key<br>
events during grabs. If any changes happen in the key state during a<br>
grab, the client having keyboard focus will go out of sync. For<br>
example, press key &#39;A&#39; down, press the key binding to start move,<b=
r>
release all keys, press and release Esc to end move. The client will<br>
still see &#39;A&#39; pressed, I assume.<br>
<br>
This is a general problem with keyboard grabs, and requires a solution.<br>
<br>
<br>
Thanks,<br>
pq<br></blockquote></div></div><div><br>Yes, many good points. As you sugge=
sted in irc, we probably should wait to see what happens at fosdem before p=
roceeding, especially since such functionality touches on protocol design a=
nd other key assets. Thank you for your timely review, I&#39;ll submit a se=
cond version of this patch after getting krh&#39;s input as well.<br>

</div></div><br>
</div><br>

--0015175d04bc52c67304b80c8529--


More information about the wayland-devel mailing list