Handling GenericEvents in XSendEvent()
Peter Hutterer
peter.hutterer at who-t.net
Tue Dec 21 16:58:04 PST 2010
On Mon, Dec 20, 2010 at 02:26:22PM +0000, Daniel Stone wrote:
> Hi Carlos,
>
> On Fri, Nov 26, 2010 at 11:19:40PM +0100, Carlos Garnacho wrote:
> > A few weeks ago I sent some patches to make XSendEvent() deal with
> > GenericEvents, which stayed mostly unreviewed, I'm reposting these,
> > rebased against recent masters, also including some docs for the XGE
> > updates.
> >
> > Any comment is welcome, IMHO it'd be great to have this so a XI2-aware
> > implementation of XEmbed can be done sooner rather than later.
>
> Sorry these have sat unreviewed for so long. I took a look at these the
> other night and they pretty much seem fine; I made a couple of minor
> adjustments (bumping the protocol package version, actually bumping the
> server's GE version so it reports having 1.1 for instance, etc) and
> added my Reviewed-by tags. I also made a patch for xcbproto.
>
> Peter, could you please comment on these?
>
> git://anongit.freedesktop.org/~daniels/xextproto:ge
reviewed 5bc6f9d92 and I pulled in the diff here, because it's easier to
comment inline.
diff --git a/configure.ac b/configure.ac
index 41269a3..9e176e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
AC_PREREQ([2.60])
-AC_INIT([XExtProto], [7.1.99.0], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([XExtProto], [7.1.99.1], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
AM_INIT_AUTOMAKE([foreign dist-bzip2])
AM_MAINTAINER_MODE
diff --git a/ge.h b/ge.h
index aca1d8a..f52821b 100644
--- a/ge.h
+++ b/ge.h
@@ -29,7 +29,7 @@
#define GE_NAME "Generic Event Extension"
#define GE_MAJOR 1
-#define GE_MINOR 0
+#define GE_MINOR 1
/*********************************************************
*
@@ -38,8 +38,9 @@
*/
#define X_GEQueryVersion 0
+#define X_GESendEvent 1
-#define GENumberRequests (X_GEQueryVersion + 1)
+#define GENumberRequests (X_GESendEvent + 1)
/*********************************************************
*
diff --git a/geproto.h b/geproto.h
index c8860dd..0c70e5f 100644
--- a/geproto.h
+++ b/geproto.h
@@ -81,5 +81,18 @@ typedef struct {
#define sz_xGEQueryVersionReply 32
+typedef struct {
+ CARD8 reqType;
+ CARD8 ReqType;
+ CARD16 length B16;
+ CARD8 propagate;
+ CARD8 pad1;
+ CARD16 pad2;
+ CARD32 destination B32;
+ CARD32 eventMask B32;
+} xGESendEventReq;
+
+#define sz_xGESendEventReq 16
+
#endif /* _GEPROTO_H_ */
diff --git a/specs/geproto.xml b/specs/geproto.xml
index 7bba357..dc9a7bf 100644
--- a/specs/geproto.xml
+++ b/specs/geproto.xml
@@ -4,6 +4,7 @@
<book>
<bookinfo>
<title>X Generic Event Extension</title>
+ <releaseinfo>Version 1.1</releaseinfo>
<author>
<firstname>Peter</firstname>
@@ -14,6 +15,13 @@
<orgname>peter.hutterer at who-t.net</orgname>
</affiliation>
</author>
+
+ <collab>
+ <collabname>Carlos Garnacho</collabname>
+ <affiliation>
+ <orgname>carlos at lanedo.com</orgname>
+ </affiliation>
+ </collab>
</bookinfo>
<chapter>
@@ -30,6 +38,9 @@
<para>GenericEvents may be longer than 32 bytes. If so, the number of 4
byte units following the initial 32 bytes must be specified in the length
field of the event.</para>
+
+ <para>As of version 1.0, no other requests are provided by this extension;
+ version 1.1 adds the GESendEvent request.</para>
</chapter>
this needs to be either reworded or moved back down again. looking at the
final documentataion, the "no other requests" is unclear as no requests have
been discussed yet. how about a subsection "Extension Versions" that list
the requests added in each version?
<chapter>
@@ -51,9 +62,6 @@
compatible changes. It is the clients responsibility to ensure that the
server supports a version which is compatible with its
expectations.</para>
-
- <para>As of version 1.0, no other requests are provided by this extension.
- </para>
</chapter>
<chapter>
@@ -77,6 +85,31 @@
</chapter>
<chapter>
+ <title>Sending events</title>
+
+ <para>Since version 1.1, GE provides a request to complement the SendEvent
+ request in the core protocol, this is a variable length request that will
^ period here instead of comma.
+ take the xGenericEventCookie being sent as the extra data.</para>
the protocol has no notion of a cookie. the cookie is a Xlib-specific
implementation only, caused by internal event size limitations of the Xlib
event queue. The same paragraph would be correct by just replacing
"xGenericEventCookie" with "GenericEvent".
+
+ <programlisting>GESendEvent
+ propagate: CARD8
+ pad1: CARD8
+ pad2: CARD16
+ destination: CARD32
+ eventMask: CARD32
+
+ Errors: Device, Value, Window</programlisting>
+
+ <para>The arguments are very similar to its core protocol counterpart, with
+ the exception of eventMask being an event mask of the extension protocol the
+ event pertains to.</para>
this could be interesting. while CARD32 works fine for core protocol event
masks (and probably most other masks that we have), it doesn't really exist
for XI2 where the event masks are device-specific and of variable length.
this needs to be discussed here.
+
+ <para>In libX11, This functionality is handled in XSendEvent() itself, so
+ if one client wants to send a GE-based event to another client, such function
+ can be used by passing a XGenericEventCookie pointer.</para>
+ </chapter>
+
+ <chapter>
<title>Notes</title>
<para>Although the wire event is of arbitrary length, the actual size of
> git://anongit.freedesktop.org/~daniels/xserver:ge
2ecd833478c302697f27e3e898a46a45c2acae83
might be nice to add a macro instead fo sprinkling 0x7f everywhere.
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
pulled in 2f3e8ab0654ee9d36c9cd20a07384a97f7157e81 as diff, really, would be
much easier to review if it was a patch file instead of a branch link (hint,
hint, nudge, nudge).
diff --git a/Xext/geext.c b/Xext/geext.c
index 8319c92..44390a9 100644
--- a/Xext/geext.c
+++ b/Xext/geext.c
@@ -31,10 +31,19 @@
#include "geint.h"
#include "geext.h"
+#include "xace.h"
+#include "inputstr.h" /* SpritePtr */
+#include "exglobals.h" /* BadDevice */
#include "protocol-versions.h"
#define rClient(obj) (clients[CLIENT_ID((obj)->resource)])
+/* Akin to the vars in dix/events.c, but
+ * to be used only with GenericEvents
+ */
+static xEvent* swapEvent = NULL;
+static int swapEventLen = 0;
+
DevPrivateKeyRec GEClientPrivateKeyRec;
int RT_GECLIENT = 0;
@@ -44,7 +53,8 @@ GEExtension GEExtensions[MAXEXTENSIONS];
/* Major available requests */
static const int version_requests[] = {
X_GEQueryVersion, /* before client sends QueryVersion */
- X_GEQueryVersion, /* must be set to last request in version 1 */
+ X_GESendEvent, /* send event, since 1.1 */
+ X_GESendEvent, /* must be set to last request in version 1.1 */
};
/* Forward declarations */
@@ -91,9 +101,106 @@ ProcGEQueryVersion(ClientPtr client)
return Success;
}
+static int
+ProcGESendEvent(ClientPtr client)
+{
+ WindowPtr pWin;
+ WindowPtr effectiveFocus = NullWindow; /* only set if dest==InputFocus */
+ DeviceIntPtr dev = PickPointer(client);
+ DeviceIntPtr keybd = GetPairedDevice(dev);
+ SpritePtr pSprite;
+ xGenericEvent *event;
+ REQUEST(xGESendEventReq);
+
+ REQUEST_AT_LEAST_SIZE(xGESendEventReq);
+
+ event = (xGenericEvent *) &stuff[1];
+
+ /* The client's event type must be a GenericEvent. */
+ if (event->type != GenericEvent) {
+ client->errorValue = event->type;
+ return BadValue;
+ }
+
+ dev = GetEventDevice(client, (xEvent *) event);
+
+ if (!dev)
+ return BadDevice;
+
+ if (IsPointerDevice (dev)) {
+ keybd = GetPairedDevice(dev);
+ } else {
+ keybd = dev;
+ dev = GetPairedDevice(keybd);
+ }
+
+ pSprite = dev->spriteInfo->sprite;
+
+ /* FIXME: Check for eventMask containing only the extension mask bits set on */
this is where it gets tricky. XI2 uses mask_len and mask[], so the simple
mask here wouldn't do it and you'd have to specify this in the protocol on
how the mask behaves. I'm really not sure how to do this other than have
per-extension specifications on how the mask is set.
a possible wire layout could be [request][event][space for mask], where the
mask space is set depending on the extension.
+
+ if (stuff->destination == PointerWindow)
+ pWin = pSprite->win;
+ else if (stuff->destination == InputFocus) {
+ WindowPtr inputFocus = (keybd) ? keybd->focus->win : NoneWin;
+
+ if (inputFocus == NoneWin)
+ return Success;
+
+ /* If the input focus is PointerRootWin, send the event to where
+ the pointer is if possible, then perhaps propogate up to root. */
+ if (inputFocus == PointerRootWin)
+ inputFocus = pSprite->spriteTrace[0]; /* Root window! */
+
+ if (IsParent(inputFocus, pSprite->win)) {
+ effectiveFocus = inputFocus;
+ pWin = pSprite->win;
+ } else {
+ effectiveFocus = pWin = inputFocus;
+ }
+ } else {
+ dixLookupWindow(&pWin, stuff->destination, client, DixSendAccess);
+ }
+
+ if (!pWin)
+ return BadWindow;
+
+ if ((stuff->propagate != xFalse) && (stuff->propagate != xTrue)) {
+ client->errorValue = stuff->propagate;
+ return BadValue;
+ }
+
+ event->type |= 0x80;
+
+ if (stuff->propagate) {
+ for (;pWin; pWin = pWin->parent) {
+ if (XaceHook(XACE_SEND_ACCESS, client, NULL, pWin, event, 1))
+ return Success;
+
+ if (DeliverEventsToWindow(dev, pWin, (xEvent *) event,
+ 1, stuff->eventMask, NullGrab))
+ return Success;
+
+ if (pWin == effectiveFocus)
+ return Success;
+
+ stuff->eventMask &= ~wDontPropagateMask(pWin);
+
+ if (!stuff->eventMask)
+ break;
+ }
+ } else if (!XaceHook(XACE_SEND_ACCESS, client, NULL, pWin, event, 1)) {
+ DeliverEventsToWindow(dev, pWin, (xEvent *) event,
+ 1, stuff->eventMask, NullGrab);
+ }
+
+ return Success;
please try to share code between ProcSendEvent and here. from what I can
tell, this is nearly identical, so code duplication will just duplicate the
bugs as well.
+}
+
int (*ProcGEVector[GENumberRequests])(ClientPtr) = {
/* Version 1.0 */
- ProcGEQueryVersion
+ ProcGEQueryVersion,
+ /* Version 1.1 */
+ ProcGESendEvent
};
/************************************************************/
@@ -112,9 +219,52 @@ SProcGEQueryVersion(ClientPtr client)
return(*ProcGEVector[stuff->ReqType])(client);
}
+static int
+SProcGESendEvent(ClientPtr client)
+{
+ xGenericEvent *ev;
+ int n, len;
+
+ REQUEST(xGESendEventReq);
+
+ swaps(&stuff->length, n);
+ REQUEST_AT_LEAST_SIZE(xGESendEventReq);
+
+ ev = (xGenericEvent *) &stuff[1];
+
+ if (ev->type != GenericEvent) {
+ client->errorValue = ev->type;
+ return BadValue;
+ }
+
+ /* Swap request fields */
+ swapl(&stuff->destination, n);
+ swapl(&stuff->eventMask, n);
+
+ /* Swap event being sent */
+ len = sizeof (xEvent) + (ev->length << 2);
^ no space here
+
+ if (len > swapEventLen) {
+ swapEventLen = len;
+ swapEvent = realloc(swapEvent, swapEventLen);
+
+ if (!swapEvent) {
+ FatalError("SProcGESendEvent: Out of memory.\n");
+ return BadValue;
+ }
+ }
+
+ SGEGenericEvent((xEvent *) ev, swapEvent);
+ memcpy(ev, swapEvent, len);
+
+ return(*ProcGEVector[stuff->ReqType])(client);
^ space here
+}
please write a simple test for this to verify with make check that the
swapping code works.
in regards to style: I think the more common style is typecasts in the
form of (type*)variable instead of "(type *) variable". (not that we have a
single style)
+
int (*SProcGEVector[GENumberRequests])(ClientPtr) = {
/* Version 1.0 */
- SProcGEQueryVersion
+ SProcGEQueryVersion,
+ /* Version 1.1 */
+ SProcGESendEvent
};
diff --git a/configure.ac b/configure.ac
index a5967ad..50ffcfc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -783,7 +783,7 @@ WINDOWSWMPROTO="windowswmproto"
APPLEWMPROTO="applewmproto >= 1.4"
dnl Core modules for most extensions, et al.
-SDK_REQUIRED_MODULES="[xproto >= 7.0.17] [randrproto >= 1.4] [renderproto >= 0.11] [xextproto >= 7.1.99] [inputproto >= 1.9.99.902] [kbproto >= 1.0.3] fontsproto"
+SDK_REQUIRED_MODULES="[xproto >= 7.0.17] [randrproto >= 1.4] [renderproto >= 0.11] [xextproto >= 7.1.99.1] [inputproto >= 1.9.99.902] [kbproto >= 1.0.3] fontsproto"
# Make SDK_REQUIRED_MODULES available for inclusion in xorg-server.pc
AC_SUBST(SDK_REQUIRED_MODULES)
diff --git a/dix/events.c b/dix/events.c
index ac07923..33ebfd5 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -447,6 +447,26 @@ GetWindowXI2Mask(DeviceIntPtr dev, WindowPtr win, xEvent* ev)
(inputMasks->xi2mask[XIAllMasterDevices][evtype/8] && IsMaster(dev)));
}
+DeviceIntPtr
+GetEventDevice(ClientPtr client, xEvent *ev)
+{
+ if (XI2_EVENT(ev)) {
+ DeviceIntPtr pDev;
+ int rc;
+
+ rc = dixLookupDevice(&pDev,
+ ((xXIGenericDeviceEvent *) ev)->deviceid,
+ client, DixWriteAccess);
+
+ if (rc == Success)
+ return pDev;
+ } else {
+ return PickPointer (client);
+ }
+
+ return NULL;
+}
+
static Mask
GetEventMask(DeviceIntPtr dev, xEvent *event, InputClients* other)
{
diff --git a/include/input.h b/include/input.h
index 8feac28..80df2c3 100644
--- a/include/input.h
+++ b/include/input.h
@@ -527,6 +527,7 @@ extern _X_EXPORT void FreeInputAttributes(InputAttributes *attrs);
/* misc event helpers */
extern Mask GetEventFilter(DeviceIntPtr dev, xEvent *event);
extern Mask GetWindowXI2Mask(DeviceIntPtr dev, WindowPtr win, xEvent* ev);
+extern DeviceIntPtr GetEventDevice(ClientPtr client, xEvent* ev);
void FixUpEventFromWindow(DeviceIntPtr pDev,
xEvent *xE,
WindowPtr pWin,
diff --git a/include/protocol-versions.h b/include/protocol-versions.h
index c8c7f5f..72de3e2 100644
--- a/include/protocol-versions.h
+++ b/include/protocol-versions.h
@@ -53,7 +53,7 @@
/* Generic event extension */
#define SERVER_GE_MAJOR_VERSION 1
-#define SERVER_GE_MINOR_VERSION 0
+#define SERVER_GE_MINOR_VERSION 1
/* GLX */
#define SERVER_GLX_MAJOR_VERSION 1
> git://anongit.freedesktop.org/~daniels/xcbproto:ge
> git://anongit.freedesktop.org/~daniels/libxcb:ge
skipped those two, since Peter already reviewed them.
> git://anongit.freedesktop.org/~daniels/libx11:ge
6194a4612c2713cf289ecea4b4458117878bf112
5b93c0fff4109b20f4fcebb6b62d2c8990ba5efa
XlibInt.h first hunk, indentation seems busted.
XESetEventCookieToWire() a few extra linebreaks would be nice to
improve readability, but I guess the code maintains the style of
the rest. same with the ifdef notdef hunk.
b7b4d7e8d30956ba9c7b7b1defbad7277ff5c616
I'd prefer to see this split up into at least one more function to
make it a bit easier to read, but the code seems correct.
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net> for the
libX11 set
Cheers,
Peter
More information about the xorg-devel
mailing list