Unexpected behavior in Button::setRepeatSpeed()


#1

I’m using setRepeatSpeed( 375, 150 ). I expect that 375 milliseconds after the initial click, I will get additional clicked() notifications every 150 milliseconds.

Instead, what is happening is that I am getting the first call to clicked() on the initial mouseDown, but then when the repeat kicks in I am getting TWO calls to clicked() right away. I believe this is the reason:

void Button::repeatTimerCallback()
{
        //...
        const int numTimesToCallback = (now > lastTimeCallbackTime) ? jmax (1, (int) (now - lastTimeCallbackTime) / repeatSpeed) : 1;
        //...
        for (int i = numTimesToCallback; --i >= 0;)
        //...
}

numTimesToCallback is two at this point. The code seems to be trying to do something clever - it measures how much time has passed since the initial click and divides that up by the repeat interval and uses that to determine the number of times to send a click. So in the even that the timer is imprecise, or that a lot of background activity is happening on the CPU, the average time between clicks during a repeat operation will be close to the desired value.

The problem is that I think this behavior is incorrect, one due to a bug, and two because it can lead to jerky scrolling. Sometimes you will scroll by one unit, sometimes two or more, depending on what is going on in the background and how long the rest of the interface is taking. Perhaps there are some use cases where this behavior is useful but I think it should be an option on the button.

I believe that the bug is that repeatTimerCallback() does not subtract out the initial delay (stored in autoRepeatDelay) when it calculates numTimesToCallback.

Specifically in my example, 375 milliseconds will elapse after the initial click until the timer fires the first time. At which point, int( 375 / 150 ) = 2, so numTimesToCallback ends up being 2.

So I think that the timer callback needs to know if its the first time the timer is getting called and in that case account for the value of autoRepeatDelay.

The other thing is that I think the behavior of repeat in Button with respect to having numTimesToCallback based on actual elapsed time should be an option.


#2

Interesting point, and you’ve certainly found a bit of code that I should take a look at, thanks.


#3

Looks like it is fixed in the latest tip - thanks.