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
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.hendr...@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
