[PATCH xserver] XKB: Redirect actions defunct with Gtk3 (XInput?)

2012-02-19 Thread Andreas Wettstein
When redirect actions are used with Gtk3, Gtk3 complains about
events not holding a GdkDevice.  This is caused by device IDs not
being set for redirect actions. The patch below sets them, but I
am not quite sure that the values are correct.  Anyway, the
warnings from Gtk3 warnings are gone.

More seriously, Gtk3 does not receive state changes redirect
actions might specify.  This is because event_set_state in
dix/inpututils.c accesses the prev_state field, but the changes
for the redirect action are only put into the state field.

Signed-off-by: Andreas Wettstein 
---
 xkb/xkbActions.c |   18 ++
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
index da0bdea..da874f0 100644
--- a/xkb/xkbActions.c
+++ b/xkb/xkbActions.c
@@ -795,7 +795,7 @@ _XkbFilterRedirectKey(  XkbSrvInfoPtr   xkbi,
 {
 DeviceEventev;
 intx,y;
-XkbStateRecold;
+XkbStateRecold, old_prev;
 unsigned   mods,mask;
 xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(xkbi->device);
 ProcessInputProc backupproc;
@@ -803,6 +803,7 @@ ProcessInputProc backupproc;
 /* never actually used uninitialised, but gcc isn't smart enough
  * to work that out. */
 memset(&old, 0, sizeof(old));
+memset(&old_prev, 0, sizeof(old_prev));
 memset(&ev, 0, sizeof(ev));
 
 if ((filter->keycode!=0)&&(filter->keycode!=keycode))
@@ -814,6 +815,7 @@ ProcessInputProc backupproc;
 ev.time = GetTimeInMillis();
 ev.root_x = x;
 ev.root_y = y;
+ev.deviceid = ev.sourceid = xkbi->device->id;
 
 if (filter->keycode==0) {  /* initial press */
if ((pAction->redirect.new_keydesc->min_key_code)||
@@ -839,6 +841,7 @@ ProcessInputProc backupproc;
 
if ( mask || mods ) {
old= xkbi->state;
+   old_prev= xkbi->prev_state;
xkbi->state.base_mods&= ~mask;
xkbi->state.base_mods|= (mods&mask);
xkbi->state.latched_mods&= ~mask;
@@ -846,15 +849,18 @@ ProcessInputProc backupproc;
xkbi->state.locked_mods&= ~mask;
xkbi->state.locked_mods|= (mods&mask);
XkbComputeDerivedState(xkbi);
+   xkbi->prev_state= xkbi->state;
}
 
UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
xkbi->device->public.processInputProc((InternalEvent*)&ev, 
xkbi->device);
COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
 backupproc,xkbUnwrapProc);
-   
-   if ( mask || mods )
+
+   if ( mask || mods ) {
xkbi->state= old;
+   xkbi->prev_state= old_prev;
+   }
 }
 else if (filter->keycode==keycode) {
 
@@ -870,6 +876,7 @@ ProcessInputProc backupproc;
 
if ( mask || mods ) {
old= xkbi->state;
+   old_prev= xkbi->prev_state;
xkbi->state.base_mods&= ~mask;
xkbi->state.base_mods|= (mods&mask);
xkbi->state.latched_mods&= ~mask;
@@ -877,6 +884,7 @@ ProcessInputProc backupproc;
xkbi->state.locked_mods&= ~mask;
xkbi->state.locked_mods|= (mods&mask);
XkbComputeDerivedState(xkbi);
+   xkbi->prev_state= xkbi->state;
}
 
UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
@@ -884,8 +892,10 @@ ProcessInputProc backupproc;
COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
 backupproc,xkbUnwrapProc);
 
-   if ( mask || mods )
+   if ( mask || mods ) {
xkbi->state= old;
+   xkbi->prev_state= old_prev;
+   }
 
filter->keycode= 0;
filter->active= 0;
-- 
1.7.6

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] XKB: Redirect actions defunct with Gtk3 (XInput?)

2012-02-19 Thread Peter Hutterer
On Sun, Feb 19, 2012 at 11:18:19AM +0100, Andreas Wettstein wrote:
> When redirect actions are used with Gtk3, Gtk3 complains about
> events not holding a GdkDevice.  This is caused by device IDs not
> being set for redirect actions. The patch below sets them, but I
> am not quite sure that the values are correct.  Anyway, the
> warnings from Gtk3 warnings are gone.
> 
> More seriously, Gtk3 does not receive state changes redirect
> actions might specify.  This is because event_set_state in
> dix/inpututils.c accesses the prev_state field, but the changes
> for the redirect action are only put into the state field.
> 
> Signed-off-by: Andreas Wettstein 
> ---
>  xkb/xkbActions.c |   18 ++
>  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index da0bdea..da874f0 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -795,7 +795,7 @@ _XkbFilterRedirectKey(XkbSrvInfoPtr   xkbi,
>  {
>  DeviceEvent  ev;
>  int  x,y;
> -XkbStateRec  old;
> +XkbStateRec  old, old_prev;
>  unsigned mods,mask;
>  xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(xkbi->device);
>  ProcessInputProc backupproc;
> @@ -803,6 +803,7 @@ ProcessInputProc backupproc;
>  /* never actually used uninitialised, but gcc isn't smart enough
>   * to work that out. */
>  memset(&old, 0, sizeof(old));
> +memset(&old_prev, 0, sizeof(old_prev));
>  memset(&ev, 0, sizeof(ev));
>  
>  if ((filter->keycode!=0)&&(filter->keycode!=keycode))
> @@ -814,6 +815,7 @@ ProcessInputProc backupproc;
>  ev.time = GetTimeInMillis();
>  ev.root_x = x;
>  ev.root_y = y;
> +ev.deviceid = ev.sourceid = xkbi->device->id;

not sure about this one. the actual device and source ID is in the caller
and you need to pass this down. they aren't always the same.
I think the rest is ok though.

Cheers,
  Peter
>  
>  if (filter->keycode==0) {/* initial press */
>   if ((pAction->redirect.new_keydesc->min_key_code)||
> @@ -839,6 +841,7 @@ ProcessInputProc backupproc;
>  
>   if ( mask || mods ) {
>   old= xkbi->state;
> + old_prev= xkbi->prev_state;
>   xkbi->state.base_mods&= ~mask;
>   xkbi->state.base_mods|= (mods&mask);
>   xkbi->state.latched_mods&= ~mask;
> @@ -846,15 +849,18 @@ ProcessInputProc backupproc;
>   xkbi->state.locked_mods&= ~mask;
>   xkbi->state.locked_mods|= (mods&mask);
>   XkbComputeDerivedState(xkbi);
> + xkbi->prev_state= xkbi->state;
>   }
>  
>   UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
>   xkbi->device->public.processInputProc((InternalEvent*)&ev, 
> xkbi->device);
>   COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
>backupproc,xkbUnwrapProc);
> - 
> - if ( mask || mods )
> +
> + if ( mask || mods ) {
>   xkbi->state= old;
> + xkbi->prev_state= old_prev;
> + }
>  }
>  else if (filter->keycode==keycode) {
>  
> @@ -870,6 +876,7 @@ ProcessInputProc backupproc;
>  
>   if ( mask || mods ) {
>   old= xkbi->state;
> + old_prev= xkbi->prev_state;
>   xkbi->state.base_mods&= ~mask;
>   xkbi->state.base_mods|= (mods&mask);
>   xkbi->state.latched_mods&= ~mask;
> @@ -877,6 +884,7 @@ ProcessInputProc backupproc;
>   xkbi->state.locked_mods&= ~mask;
>   xkbi->state.locked_mods|= (mods&mask);
>   XkbComputeDerivedState(xkbi);
> + xkbi->prev_state= xkbi->state;
>   }
>  
>   UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
> @@ -884,8 +892,10 @@ ProcessInputProc backupproc;
>   COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
>backupproc,xkbUnwrapProc);
>  
> - if ( mask || mods )
> + if ( mask || mods ) {
>   xkbi->state= old;
> + xkbi->prev_state= old_prev;
> + }
>  
>   filter->keycode= 0;
>   filter->active= 0;
> -- 
> 1.7.6
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] XKB: Redirect actions defunct with Gtk3 (XInput?)

2012-02-21 Thread wettstein509
> > +ev.deviceid = ev.sourceid = xkbi->device->id;
> 
> not sure about this one. the actual device and source ID is in the caller
> and you need to pass this down. they aren't always the same.

Do you know if either ev.deviceid or ev.sourceid should always be equal
to xkbi->device->id?  In that case, I could pass the other ID through
filter->priv.  Otherwise, I would have to change the function signature.
This will have ripple effects, as the signature is fixed in
XkbFilterRec.

Andreas
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] XKB: Redirect actions defunct with Gtk3 (XInput?)

2012-02-21 Thread Peter Hutterer
On Tue, Feb 21, 2012 at 08:43:45PM +0100, wettstein...@solnet.ch wrote:
> > > +ev.deviceid = ev.sourceid = xkbi->device->id;
> > 
> > not sure about this one. the actual device and source ID is in the caller
> > and you need to pass this down. they aren't always the same.
> 
> Do you know if either ev.deviceid or ev.sourceid should always be equal
> to xkbi->device->id?  In that case, I could pass the other ID through
> filter->priv.  Otherwise, I would have to change the function signature.
> This will have ripple effects, as the signature is fixed in
> XkbFilterRec.

general rule:
ev.deviceid is the device id of the device the event is posted for.
ev.sourceid is the device id of the device the event originated from.
so if you get an event through a master device, the deviceid will be the
master's ID while the sourceid will be the slave device ID that generated
the event. 

XKB actions don't work across devices, so in this case this means that
xkbi->device->id == ev.deviceid, so you can take this shortcut here (add a
comment though please).

Cheers,
  Peter
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] XKB: Redirect actions defunct with Gtk3 (XInput?)

