[PATCH xserver 1/2] animcur: Fix transitions between animated cursors

[Aaron Plattner]

On 01/09/2018 10:08 AM, Aaron Plattner wrote:
> On 01/09/2018 08:51 AM, Adam Jackson wrote:
>> We weren't cancelling the old timer when changing cursors, making things
>> go all crashy. Logically we could always cancel the timer first, but
>> then we'd have to call TimerSet to re-arm ourselves, and GetTimeInMillis
>> is potentially expensive.
>>
>> Reported-by: https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967
>> Signed-off-by: Adam Jackson <ajax at redhat.com>
>> ---
>>  render/animcur.c | 25 +++++++++++++++----------
>>  1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/render/animcur.c b/render/animcur.c
>> index 058bc1b323..797029443c 100644
>> --- a/render/animcur.c
>> +++ b/render/animcur.c
>> @@ -151,11 +151,20 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void *arg)
>>      return ac->elts[elt].delay;
>>  }
>>  
>> +static void
>> +AnimCurCancelTimer(DeviceIntPtr pDev)
>> +{
>> +    CursorPtr cur = pDev->spriteInfo->anim.pCursor;
>> +
>> +    if (IsAnimCur(cur))
>> +        TimerCancel(GetAnimCur(cur)->timer);
>> +}
>> +
>>  static Bool
>>  AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
>>  {
>>      AnimCurScreenPtr as = GetAnimCurScreen(pScreen);
>> -    Bool ret;
>> +    Bool ret = TRUE;
>>  
>>      if (IsFloating(pDev))
>>          return FALSE;
>> @@ -165,8 +174,10 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
>>          if (pCursor != pDev->spriteInfo->anim.pCursor) {
>>              AnimCurPtr ac = GetAnimCur(pCursor);
>>  
>> -            ret = (*pScreen->DisplayCursor)
>> -                (pDev, pScreen, ac->elts[0].pCursor);
>> +            AnimCurCancelTimer(pDev);
>> +            ret = (*pScreen->DisplayCursor) (pDev, pScreen,
>> +                                             ac->elts[0].pCursor);
>> +
> 
> This is a slight change in behavior if DisplayCursor fails since it will
> cancel the previous timer whereas before it wouldn't. Are there any
> weird cases where failing to change the cursor is expected and canceling
> animation of the previous cursor would be a problem?
> 
> Assuming not, both changes
> Reviewed-by: Aaron Plattner <aplattner at nvidia.com>
> 
> I'll try to put together a build to test these today.

I re-verified the crash and verified that this series fixes it, so you
can add
Tested-by: Aaron Plattner <aplattner at nvidia.com>

>>              if (ret) {
>>                  pDev->spriteInfo->anim.elt = 0;
>>                  pDev->spriteInfo->anim.pCursor = pCursor;
>> @@ -176,15 +187,9 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
>>                                       AnimCurTimerNotify, pDev);
>>              }
>>          }
>> -        else
>> -            ret = TRUE;
>>      }
>>      else {
>> -        CursorPtr old = pDev->spriteInfo->anim.pCursor;
>> -
>> -        if (old && IsAnimCur(old))
>> -            TimerCancel(GetAnimCur(old)->timer);
>> -
>> +        AnimCurCancelTimer(pDev);
>>          pDev->spriteInfo->anim.pCursor = 0;
>>          pDev->spriteInfo->anim.pScreen = 0;
>>          ret = (*pScreen->DisplayCursor) (pDev, pScreen, pCursor);
>>
>