It's a constant value for all handler invocations for a given frame, I
think that part is correct. It should not matter that a later handler
is doing its calculations for the next frame at a different point in
real time; it would still be doing calculations for the next frame, and
as long as it completes those before the next frame is rendered it will
be "on time".
So if I have two handlers, A and B, and A is called at (real) time X,
and B is called at (real) time X + 200 ns, they both should get a "now"
value that corresponds to the next frame to be rendered -- this doesn't
have to be derived from System.nanoTime at all. It only needs to start
at some value and be incremented for each frame rendered (including
missed frames). AnimationTimer is not documented to say that the value
is derived from nanoTime -- it is just happenstance that the value
currently used corresponds closely to System.nanoTime.
--John
On 29/08/2024 20:53, Andy Goryachev wrote:
This might be slightly off-topic, but I noticed that instead of
computing 'now' time for AnimationTimer.handle(long) just before
calling the actual handler, we do compute it once before sending to
possibly multiple handlers. Each subsequent invocation produces
progressively larger discrepancy (AbstractPrimaryTimer:264).
Does not explain the issue John raised though.
-andy
*From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of
Michael Strauß <michaelstr...@gmail.com>
*Date: *Thursday, August 29, 2024 at 11:45
*To: *
*Cc: *openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>
*Subject: *Re: Bug: Times passed to AnimationTimer should not fluctuate
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
