[PATCH v2 libXi 1/2] SizeClassInfo can return 0 even without an error
From: Niels Ole SalscheiderCatch the error case separately. Commit 19a9cd607d added length checking to SizeClassInfo but re-used the return value of 0 for an error. A device without classes (as is initialized by xf86-input-libinput for tablets) can legitimately return 0 and erroneously triggers an error. Fix this by using a separate value for the error. Reproducible by calling XListInputDevices() with a tablet attached. This fixes a regression introduced in commit 19a9cd607d. Signed-off-by: Niels Ole Salscheider Signed-off-by: Peter Hutterer --- Changes to v1: - don't touch *size until we're sure. - expand commit message Niels: I left you as author and your signed-off-by since it's essentially your patch with a minor change. src/XListDev.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/XListDev.c b/src/XListDev.c index f850cd0..e4bd3d5 100644 --- a/src/XListDev.c +++ b/src/XListDev.c @@ -73,27 +73,28 @@ static int pad_to_xid(int base_size) return ((base_size + padsize - 1)/padsize) * padsize; } -static size_t -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes) +static int +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t *size) { -int size = 0; int j; +size_t sz = 0; + for (j = 0; j < num_classes; j++) { switch ((*any)->class) { case KeyClass: -size += pad_to_xid(sizeof(XKeyInfo)); +sz += pad_to_xid(sizeof(XKeyInfo)); break; case ButtonClass: -size += pad_to_xid(sizeof(XButtonInfo)); +sz += pad_to_xid(sizeof(XButtonInfo)); break; case ValuatorClass: { xValuatorInfoPtr v; if (len < sizeof(v)) -return 0; +return 1; v = (xValuatorInfoPtr) *any; -size += pad_to_xid(sizeof(XValuatorInfo) + +sz += pad_to_xid(sizeof(XValuatorInfo) + (v->num_axes * sizeof(XAxisInfo))); break; } @@ -101,11 +102,13 @@ SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes) break; } if ((*any)->length > len) -return 0; +return 1; *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length); } -return size; +*size = sz; + +return 0; } static void @@ -220,8 +223,7 @@ XListInputDevices( sav_any = any; end = (char *)list + rlen; for (i = 0; i < *ndevices; i++, list++) { -s = SizeClassInfo(, end - (char *)any, (int)list->num_classes); -if (!s) +if(SizeClassInfo(, end - (char *)any, (int)list->num_classes, )) goto out; size += s; } -- 2.7.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH v2 libXi 2/2] XListInputDevices: don't touch ndevices in case of error
We used to always set *ndevices to the number of devices returned by the server. This magically worked because we pretty much never returned an error except on faulty server or library implementations. With 19a9cd60 we now have more chances of getting an error, so the polite thing is to just leave *ndevices alone when we error out. Document it as such in the man page, just in case someone accidentally reads it. Signed-off-by: Peter HuttererCC: Niels Ole Salscheider --- Changes to v1: - Niels' first patch set ndevices to 0, this one leaves it untouched man/XListInputDevices.txt | 12 ++-- src/XListDev.c| 21 - 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/man/XListInputDevices.txt b/man/XListInputDevices.txt index 276660d..450f377 100644 --- a/man/XListInputDevices.txt +++ b/man/XListInputDevices.txt @@ -220,5 +220,13 @@ DESCRIPTION Floating. If the device is a master device, attached specifies the device ID of the master device this device is paired with. - To free the XDeviceInfo array created by XListInputDevices, use - XFreeDeviceList. +RETURN VALUE + + + XListInputDevices returns a pointer to an array of XDeviceInfo + structs and sets ndevices_return to the number of elements in + that array. To free the XDeviceInfo array created by + XListInputDevices, use XFreeDeviceList. + + On error, XListInputDevices returns NULL and ndevices_return is + left unmodified. diff --git a/src/XListDev.c b/src/XListDev.c index e4bd3d5..dda6011 100644 --- a/src/XListDev.c +++ b/src/XListDev.c @@ -175,7 +175,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int num_classes) XDeviceInfo * XListInputDevices( register Display *dpy, -int*ndevices) +int*ndevices_return) { size_t s, size; xListInputDevicesReq *req; @@ -190,6 +190,7 @@ XListInputDevices( int i; unsigned long rlen; XExtDisplayInfo *info = XInput_find_display(dpy); +int ndevices; LockDisplay(dpy); if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1) @@ -205,8 +206,8 @@ XListInputDevices( return (XDeviceInfo *) NULL; } -if ((*ndevices = rep.ndevices)) { /* at least 1 input device */ - size = *ndevices * sizeof(XDeviceInfo); +if ((ndevices = rep.ndevices)) { /* at least 1 input device */ + size = ndevices * sizeof(XDeviceInfo); if (rep.length < (INT_MAX >> 2)) { rlen = rep.length << 2; /* multiply length by 4*/ slist = list = Xmalloc(rlen); @@ -219,17 +220,17 @@ XListInputDevices( } _XRead(dpy, (char *)list, rlen); - any = (xAnyClassPtr) ((char *)list + (*ndevices * sizeof(xDeviceInfo))); + any = (xAnyClassPtr) ((char *)list + (ndevices * sizeof(xDeviceInfo))); sav_any = any; end = (char *)list + rlen; - for (i = 0; i < *ndevices; i++, list++) { + for (i = 0; i < ndevices; i++, list++) { if(SizeClassInfo(, end - (char *)any, (int)list->num_classes, )) goto out; size += s; } Nptr = ((unsigned char *)list) + rlen; - for (i = 0, nptr = (unsigned char *)any; i < *ndevices; i++) { + for (i = 0, nptr = (unsigned char *)any; i < ndevices; i++) { if (nptr >= Nptr) goto out; size += *nptr + 1; @@ -245,10 +246,10 @@ XListInputDevices( } sclist = clist; Any = (XAnyClassPtr) ((char *)clist + - (*ndevices * sizeof(XDeviceInfo))); + (ndevices * sizeof(XDeviceInfo))); list = slist; any = sav_any; - for (i = 0; i < *ndevices; i++, list++, clist++) { + for (i = 0; i < ndevices; i++, list++, clist++) { clist->type = list->type; clist->id = list->id; clist->use = list->use; @@ -261,7 +262,7 @@ XListInputDevices( clist = sclist; nptr = (unsigned char *)any; Nptr = (unsigned char *)Any; - for (i = 0; i < *ndevices; i++, clist++) { + for (i = 0; i < ndevices; i++, clist++) { clist->name = (char *)Nptr; memcpy(Nptr, nptr + 1, *nptr); Nptr += (*nptr); @@ -270,6 +271,8 @@ XListInputDevices( } } +*ndevices_return = ndevices; + out: XFree((char *)slist); UnlockDisplay(dpy); -- 2.7.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi] SizeClassInfo can return 0 even without an error
On Fri, Oct 07, 2016 at 09:46:44PM +0200, Niels Ole Salscheider wrote: > Catch the error case separately. This fixes a few crashes on my computer. others have mentioned this already, but "fixes a few crashes" is too generic for a commit message. you should always explain *why* you're doing something in as much detail as required for someone to reproduce or at least understand the issue. in a few months time, most of us will have forgotten what this patch was about and then rely on the commit message to refresh our memory. > Signed-off-by: Niels Ole Salscheider> --- > src/XListDev.c | 21 ++--- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/src/XListDev.c b/src/XListDev.c > index f850cd0..d0c6bf2 100644 > --- a/src/XListDev.c > +++ b/src/XListDev.c > @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size) > return ((base_size + padsize - 1)/padsize) * padsize; > } > > -static size_t > -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes) > +static int > +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t *size) > { > -int size = 0; > int j; > +*size = 0; happy to merge this with this bit removed and adjusted so that size only changes in the success case. but I will need a commit message that explains bits in more detail. Cheers, Peter > for (j = 0; j < num_classes; j++) { > switch ((*any)->class) { > case KeyClass: > -size += pad_to_xid(sizeof(XKeyInfo)); > +*size += pad_to_xid(sizeof(XKeyInfo)); > break; > case ButtonClass: > -size += pad_to_xid(sizeof(XButtonInfo)); > +*size += pad_to_xid(sizeof(XButtonInfo)); > break; > case ValuatorClass: > { > xValuatorInfoPtr v; > > if (len < sizeof(v)) > -return 0; > +return 1; > v = (xValuatorInfoPtr) *any; > -size += pad_to_xid(sizeof(XValuatorInfo) + > +*size += pad_to_xid(sizeof(XValuatorInfo) + > (v->num_axes * sizeof(XAxisInfo))); > break; > } > @@ -101,11 +101,11 @@ SizeClassInfo(xAnyClassPtr *any, size_t len, int > num_classes) > break; > } > if ((*any)->length > len) > -return 0; > +return 1; > *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length); > } > > -return size; > +return 0; > } > > static void > @@ -220,8 +220,7 @@ XListInputDevices( > sav_any = any; > end = (char *)list + rlen; > for (i = 0; i < *ndevices; i++, list++) { > -s = SizeClassInfo(, end - (char *)any, > (int)list->num_classes); > -if (!s) > +if(SizeClassInfo(, end - (char *)any, > (int)list->num_classes, )) > goto out; > size += s; > } > -- > 2.10.1 > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi] SizeClassInfo can return 0 even without an error
On Fri, Oct 07, 2016 at 09:46:44PM +0200, Niels Ole Salscheider wrote: > Catch the error case separately. This fixes a few crashes on my computer. others have mentioned this already, but "fixes a few crashes" is too generic for a commit message. you should always explain *why* you're doing something in as much detail as required for someone to reproduce or at least understand the issue. in a few months time, most of us will have forgotten what this patch was about and then rely on the commit message to refresh our memory. > Signed-off-by: Niels Ole Salscheider> --- > src/XListDev.c | 21 ++--- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/src/XListDev.c b/src/XListDev.c > index f850cd0..d0c6bf2 100644 > --- a/src/XListDev.c > +++ b/src/XListDev.c > @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size) > return ((base_size + padsize - 1)/padsize) * padsize; > } > > -static size_t > -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes) > +static int > +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t *size) > { > -int size = 0; > int j; > +*size = 0; happy to merge this with this bit removed and adjusted so that size only changes in the success case. but I will need a commit message that explains bits in more detail. Cheers, Peter > for (j = 0; j < num_classes; j++) { > switch ((*any)->class) { > case KeyClass: > -size += pad_to_xid(sizeof(XKeyInfo)); > +*size += pad_to_xid(sizeof(XKeyInfo)); > break; > case ButtonClass: > -size += pad_to_xid(sizeof(XButtonInfo)); > +*size += pad_to_xid(sizeof(XButtonInfo)); > break; > case ValuatorClass: > { > xValuatorInfoPtr v; > > if (len < sizeof(v)) > -return 0; > +return 1; > v = (xValuatorInfoPtr) *any; > -size += pad_to_xid(sizeof(XValuatorInfo) + > +*size += pad_to_xid(sizeof(XValuatorInfo) + > (v->num_axes * sizeof(XAxisInfo))); > break; > } > @@ -101,11 +101,11 @@ SizeClassInfo(xAnyClassPtr *any, size_t len, int > num_classes) > break; > } > if ((*any)->length > len) > -return 0; > +return 1; > *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length); > } > > -return size; > +return 0; > } > > static void > @@ -220,8 +220,7 @@ XListInputDevices( > sav_any = any; > end = (char *)list + rlen; > for (i = 0; i < *ndevices; i++, list++) { > -s = SizeClassInfo(, end - (char *)any, > (int)list->num_classes); > -if (!s) > +if(SizeClassInfo(, end - (char *)any, > (int)list->num_classes, )) > goto out; > size += s; > } > -- > 2.10.1 > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Handle animated cursor on shared sprite
Hi Pierre, On 12 October 2016 at 10:05, Pierre Ossmanwrote: > Sprites (and hence cursors) can be shared between multiple devices. > However the animation code was not prepared for this and could wind > up in a case where it would continue to animate a free:d cursor. > --- > include/inputstr.h | 14 - > render/animcur.c | 85 > +- > 2 files changed, 51 insertions(+), 48 deletions(-) > > diff --git a/include/inputstr.h b/include/inputstr.h > index 568f5f9..a485f5e 100644 > --- a/include/inputstr.h > +++ b/include/inputstr.h > @@ -246,6 +246,12 @@ typedef struct _SpriteRec { > ScreenPtr pEnqueueScreen; > ScreenPtr pDequeueScreen; > +/* keep states for animated cursor */ > +struct { > +ScreenPtr pScreen; > +int elt; > +CARD32 time; > +} anim; > } SpriteRec; Note that this changes the ABI so it might not be good for the point releases. Unless one wants to make things extra 'fun' for binary only drivers ;-) Haven't looked at the patch in detail, so I cannot comment the issue can be fixed without breaking the ABI. -Emil ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Handle animated cursor on shared sprite
Am 12.10.2016 11:05, schrieb Pierre Ossman: > Sprites (and hence cursors) can be shared between multiple devices. > However the animation code was not prepared for this and could wind > up in a case where it would continue to animate a free:d cursor. > --- > include/inputstr.h | 14 - > render/animcur.c | 85 > +- > 2 files changed, 51 insertions(+), 48 deletions(-) > > diff --git a/include/inputstr.h b/include/inputstr.h > index 568f5f9..a485f5e 100644 > --- a/include/inputstr.h > +++ b/include/inputstr.h > @@ -246,6 +246,12 @@ typedef struct _SpriteRec { > ScreenPtr pEnqueueScreen; > ScreenPtr pDequeueScreen; > +/* keep states for animated cursor */ > +struct { > +ScreenPtr pScreen; > +int elt; > +CARD32 time; > +} anim; > } SpriteRec; > typedef struct _KeyClassRec { > @@ -509,14 +515,6 @@ typedef struct _SpriteInfoRec { > DeviceIntPtr paired;/* The paired device. Keyboard if > spriteOwner is TRUE, otherwise the > pointer that owns the sprite. */ > - > -/* keep states for animated cursor */ > -struct { > -CursorPtr pCursor; > -ScreenPtr pScreen; > -int elt; > -CARD32 time; > -} anim; > } SpriteInfoRec, *SpriteInfoPtr; > /* device types */ > diff --git a/render/animcur.c b/render/animcur.c > index 52e6b8b..fc87e0d 100644 > --- a/render/animcur.c > +++ b/render/animcur.c > @@ -142,35 +142,42 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void > *arg) > CARD32 soonest = ~0; /* earliest time to wakeup again */ > for (dev = inputInfo.devices; dev; dev = dev->next) { > -if (IsPointerDevice(dev) && pScreen == > dev->spriteInfo->anim.pScreen) { > -if (!activeDevice) > -activeDevice = TRUE; > - > -if ((INT32) (now - dev->spriteInfo->anim.time) >= 0) { > -AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor); > -int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt; > -DisplayCursorProcPtr DisplayCursor; > - > -/* > - * Not a simple Unwrap/Wrap as this > - * isn't called along the DisplayCursor > - * wrapper chain. > - */ > -DisplayCursor = pScreen->DisplayCursor; > -pScreen->DisplayCursor = as->DisplayCursor; > -(void) (*pScreen->DisplayCursor) (dev, > - pScreen, > - ac->elts[elt].pCursor); > -as->DisplayCursor = pScreen->DisplayCursor; > -pScreen->DisplayCursor = DisplayCursor; > - > -dev->spriteInfo->anim.elt = elt; > -dev->spriteInfo->anim.time = now + ac->elts[elt].delay; > -} > - > -if (soonest > dev->spriteInfo->anim.time) > -soonest = dev->spriteInfo->anim.time; > +if (!IsPointerDevice(dev)) > +continue; > +if (!dev->spriteInfo->spriteOwner) > +continue; > +if (!IsAnimCur(dev->spriteInfo->sprite->current)) > +continue; > +if (pScreen != dev->spriteInfo->sprite->anim.pScreen) > +continue; > + > +if (!activeDevice) > +activeDevice = TRUE; why the check here ? just set. > + > +if ((INT32) (now - dev->spriteInfo->sprite->anim.time) >= 0) { > +AnimCurPtr ac = GetAnimCur(dev->spriteInfo->sprite->current); > +int elt = (dev->spriteInfo->sprite->anim.elt + 1) % ac->nelt; > +DisplayCursorProcPtr DisplayCursor; > + > +/* > + * Not a simple Unwrap/Wrap as this > + * isn't called along the DisplayCursor > + * wrapper chain. > + */ > +DisplayCursor = pScreen->DisplayCursor; > +pScreen->DisplayCursor = as->DisplayCursor; > +(void) (*pScreen->DisplayCursor) (dev, > + pScreen, > + ac->elts[elt].pCursor); > +as->DisplayCursor = pScreen->DisplayCursor; > +pScreen->DisplayCursor = DisplayCursor; > + > +dev->spriteInfo->sprite->anim.elt = elt; > +dev->spriteInfo->sprite->anim.time = now + ac->elts[elt].delay; > } > + > +if (soonest > dev->spriteInfo->sprite->anim.time) > +soonest = dev->spriteInfo->sprite->anim.time; > } > if (activeDevice) > @@ -192,17 +199,17 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr > pScreen, CursorPtr pCursor) > Unwrap(as, pScreen, DisplayCursor); > if (IsAnimCur(pCursor)) { > -if (pCursor != pDev->spriteInfo->anim.pCursor) { > +
[PATCH xserver] inputthread: Re-add fd to the inputThreadInfo->fds pollfd set when re-added
If the following call sequence happens: 1) InputThreadUnregisterDev(fd) 2) close(fd) 3) fd = open(...) /* returns same fd as just closed */ 4) InputThreadRegisterDev(fd, ...) Without InputThreadDoWork(); running in the mean time, then we would keep the closed fd in the inputThreadInfo->fds pollfd set, rather then removing it and adding the new one, causing some devices to not work after a vt-switch when using xf86-input-evdev. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=97880 Reported-and-tested-by: Mihail KonevSigned-off-by: Hans de Goede --- os/inputthread.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/os/inputthread.c b/os/inputthread.c index ab1559f..c20c21c 100644 --- a/os/inputthread.c +++ b/os/inputthread.c @@ -49,6 +49,7 @@ Bool InputThreadEnable = TRUE; typedef enum _InputDeviceState { device_state_added, +device_state_re_added, device_state_running, device_state_removed } InputDeviceState; @@ -206,8 +207,8 @@ InputThreadRegisterDev(int fd, if (dev) { dev->readInputProc = readInputProc; dev->readInputArgs = readInputArgs; -/* Override possible unhandled state == device_state_removed */ -dev->state = device_state_running; +if (dev->state == device_state_removed) +dev->state = device_state_re_added; } else { dev = calloc(1, sizeof(InputThreadDevice)); if (dev == NULL) { @@ -344,6 +345,16 @@ InputThreadDoWork(void *arg) ospoll_listen(inputThreadInfo->fds, dev->fd, X_NOTIFY_READ); dev->state = device_state_running; break; +case device_state_re_added: +/* Device might use a new fd with the same number */ +ospoll_remove(inputThreadInfo->fds, dev->fd); +ospoll_add(inputThreadInfo->fds, dev->fd, + ospoll_trigger_level, + InputReady, + dev); +ospoll_listen(inputThreadInfo->fds, dev->fd, X_NOTIFY_READ); +dev->state = device_state_running; +break; case device_state_running: break; case device_state_removed: -- 2.9.3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] os: add an optional offset to -displayfd parameter
Hi, On Mon, Jun 13, 2016 at 11:44:55PM +0700, Antoine Martin wrote: > Reviewed-by: Antoine Martinquite some time past since my initial submit. What are the next steps to get this merged? Did I miss anything? Thank you. So long, Bernhard ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option
On 11 October 2016 at 14:34, Jon Turneywrote: > On 10/10/2016 19:31, Mihail Konev wrote: >> >> Hello. >> >> On Mon Oct 10 14:59:52 UTC 2016, Jon Turney wrote: >>> >>> @@ -16,11 +20,10 @@ AM_CPPFLAGS = \ >>> -I$(top_srcdir)/hw/xfree86/os-support/bus \ >>> -I$(top_srcdir)/hw/xfree86/common \ >>> -I$(top_srcdir)/hw/xfree86/dri \ >>> + -I$(top_srcdir)/hw/xfree86/dri2 >>> -I$(top_srcdir)/mi \ >>> -I$(top_srcdir)/present >> >> >> Shouldn't there be a backslash? > > > Absolutely! Revised patch attached. > One could have kept the include path as-is or even wrap it in if DRI2 (like the original code). Either way, the updated patch looks correct and is Reviewed-by: Emil Velikov Emil ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Handle animated cursor on shared sprite
This was causing crashes in TigerVNC with Chrome/Chromium. Regards -- Pierre Ossman Software Development Cendio AB https://cendio.com Teknikringen 8 https://twitter.com/ThinLinc 583 30 Linköpinghttps://facebook.com/ThinLinc Phone: +46-13-214600https://plus.google.com/+CendioThinLinc A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] Handle animated cursor on shared sprite
Sprites (and hence cursors) can be shared between multiple devices. However the animation code was not prepared for this and could wind up in a case where it would continue to animate a free:d cursor. --- include/inputstr.h | 14 - render/animcur.c | 85 +- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/include/inputstr.h b/include/inputstr.h index 568f5f9..a485f5e 100644 --- a/include/inputstr.h +++ b/include/inputstr.h @@ -246,6 +246,12 @@ typedef struct _SpriteRec { ScreenPtr pEnqueueScreen; ScreenPtr pDequeueScreen; +/* keep states for animated cursor */ +struct { +ScreenPtr pScreen; +int elt; +CARD32 time; +} anim; } SpriteRec; typedef struct _KeyClassRec { @@ -509,14 +515,6 @@ typedef struct _SpriteInfoRec { DeviceIntPtr paired;/* The paired device. Keyboard if spriteOwner is TRUE, otherwise the pointer that owns the sprite. */ - -/* keep states for animated cursor */ -struct { -CursorPtr pCursor; -ScreenPtr pScreen; -int elt; -CARD32 time; -} anim; } SpriteInfoRec, *SpriteInfoPtr; /* device types */ diff --git a/render/animcur.c b/render/animcur.c index 52e6b8b..fc87e0d 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -142,35 +142,42 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void *arg) CARD32 soonest = ~0; /* earliest time to wakeup again */ for (dev = inputInfo.devices; dev; dev = dev->next) { -if (IsPointerDevice(dev) && pScreen == dev->spriteInfo->anim.pScreen) { -if (!activeDevice) -activeDevice = TRUE; - -if ((INT32) (now - dev->spriteInfo->anim.time) >= 0) { -AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor); -int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt; -DisplayCursorProcPtr DisplayCursor; - -/* - * Not a simple Unwrap/Wrap as this - * isn't called along the DisplayCursor - * wrapper chain. - */ -DisplayCursor = pScreen->DisplayCursor; -pScreen->DisplayCursor = as->DisplayCursor; -(void) (*pScreen->DisplayCursor) (dev, - pScreen, - ac->elts[elt].pCursor); -as->DisplayCursor = pScreen->DisplayCursor; -pScreen->DisplayCursor = DisplayCursor; - -dev->spriteInfo->anim.elt = elt; -dev->spriteInfo->anim.time = now + ac->elts[elt].delay; -} - -if (soonest > dev->spriteInfo->anim.time) -soonest = dev->spriteInfo->anim.time; +if (!IsPointerDevice(dev)) +continue; +if (!dev->spriteInfo->spriteOwner) +continue; +if (!IsAnimCur(dev->spriteInfo->sprite->current)) +continue; +if (pScreen != dev->spriteInfo->sprite->anim.pScreen) +continue; + +if (!activeDevice) +activeDevice = TRUE; + +if ((INT32) (now - dev->spriteInfo->sprite->anim.time) >= 0) { +AnimCurPtr ac = GetAnimCur(dev->spriteInfo->sprite->current); +int elt = (dev->spriteInfo->sprite->anim.elt + 1) % ac->nelt; +DisplayCursorProcPtr DisplayCursor; + +/* + * Not a simple Unwrap/Wrap as this + * isn't called along the DisplayCursor + * wrapper chain. + */ +DisplayCursor = pScreen->DisplayCursor; +pScreen->DisplayCursor = as->DisplayCursor; +(void) (*pScreen->DisplayCursor) (dev, + pScreen, + ac->elts[elt].pCursor); +as->DisplayCursor = pScreen->DisplayCursor; +pScreen->DisplayCursor = DisplayCursor; + +dev->spriteInfo->sprite->anim.elt = elt; +dev->spriteInfo->sprite->anim.time = now + ac->elts[elt].delay; } + +if (soonest > dev->spriteInfo->sprite->anim.time) +soonest = dev->spriteInfo->sprite->anim.time; } if (activeDevice) @@ -192,17 +199,17 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) Unwrap(as, pScreen, DisplayCursor); if (IsAnimCur(pCursor)) { -if (pCursor != pDev->spriteInfo->anim.pCursor) { +if (pDev->spriteInfo->spriteOwner && +(pCursor != pDev->spriteInfo->sprite->current)) { AnimCurPtr ac = GetAnimCur(pCursor); ret = (*pScreen->DisplayCursor) (pDev, pScreen, ac->elts[0].pCursor); if (ret) { -pDev->spriteInfo->anim.elt = 0; -