[PATCH] Handle animated cursor on shared sprite

[Alan Hourihane]

Keith/Adam,

Another patch for cursor issues that can cause the Xserver to crash
that's over 12 months old and hasn't been committed or commented on.

Can you take a look at this too ?

Thanks,

Alan.


On 12/10/16 10:05, Pierre Ossman wrote:
> 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;
> -                pDev->spriteInfo->anim.time =
> +                pDev->spriteInfo->sprite->anim.elt = 0;
> +                pDev->spriteInfo->sprite->anim.time =
>                      GetTimeInMillis() + ac->elts[0].delay;
> -                pDev->spriteInfo->anim.pCursor = pCursor;
> -                pDev->spriteInfo->anim.pScreen = pScreen;
> +                pDev->spriteInfo->sprite->anim.pScreen = pScreen;
>                   if (!as->timer_set) {
>                      TimerSet(as->timer, TimerAbsolute, pDev->spriteInfo->anim.time,
> @@ -214,11 +221,8 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
>          else
>              ret = TRUE;
>      }
> -    else {
> -        pDev->spriteInfo->anim.pCursor = 0;
> -        pDev->spriteInfo->anim.pScreen = 0;
> +    else
>          ret = (*pScreen->DisplayCursor) (pDev, pScreen, pCursor);
> -    }
>      Wrap(as, pScreen, DisplayCursor, AnimCurDisplayCursor);
>      return ret;
>  }
> @@ -231,8 +235,9 @@ AnimCurSetCursorPosition(DeviceIntPtr pDev,
>      Bool ret;
>       Unwrap(as, pScreen, SetCursorPosition);
> -    if (pDev->spriteInfo->anim.pCursor) {
> -        pDev->spriteInfo->anim.pScreen = pScreen;
> +    if (pDev->spriteInfo->spriteOwner &&
> +        IsAnimCur(pDev->spriteInfo->sprite->current)) {
> +        pDev->spriteInfo->sprite->anim.pScreen = pScreen;
>           if (!as->timer_set) {
>              TimerSet(as->timer, TimerAbsolute, pDev->spriteInfo->anim.time,
> @@ -296,7 +301,7 @@ AnimCurRecolorCursor(DeviceIntPtr pDev,
>          for (i = 0; i < ac->nelt; i++)
>              (*pScreen->RecolorCursor) (pDev, pScreen, ac->elts[i].pCursor,
>                                         displayed &&
> -                                       pDev->spriteInfo->anim.elt == i);
> +                                       pDev->spriteInfo->sprite->anim.elt == i);
>      }
>      else
>          (*pScreen->RecolorCursor) (pDev, pScreen, pCursor, displayed);