2012-02-25 Thread wettstein509
When redirect actions are used with Gtk3, Gtk3 complained about
events not holding a GdkDevice.  This was caused by device IDs
not being set for redirect actions.

More seriously, Gtk3 did not receive state changes redirect
actions might specify.  This was because event_set_state in
dix/inpututils.c accesses the prev_state field, but the changes
for the redirect action were only put into the state field.

Signed-off-by: Andreas Wettstein 
---
 xkb/xkbActions.c |   28 +++-
 1 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
index da0bdea..891179b 100644
--- a/xkb/xkbActions.c
+++ b/xkb/xkbActions.c
@@ -795,7 +795,7 @@ _XkbFilterRedirectKey(  XkbSrvInfoPtr   xkbi,
 {
 DeviceEventev;
 intx,y;
-XkbStateRecold;
+XkbStateRecold, old_prev;
 unsigned   mods,mask;
 xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(xkbi->device);
 ProcessInputProc backupproc;
@@ -803,6 +803,7 @@ ProcessInputProc backupproc;
 /* never actually used uninitialised, but gcc isn't smart enough
  * to work that out. */
 memset(&old, 0, sizeof(old));
+memset(&old_prev, 0, sizeof(old_prev));
 memset(&ev, 0, sizeof(ev));
 
 if ((filter->keycode!=0)&&(filter->keycode!=keycode))
@@ -814,6 +815,11 @@ ProcessInputProc backupproc;
 ev.time = GetTimeInMillis();
 ev.root_x = x;
 ev.root_y = y;
+/* redirect actions do not work across devices, therefore the following is
+ * correct: */
+ev.deviceid = xkbi->device->id;
+/* filter->priv must be set up by the caller for the initial press. */
+ev.sourceid = filter->priv;
 
 if (filter->keycode==0) {  /* initial press */
if ((pAction->redirect.new_keydesc->min_key_code)||
@@ -823,7 +829,6 @@ ProcessInputProc backupproc;
filter->keycode = keycode;
filter->active = 1;
filter->filterOthers = 0;
-   filter->priv = 0;
filter->filter = _XkbFilterRedirectKey;
filter->upAction = *pAction;
 
@@ -839,6 +844,7 @@ ProcessInputProc backupproc;
 
if ( mask || mods ) {
old= xkbi->state;
+   old_prev= xkbi->prev_state;
xkbi->state.base_mods&= ~mask;
xkbi->state.base_mods|= (mods&mask);
xkbi->state.latched_mods&= ~mask;
@@ -846,15 +852,18 @@ ProcessInputProc backupproc;
xkbi->state.locked_mods&= ~mask;
xkbi->state.locked_mods|= (mods&mask);
XkbComputeDerivedState(xkbi);
+   xkbi->prev_state= xkbi->state;
}
 
UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
xkbi->device->public.processInputProc((InternalEvent*)&ev, 
xkbi->device);
COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
 backupproc,xkbUnwrapProc);
-   
-   if ( mask || mods )
+
+   if ( mask || mods ) {
xkbi->state= old;
+   xkbi->prev_state= old_prev;
+   }
 }
 else if (filter->keycode==keycode) {
 
@@ -870,6 +879,7 @@ ProcessInputProc backupproc;
 
if ( mask || mods ) {
old= xkbi->state;
+   old_prev= xkbi->prev_state;
xkbi->state.base_mods&= ~mask;
xkbi->state.base_mods|= (mods&mask);
xkbi->state.latched_mods&= ~mask;
@@ -877,6 +887,7 @@ ProcessInputProc backupproc;
xkbi->state.locked_mods&= ~mask;
xkbi->state.locked_mods|= (mods&mask);
XkbComputeDerivedState(xkbi);
+   xkbi->prev_state= xkbi->state;
}
 
UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
@@ -884,8 +895,10 @@ ProcessInputProc backupproc;
COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
 backupproc,xkbUnwrapProc);
 
-   if ( mask || mods )
+   if ( mask || mods ) {
xkbi->state= old;
+   xkbi->prev_state= old_prev;
+   }
 
filter->keycode= 0;
filter->active= 0;
@@ -1155,6 +1168,11 @@ xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(dev);
break;
case XkbSA_RedirectKey:
filter = _XkbNextFreeFilter(xkbi);
+   /* redirect actions must create a new DeviceEvent.  The
+* source device id for this event cannot be obtained from
+* xkbi, so we pass it here explicitly. The field deviceid
+* equals to xkbi->device->id. */
+   filter->priv = event->sourceid;
sendEvent= _XkbFilterRedirectKey(xkbi,filter,key,&act);
break;
case XkbSA_DeviceBtn:
-- 
1.7.6

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] XKB: Redirect actions defunct with Gtk3 (XInput?)

2012-02-26 Thread Peter Hutterer
On Sat, Feb 25, 2012 at 08:48:17PM +0100, wettstein...@solnet.ch wrote:
> When redirect actions are used with Gtk3, Gtk3 complained about
> events not holding a GdkDevice.  This was caused by device IDs
> not being set for redirect actions.
> 
> More seriously, Gtk3 did not receive state changes redirect
> actions might specify.  This was because event_set_state in
> dix/inpututils.c accesses the prev_state field, but the changes
> for the redirect action were only put into the state field.
> 
> Signed-off-by: Andreas Wettstein 

Thanks. I've merged this into my -next branch. With the release due this
weekend, we're down to blockers only.

Cheers,
  Peter

> ---
>  xkb/xkbActions.c |   28 +++-
>  1 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index da0bdea..891179b 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -795,7 +795,7 @@ _XkbFilterRedirectKey(XkbSrvInfoPtr   xkbi,
>  {
>  DeviceEvent  ev;
>  int  x,y;
> -XkbStateRec  old;
> +XkbStateRec  old, old_prev;
>  unsigned mods,mask;
>  xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(xkbi->device);
>  ProcessInputProc backupproc;
> @@ -803,6 +803,7 @@ ProcessInputProc backupproc;
>  /* never actually used uninitialised, but gcc isn't smart enough
>   * to work that out. */
>  memset(&old, 0, sizeof(old));
> +memset(&old_prev, 0, sizeof(old_prev));
>  memset(&ev, 0, sizeof(ev));
>  
>  if ((filter->keycode!=0)&&(filter->keycode!=keycode))
> @@ -814,6 +815,11 @@ ProcessInputProc backupproc;
>  ev.time = GetTimeInMillis();
>  ev.root_x = x;
>  ev.root_y = y;
> +/* redirect actions do not work across devices, therefore the following 
> is
> + * correct: */
> +ev.deviceid = xkbi->device->id;
> +/* filter->priv must be set up by the caller for the initial press. */
> +ev.sourceid = filter->priv;
>  
>  if (filter->keycode==0) {/* initial press */
>   if ((pAction->redirect.new_keydesc->min_key_code)||
> @@ -823,7 +829,6 @@ ProcessInputProc backupproc;
>   filter->keycode = keycode;
>   filter->active = 1;
>   filter->filterOthers = 0;
> - filter->priv = 0;
>   filter->filter = _XkbFilterRedirectKey;
>   filter->upAction = *pAction;
>  
> @@ -839,6 +844,7 @@ ProcessInputProc backupproc;
>  
>   if ( mask || mods ) {
>   old= xkbi->state;
> + old_prev= xkbi->prev_state;
>   xkbi->state.base_mods&= ~mask;
>   xkbi->state.base_mods|= (mods&mask);
>   xkbi->state.latched_mods&= ~mask;
> @@ -846,15 +852,18 @@ ProcessInputProc backupproc;
>   xkbi->state.locked_mods&= ~mask;
>   xkbi->state.locked_mods|= (mods&mask);
>   XkbComputeDerivedState(xkbi);
> + xkbi->prev_state= xkbi->state;
>   }
>  
>   UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
>   xkbi->device->public.processInputProc((InternalEvent*)&ev, 
> xkbi->device);
>   COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
>backupproc,xkbUnwrapProc);
> - 
> - if ( mask || mods )
> +
> + if ( mask || mods ) {
>   xkbi->state= old;
> + xkbi->prev_state= old_prev;
> + }
>  }
>  else if (filter->keycode==keycode) {
>  
> @@ -870,6 +879,7 @@ ProcessInputProc backupproc;
>  
>   if ( mask || mods ) {
>   old= xkbi->state;
> + old_prev= xkbi->prev_state;
>   xkbi->state.base_mods&= ~mask;
>   xkbi->state.base_mods|= (mods&mask);
>   xkbi->state.latched_mods&= ~mask;
> @@ -877,6 +887,7 @@ ProcessInputProc backupproc;
>   xkbi->state.locked_mods&= ~mask;
>   xkbi->state.locked_mods|= (mods&mask);
>   XkbComputeDerivedState(xkbi);
> + xkbi->prev_state= xkbi->state;
>   }
>  
>   UNWRAP_PROCESS_INPUT_PROC(xkbi->device,xkbPrivPtr, backupproc);
> @@ -884,8 +895,10 @@ ProcessInputProc backupproc;
>   COND_WRAP_PROCESS_INPUT_PROC(xkbi->device, xkbPrivPtr,
>backupproc,xkbUnwrapProc);
>  
> - if ( mask || mods )
> + if ( mask || mods ) {
>   xkbi->state= old;
> + xkbi->prev_state= old_prev;
> + }
>  
>   filter->keycode= 0;
>   filter->active= 0;
> @@ -1155,6 +1168,11 @@ xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(dev);
>   break;
>   case XkbSA_RedirectKey:
>   filter = _XkbNextFreeFilter(xkbi);
> + /* redirect actions must create a new DeviceEvent.  The
> +  * source device id for this event cannot be obtained from
> +  * xkbi, so we pass it here explicitly. The field deviceid
> +  * equals to xkbi->device->id. */
> + filter->priv = event->sourceid;
>   sendEvent= _XkbFilterRedirectKey(xkbi,filter,key,&act);
>   break;
>