ScreenSaver SetTickSize(): apparent bug? and patch

[rogueeve]

I believe I may have found a bug in the ScreenSaverRunner (/src/kits/screensaver/ScreenSaverRunner.cpp:202).

The Be Book states in regards to the SetTickSize() function:

<< The default tick size is 50000 (50 milliseconds). Setting the tick size to 0 causes Draw()/DirectDraw() to be called with almost no delay; in reality, there will be some variable delay depending on scheduling latency >>

-- which implies that there is no specific minimum tick rate. However, the replacement ScreenSaverRunner used by Haiku is unable to honor tick sizes smaller than 50ms, and will never tick at a rate faster than 50ms. If you check the code:

const uint32 kTickBase = 50000;

<--snip-->

break the idle time up into ticks so that we can evaluate

the quit condition with greater responsiveness

otherwise a screen saver that sets, say, a 30 second tick

will result in the screen saver not responding to deactivation

for that length of time

snooze(kTickBase);

if (system_time() - lastTickTime < tick)

continue;

else {

re-evaluate the tick time after each successful wakeup -

screensavers can adjust it on the fly and we must be prepared to accomodate that

tick = fSaver ? fSaver->TickSize() : kTickBase;

lastTickTime = system_time();

}

If the screen saver sets the tick size smaller than kTickBase (50000us), then the runner will see it but stills snoozes for 50000 before checking if the time is up. This prevents the screensaver from running at it's requested rate.

The addition of something such as

if (tick < kTickBase) kTickBase = tick;

else if (kTickBase < 50000 && tick >= 50000) kTickBase = 50000;

after "tick = fSaver ? fSaver->TickSize() : kTickBase", should solve the issue. Of course a #define to replace 50000 might be nice as well. I assume the "k" in "kTickBase" is hungarian notation for "constant", so that might would want to be changed also.

[leavengood]

I agree this is a bug as far as being compatible with BeOS, but it sure seems unwise for a screensaver to be animating at such a high rate. I know I would be annoyed with and would not use a screensaver that consumed more CPU on my idle computer than is used when I'm using it.

But it isn't up to Haiku to decide how fast a screensaver should animate, so I'm OK with this getting fixed.

I'd take ownership but I don't have time at the moment.

[rogueeve]

Obviously setting the rate to 0 as mentioned in the book is probably a sign of a really-not-very-nice screensaver, but I don't think reasonable rates other than that are necessarily cpu hogs-- you also have to take into account how much work is being done per tick. In my case I was porting a simple screensaver from Linux which does an animation every 10 milliseconds. Running flat out; I got >2000fps out of this thing, so that gives you an idea how little work is being done per frame at the usual frame rate.

I applied the patch I suggested to my own build and it does work.

[leavengood]

OK, you make a good point. Could you create a patch from your changes and attach it to this ticket? That way it will be extra easy to fix. Usually we try to create patches from the root of trunk, so:

svn diff src/kits/screensaver > ScreenSaverRunnerFix_4628.diff

[rogueeve]

Sure; give that a try.

[leavengood]

Fixed in hrev33431.