Bug: Times passed to AnimationTimer should not fluctuate

[John Hendrikx]

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:

privatelongnextPulseTime= nanos();

privatelonglastPulseDuration= Integer.MIN_VALUE;

@Override

publicvoidrun() {

if(paused) {

return;

}

finallongnow = 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


On 29/08/2024 18:18, Michael Strauß wrote:
> The entire idea of using a system timer in the presence of v-sync is
> wrong, it simply can't work reliably. We get accurate frame
> synchronization for free from Direct3D, we should use that instead of
> system timers.
>
> There's a JBS issue with a detailed analysis:
> https://bugs.openjdk.org/browse/JDK-8136536
>
>
> On Thu, Aug 29, 2024 at 5:40 PM John Hendrikx<john.hendrikx at gmail.com>  wrote:
>> TLDR; AnimationTimer is being fed a time not based on the time the next
>> frame is expected to be rendered but the time of when the FX thread is
>> finally scheduled to do the callbacks.  This time fluctuates wildly, and
>> is a time that's useless for doing frame accurate calculations of
>> animations (such a time shouldn't fluctuate at all, unless a frame gets
>> skipped).
>>
>> Longer version:
>>
>> I've been investigating an "issue" with AnimationTimers.
>>
>> Timers have a `void handle(long)` method that is called with a time in
>> nano seconds that is supposed to be the reference value to be used for
>> animations. The reference value is used to calculate in the
>> implementation how the animation should look at a specific point in time.
>>
>> However, I've found that the values being received by the AnimationTimer
>> fluctuate wildly, and the differences between the current and previous
>> calls of the time passed to the handle method can be anywhere from 1ms
>> to 30ms.
>>
>> The frequency of the calls is however still close to 60 fps -- now,
>> correct me if I'm wrong, but when drawing animations that are supposed
>> to be displayed on a 60 fps screen, shouldn't the AnimationTimer be
>> called (which is supposed to "prepare" things for the **next** frame)
>> with a relatively consistently increasing value?  (ie. 0, 16, 32, 48, 64
>> ms) instead of what I'm seeing now (0, 30, 33, 60, 66 ms).  Assuming
>> that the animation will simply be visible on the next frame, then my
>> calculations using this fluctuating timer are going to be quite jittery
>> when put on a screen that refreshes at exactly 60 fps.  My animation
>> calculations would look like:
>>
>>       |------|-|------|-|--------|-|------|-|--
>>
>> While being displayed on a screen that does this:
>>
>>       |---|---|---|---|---|---|---|---|---|---
>>
>> I compared the values I receive from `handle(long)` with
>> `System.nanoTime()` and found no significant differences between them;
>> in other words, the actual calling code is already quite a jittery process.
>>
>> So I dug a bit deeper -- internally, a subclass of Timer is used (in my
>> case WinTimer, the Windows one).  Assuming that the problem stemmed from
>> there, I replaced this with a **highly** accurate timer that simply busy
>> waits until the appropriate frame time is reached.  The accuracy of this
>> is almost nanosecond perfect (ie, within 0.000001 of a millisecond, at
>> the cost of a CPU core of course, but that's not relevant for the
>> problem).  This however had 0 impact on the accuracy of the
>> AnimationTimer -- it was still fluctuating wildly, on the order of 20-30
>> milliseconds (several million times less accurate than my Timer
>> implementation).
>>
>> Digging further, the AnimationTimer is not actually called directly from
>> the timing thread (which is a different thread than the FX thread, so
>> that makes sense).  Instead, the timer is called from the FX thread by
>> having my highly accurate timer call Application::invokeLater (similar
>> to Platform::runLater).  Once the FX thread picks up this task, it calls
>> `System.nanoTime()` as a "base" for all AnimationTimer callbacks...
>> however, this value is now distorted by how long it takes the scheduler
>> to wake up the FX thread and pick up the invokeLater task.
>>
>> I find this highly surprising.  To make jitter free animations, you want
>> a base time that increments consistently, and maybe sometimes skips a
>> frame.  So you'd expect the base time value to increment with 1 /
>> frameRate, not some arbitrary time for when the work was started
>> (fluctuating with thread scheduling delays).  I mean, I'm preparing
>> things for the **next** frame, so the time I receive should reflect when
>> that frame is likely to be drawn, not roughly be the same time my method
>> is actually being called (if I wanted that, I can
>> call `System.nanoTime()` myself...).
>>
>> I feel this is actually a bug.  It is probably a bit worse on the
>> Windows platform where the scheduler generally has a 15 ms period (which
>> explains the fluctuations I'm seeing between 0 and 30 ms). However, this
>> really shouldn't matter.  When doing animations, you want to call not
>> with the current system time, but with the expected render time of the
>> next frame.  Calling `System.nanoTime` and using that as base time is
>> just simply incorrect, although can be "close" to correct if your
>> platform schedules say with a 1 ms period (it will only fluctuate
>> between 0 and 2 ms then).
>>
>> There should however be 0 fluctuation, unless a frame was skipped.  With
>> a 60 fps frame rate, all the nano values passed to AnimationTimer should
>> be multiples of 16,666,666 ns.
>>
>> Let me know what you think, perhaps I'm missing something here.
>>
>> --John
>>
>> PS. I think it is possible to do the pulse waiting on the FX thread
>> itself; the FX thread can be interrupted when a Platform::runLater call
>> comes in, but at all other times it can just wait for the next pulse
>> directly on that thread.  With my highly accurate timer implementation
>> (I made another variant that is accurate to within a few tenths of a
>> milliseconds without busy waiting) Animation code would be called much
>> faster, so it would have more time run before the next frame is
>> displayed... at it is now, at least on Windows, the AnimationTimer can
>> get called just a few milliseconds before the next frame is displayed,
>> leaving precious little time to do any kind of calculations.
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20240829/b7e4daba/attachment-0001.htm>