[PATCH] Re-enable RECORD extension.

Peter Hutterer peter.hutterer at who-t.net
Tue Feb 16 21:42:58 PST 2010


On Thu, Feb 11, 2010 at 04:34:09PM +1000, Peter Hutterer wrote:
> From: Chris Dekter <cdekter at gmail.com>
> 
> RECORD was disabled during the switch to internal events. This patch
> modifies the record callback to work with internal events instead of
> xEvents. The InternalEvents are converted to core/Xi events as needed.
> 
> Since record is a loadable extension, the EventTo* calls must be externed.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---

ping?
We have one successful tester on the bug and Tiago's tested-by. Who wants to
take the Reviewed-by? First come, first serve - this is your chance!

Cheers,
  Peter

> This is Chris' latest patch, cleaned up and reshuffled a bit.
> I _think_ this is it now. cnee seems to report sane data and the code is
> saner than the last patch sent to the list.
> The only issue I have with this is that we've changed the definition of the
> DeviceEventInfoRec. I'm not sure if that qualifies as ABI since no-one
> except RECORD uses it, but in theory there could be other extensions that
> relied on the old behaviour.
> Though I doubt that, considering for the whole 1.7 cycle the callback wasn't
> actually invoked, so we might have heard from them by now...
> 
> 
>  Xi/exevents.c          |   13 +++--
>  dix/events.c           |   15 +++---
>  include/dix.h          |    6 +-
>  include/eventconvert.h |    6 +-
>  record/record.c        |  123 +++++++++++++++++++++++++++---------------------
>  5 files changed, 91 insertions(+), 72 deletions(-)
> 
> diff --git a/Xi/exevents.c b/Xi/exevents.c
> index cb2452b..6057c0e 100644
> --- a/Xi/exevents.c
> +++ b/Xi/exevents.c
> @@ -1031,16 +1031,19 @@ ProcessOtherEvent(InternalEvent *ev, DeviceIntPtr device)
>              break;
>      }
>  
> -#if 0
> -    /* FIXME: I'm broken. Please fix me. Thanks */
>      if (DeviceEventCallback) {
>  	DeviceEventInfoRec eventinfo;
> +	SpritePtr pSprite = device->spriteInfo->sprite;
>  
> -	eventinfo.events = (xEventPtr) xE;
> -	eventinfo.count = count;
> +	/* see comment in EnqueueEvents regarding the next three lines */
> +	if (ev->any.type == ET_Motion)
> +	    ev->device_event.root = WindowTable[pSprite->hotPhys.pScreen->myNum]->drawable.id;
> +
> +	eventinfo.device = device;
> +	eventinfo.event = ev;
>  	CallCallbacks(&DeviceEventCallback, (pointer) & eventinfo);
>      }
> -#endif
> +
>      grab = device->deviceGrab.grab;
>  
>      switch(event->type)
> diff --git a/dix/events.c b/dix/events.c
> index 85c8f9a..c085a75 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -1131,11 +1131,10 @@ EnqueueEvent(InternalEvent *ev, DeviceIntPtr device)
>          event->type == ET_KeyRelease)
>  	AccessXCancelRepeatKey(device->key->xkbInfo, event->detail.key);
>  
> -#if 0
> -        /* FIXME: I'm broken now. Please fix me. */
>      if (DeviceEventCallback)
>      {
>  	DeviceEventInfoRec eventinfo;
> +
>  	/*  The RECORD spec says that the root window field of motion events
>  	 *  must be valid.  At this point, it hasn't been filled in yet, so
>  	 *  we do it here.  The long expression below is necessary to get
> @@ -1145,14 +1144,14 @@ EnqueueEvent(InternalEvent *ev, DeviceIntPtr device)
>  	 *  the data that GetCurrentRootWindow relies on hasn't been
>  	 *  updated yet.
>  	 */
> -	if (xE->u.u.type == DeviceMotionNotify)
> -	    XE_KBPTR.root =
> -		WindowTable[pSprite->hotPhys.pScreen->myNum]->drawable.id;
> -	eventinfo.events = xE;
> -	eventinfo.count = nevents;
> +	if (ev->any.type == ET_Motion)
> +	    ev->device_event.root = WindowTable[pSprite->hotPhys.pScreen->myNum]->drawable.id;
> +
> +	eventinfo.event = ev;
> +	eventinfo.device = device;
>  	CallCallbacks(&DeviceEventCallback, (pointer)&eventinfo);
>      }
> -#endif
> +
>      if (event->type == ET_Motion)
>      {
>  #ifdef PANORAMIX
> diff --git a/include/dix.h b/include/dix.h
> index ed3acb6..6505fd0 100644
> --- a/include/dix.h
> +++ b/include/dix.h
> @@ -576,8 +576,8 @@ typedef struct {
>  extern _X_EXPORT CallbackListPtr DeviceEventCallback;
>  
>  typedef struct {
> -    xEventPtr events;
> -    int count;
> +    InternalEvent *event;
> +    DeviceIntPtr device;
>  } DeviceEventInfoRec;
>  
>  extern int XItoCoreType(int xi_type);
> @@ -585,7 +585,7 @@ extern Bool DevHasCursor(DeviceIntPtr pDev);
>  extern Bool _X_EXPORT IsPointerDevice( DeviceIntPtr dev);
>  extern Bool _X_EXPORT IsKeyboardDevice(DeviceIntPtr dev);
>  extern Bool IsPointerEvent(InternalEvent *event);
> -extern Bool IsMaster(DeviceIntPtr dev);
> +extern _X_EXPORT Bool IsMaster(DeviceIntPtr dev);
>  
>  extern _X_HIDDEN void CopyKeyClass(DeviceIntPtr device, DeviceIntPtr master);
>  extern _X_HIDDEN int CorePointerProc(DeviceIntPtr dev, int what);
> diff --git a/include/eventconvert.h b/include/eventconvert.h
> index 277a6c4..b1196a0 100644
> --- a/include/eventconvert.h
> +++ b/include/eventconvert.h
> @@ -30,9 +30,9 @@
>  
>  #define FP1616(integral, frac) ((integral) * (1 << 16) + (frac) * (1 << 16))
>  
> -_X_INTERNAL int EventToCore(InternalEvent *event, xEvent *core);
> -_X_INTERNAL int EventToXI(InternalEvent *ev, xEvent **xi, int *count);
> -_X_INTERNAL int EventToXI2(InternalEvent *ev, xEvent **xi);
> +_X_EXPORT int EventToCore(InternalEvent *event, xEvent *core);
> +_X_EXPORT int EventToXI(InternalEvent *ev, xEvent **xi, int *count);
> +_X_EXPORT int EventToXI2(InternalEvent *ev, xEvent **xi);
>  _X_INTERNAL int GetCoreType(InternalEvent* ev);
>  _X_INTERNAL int GetXIType(InternalEvent* ev);
>  _X_INTERNAL int GetXI2Type(InternalEvent* ev);
> diff --git a/record/record.c b/record/record.c
> index 242544f..1a10c9e 100644
> --- a/record/record.c
> +++ b/record/record.c
> @@ -42,6 +42,8 @@ and Jim Haggerty of Metheus.
>  #include "set.h"
>  #include "swaprep.h"
>  #include "inputstr.h"
> +#include "eventconvert.h"
> +
>  
>  #include <stdio.h>
>  #include <assert.h>
> @@ -139,7 +141,8 @@ static int RecordDeleteContext(
>      XID /*id*/
>  );
>  
> -
> +void RecordExtensionInit(void);
> +
>  /***************************************************************************/
>  
>  /* client private stuff */
> @@ -727,6 +730,59 @@ RecordADeliveredEventOrError(CallbackListPtr *pcbl, pointer nulldata, pointer ca
>  } /* RecordADeliveredEventOrError */
>  
>  
> +static void
> +RecordSendProtocolEvents(RecordClientsAndProtocolPtr pRCAP,
> +			RecordContextPtr pContext,
> +			xEvent* pev, int count)
> +{
> +    int ev; /* event index */
> +
> +    for (ev = 0; ev < count; ev++, pev++)
> +    {
> +	if (RecordIsMemberOfSet(pRCAP->pDeviceEventSet,
> +		    pev->u.u.type & 0177))
> +	{
> +	    xEvent swappedEvent;
> +	    xEvent *pEvToRecord = pev;
> +#ifdef PANORAMIX
> +	    xEvent shiftedEvent;
> +
> +	    if (!noPanoramiXExtension &&
> +		    (pev->u.u.type == MotionNotify ||
> +		     pev->u.u.type == ButtonPress ||
> +		     pev->u.u.type == ButtonRelease ||
> +		     pev->u.u.type == KeyPress ||
> +		     pev->u.u.type == KeyRelease)) {
> +		int scr = XineramaGetCursorScreen(inputInfo.pointer);
> +		memcpy(&shiftedEvent, pev, sizeof(xEvent));
> +		shiftedEvent.u.keyButtonPointer.rootX +=
> +		    panoramiXdataPtr[scr].x -
> +		    panoramiXdataPtr[0].x;
> +		shiftedEvent.u.keyButtonPointer.rootY +=
> +		    panoramiXdataPtr[scr].y -
> +		    panoramiXdataPtr[0].y;
> +		pEvToRecord = &shiftedEvent;
> +	    }
> +#endif /* PANORAMIX */
> +
> +	    if (pContext->pRecordingClient->swapped)
> +	    {
> +		(*EventSwapVector[pEvToRecord->u.u.type & 0177])
> +		    (pEvToRecord, &swappedEvent);
> +		pEvToRecord = &swappedEvent;
> +	    }
> +
> +	    RecordAProtocolElement(pContext, NULL,
> +		    XRecordFromServer,  pEvToRecord, SIZEOF(xEvent), 0);
> +	    /* make sure device events get flushed in the absence
> +	     * of other client activity
> +	     */
> +	    SetCriticalOutputPending();
> +	}
> +    } /* end for each event */
> +
> +} /* RecordADeviceEvent */
> +
>  /* RecordADeviceEvent
>   *
>   * Arguments:
> @@ -756,55 +812,24 @@ RecordADeviceEvent(CallbackListPtr *pcbl, pointer nulldata, pointer calldata)
>  	{
>  	    if (pRCAP->pDeviceEventSet)
>  	    {
> -		int ev; /* event index */
> -		xEvent *pev = pei->events;
> -		for (ev = 0; ev < pei->count; ev++, pev++)
> -		{
> -		    if (RecordIsMemberOfSet(pRCAP->pDeviceEventSet,
> -					    pev->u.u.type & 0177))
> -		    {
> -		        xEvent swappedEvent;
> -		        xEvent *pEvToRecord = pev;
> -#ifdef PANORAMIX
> -		        xEvent shiftedEvent;
> -
> -			if (!noPanoramiXExtension &&
> -			    (pev->u.u.type == MotionNotify ||
> -			     pev->u.u.type == ButtonPress ||
> -			     pev->u.u.type == ButtonRelease ||
> -			     pev->u.u.type == KeyPress ||
> -			     pev->u.u.type == KeyRelease)) {
> -				int scr = XineramaGetCursorScreen(inputInfo.pointer);
> -				memcpy(&shiftedEvent, pev, sizeof(xEvent));
> -				shiftedEvent.u.keyButtonPointer.rootX +=
> -				    panoramiXdataPtr[scr].x - 
> -					panoramiXdataPtr[0].x;
> -				shiftedEvent.u.keyButtonPointer.rootY +=
> -				    panoramiXdataPtr[scr].y -
> -					panoramiXdataPtr[0].y;
> -				pEvToRecord = &shiftedEvent;
> -			}
> -#endif /* PANORAMIX */
> +		int count;
> +		xEvent *xi_events = NULL;
>  
> -			if (pContext->pRecordingClient->swapped)
> -			{
> -			    (*EventSwapVector[pEvToRecord->u.u.type & 0177])
> -				(pEvToRecord, &swappedEvent);
> -			    pEvToRecord = &swappedEvent;
> -			}
> +		/* TODO check return values */
> +		if (IsMaster(pei->device))
> +		{
> +		    xEvent xE;
> +		    EventToCore(pei->event, &xE);
> +		    RecordSendProtocolEvents(pRCAP, pContext, &xE, 1);
> +		}
>  
> -			RecordAProtocolElement(pContext, NULL,
> -			   XRecordFromServer,  pEvToRecord, SIZEOF(xEvent), 0);
> -			/* make sure device events get flushed in the absence
> -			 * of other client activity
> -			 */
> -			SetCriticalOutputPending();
> -		    }
> -		} /* end for each event */
> +		EventToXI(pei->event, &xi_events, &count);
> +		RecordSendProtocolEvents(pRCAP, pContext, xi_events, count);
> +		xfree(xi_events);
>  	    } /* end this RCAP selects device events */
>  	} /* end for each RCAP on this context */
>      } /* end for each enabled context */
> -} /* RecordADeviceEvent */
> +}
>  
>  
>  /* RecordFlushAllContexts
> @@ -2866,13 +2891,6 @@ RecordCloseDown(ExtensionEntry *extEntry)
>  void 
>  RecordExtensionInit(void)
>  {
> -    /* FIXME Record is currently broken. Dont initialize it so that clients
> -     * that require it can bail out correctly rather than waiting for stuff
> -     * that'll never happen */
> -    ErrorF("record: RECORD extension enabled at configure time.\n");
> -    ErrorF("record: This extension is known to be broken, disabling extension now..\n");
> -    ErrorF("record: http://bugs.freedesktop.org/show_bug.cgi?id=20500\n");
> -#if 0
>      ExtensionEntry *extentry;
>  
>      RTContext = CreateNewResourceType(RecordDeleteContext, "RecordContext");
> @@ -2895,6 +2913,5 @@ RecordExtensionInit(void)
>      }
>      RecordErrorBase = extentry->errorBase;
>  
> -#endif
>  } /* RecordExtensionInit */
>  
> -- 
> 1.6.6.1


More information about the xorg-devel mailing list