Bug: Times passed to AnimationTimer should not fluctuate

[John Hendrikx]

On 29/08/2024 20:45, Michael Strauß wrote:
> Yes, that makes sense. In any case, we shouldn't be using a system
> timer, but simply record the timestamp at v-sync, and then pass this
> precise timestamp to all AnimationTimers. It shouldn't matter when

Yeah, I still think however that this is not entirely correct either.  I 
mean, when I'm determining the position of say a bouncing ball at 60 
fps, then the frame times will be fixed numbers known in advance.  
Recording a precise time does not help my calculations here.  I mean, we 
know the frames will be displayed at:

        frame #0 at 0 ns
        frame #1 at 16,666,666 ns
        frame #2 at 33,333,332 ns
        ...

With a precise time recorded after VSync, that would become:

        frame #0 at 0 ns + small random ns value
        frame #1 at 16,666,666 ns + small random ns value
        frame #2 at 33,333,332 ns + small random ns value
        ...

There's no point in doing so, and it will subtly change the calculation 
of where the bouncing ball should be on screen (although admittedly, 
such a small ns difference won't matter, unlike the thread scheduling 
differences which are on the order of milliseconds).

Perhaps however this is the best achievable, given that even if we know 
something is 60 fps, it might in reality be 60.00002 fps, or maybe in 
reality it fluctuates slightly.  Having said that, perhaps it would be 
even better to round the value to some arbitrary precision so Animation 
code doesn't see meaningless random low order bits at all.

--John

PS. I think the issue you linked also made a small comment regarding 
this specific problem. They said this:

 > Also under the hood animation frame timestamp deltas are still weird 
(more than half of timestamp deltas are greater than 17ms). It can (and 
does) miss vsync here and there. On weak devices even increasing pulse 
frequency can not eliminate the issue completely and loads on CPU.

I think this is about the times received by AnimationTimer or the times 
used by Animation/Timeline -- these would be subject to the thread 
scheduling jitter.

So it looks like this is two separate problems.  Where the issue you 
linked is about an increasing error which will cause a jitter every X 
frames, my issue (on Windows) is a constant jitter (due to 
miscalculation by supplying a bad time value) that happens every frame.

> AnimationTimers are invoked between frames, as long as the timestamp
> corresponds to the v-sync signal. (Well, unless the timer callback
> measures its own time, which it shouldn't do.)
>
>
> On Thu, Aug 29, 2024 at 8:20 PM John Hendrikx <john.hendrikx at gmail.com> wrote:
>> I think they're a bit separate.  Even with VSync, the time it takes to kick the FX thread in action is still going to be between 0-30ms.  If it then passes `System.nanoTime()` to the AnimationRunnables, you're basically saying that they should render a frame at the precise time of VSync-Time + random time it took to schedule the FX thread... suffice to say that the extra accuracy of the more accurate VSync timer (just like my far more accurate timer) is made completely redundant by the jitter introduced by the scheduler.
>>
>> This brings me back to my original point: we should not be passing `System.nanoTime()` to AnimationRunnables.  Passing `System.nanoTime()` is basically asking to create a frame with a time index that will NEVER be rendered, so why are we asking Animations to use this value for calculating animation locations/offsets/sizes ?
>>
>> This problem is also present on Mac and Linux, just less noticeable because their schedulers generally react within 0-2 ms (vs 0-30 ms on Windows).  2 ms is "close enough" to the most commonly used frame rates (60 fps, at 16.667 ms per frame), but on Windows it can practically be a two frame difference.
>>
>> Even in the absence of V-sync, when JavaFX arbitrarily picks 60 Hz as its refresh frequency, the times passed to AnimationTimer should be multiples of 16.667 ms, not 16.667 ms + however long it took to wake up the FX thread.  In other words this code in AbstactPrimaryTimer:
>>
>> private long nextPulseTime = nanos();
>>
>> private long lastPulseDuration = Integer.MIN_VALUE;
>>
>> @Override
>>
>> public void run() {
>>
>> if (paused) {
>>
>> return;
>>
>> }
>>
>> final long now = nanos();
>>
>> recordStart((nextPulseTime - now) / 1000000);
>>
>> timePulseImpl(now);
>>
>> recordEnd();
>>
>> updateNextPulseTime(now);
>>
>> // reschedule animation runnable if needed
>>
>> updateAnimationRunnable();
>>
>> }
>>
>> ...would be far better if it passed "nextPulseTime" to `timePulseImpl` (which eventually calls the AnimationRunnables) instead of "now".
>>
>> Note: this is assuming the adaptive pulse flag is disabled.  If it is enabled, nextPulseTime won't be a nice multiple of the frame rate -- so when this is enabled we may want to round it up/down before passing it to the AnimationRunnables.
>>
>> Note 2: you can **already** achieve far smoother animation even on Windows by rounding the value you get passed in to a multiple of 1/frameRate. This only works when you have access to the this time. It won't solve Timeline calculations -- they will still calculate positions and values for frames that will never exist, subject to FX thread scheduling jitter...
>>
>> --John