Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hysteresis for smoother physics update frequency #17353

Merged
merged 1 commit into from
May 7, 2018

Conversation

zmanuel
Copy link
Contributor

@zmanuel zmanuel commented Mar 7, 2018

Ensures that the number of physics iterations per render frame is as consistent as possible. Thorough fix for issue #15270.

The minimal sample fix attached on the issue itself (and this PR up to revision 470a5a0) only handles the case where the physics update FPS is an integer multiple of the monitor's refresh. The full fix handles a large range of real monitor refresh rates and 60 FPS physics updates, especially the oddly common 144Hz.

It's three separate commits still because I anticipate feedback and I can make changes better that way. I have some questions myself:

  1. Is the full fix really of general interest? It is quite bulky and I wish I could have made it more understandable. I personally don't even see the extra jitter at 144Hz monitor frequency. A special solution that handles some select integer multiples of the physics FPS would be much leaner.
  2. Do we want a floating point parameter like I implemented? An on/off switch and a well chosen threshold might be better. Or enable it permanently.

@zmanuel
Copy link
Contributor Author

zmanuel commented Mar 11, 2018

My personal answer to question 1 is "No, we don't want the complicated version" from 9a55346 . So I reverted it here and implemented the simpler version: Pretend to the basic update calculator that physics_fps is 12 times its actual value, then calculate the actual number of physics updates from that output with integer logic. This gives the base class near integer number of updates per render frame (at physics_fps 60) for many common refresh rates (all divisors of 720), including 144Hz unlike I originally thought.
In this new method, it was also trivial to add a filter to the delta argument of _process. It works much better than I would have thought:
screenshot_2
That's with multithreaded rendering. The odd dips there get almost completely ironed away.

A branch with those changes squished is here: https://github.com/zmanuel/godot/tree/timer_hysteresis_multiframe_pr1_squashed

@hpvb hpvb requested a review from reduz March 15, 2018 21:30
@zmanuel
Copy link
Contributor Author

zmanuel commented Mar 17, 2018

Ah, sorry. I was wrong. The simplified version has some bad behaviour on CPU spikes that almost cause frame drops because the low level code has no clue when actual physics steps take place. It'll go forward one step too many and compensate later. At high enough settings for physics_jtter_fix, It should either not go forward at all or not go backward. So I reverted the simplification and implemented the animation time clamping/smoothing differently.

Of course, this branch (which I haven't rebased because it's public already) has now entirely too many revisions. The good commits to merge would be:
98d63e8 the whole thing
9a55346 the original submission
572e7a9 minimal version without support for high refresh rates
Take your pick and let me know. Also tell me if it's OK if I rebase -i this branch, I'm kind of new to this github submission process.

The squished/sqhashed branch has been updated to include everything.

Usually, I'd be on IRC as Z-Man/zmanuel, but my home internet is out for probably another week.

@reduz
Copy link
Member

reduz commented Apr 8, 2018

This is interesting, but please sqash commits so it can be made more readable

Add new class _TimerSync to manage timestep calculations.
The new class handles the decisions about simulation progression
previously handled by main::iteration(). It is fed the current timer
ticks and determines how many physics updates are to be run and what
the delta argument to the _process() functions should be.

The new class tries to keep the number of physics updates per frame as
constant as possible from frame to frame. Ideally, it would be N steps
every render frame, but even with perfectly regular rendering, the
general case is that N or N+1 steps are required per frame, for some
fixed N. The best guess for N is stored in typical_physics_steps.

When determining the number of steps to take, no restrictions are
imposed between the choice of typical_physics_steps and
typical_physics_steps+1 steps. Should more or less steps than that be
required, the accumulated remaining time (as before, stored in
time_accum) needs to surpass its boundaries by some minimal threshold.
Once surpassed, typical_physics_steps is updated to allow the new step
count for future updates.

Care is taken that the modified calculation of the number of physics
steps is not observable from game code that only checks the delta
parameters to the _process and _physics_process functions; in addition
to modifying the number of steps, the _process argument is modified as
well to stay in expected bounds. Extra care is taken that the accumulated
steps still sum up to roughly the real elapsed time, up to a maximum
tolerated difference.

To allow the hysteresis code to work correctly on higher refresh
monitors, the number of typical physics steps is not only recorded and
kept consistent for single render frames, but for groups of them.
Currently, up to 12 frames are grouped that way.

The engine parameter physics_jitter_fix controls both the maximum
tolerated difference between wall clock time and summed up _process
arguments and the threshold for changing typical_physics_steps. It is
given in units of the real physics frame slice 1/physics_fps. Set
physics_jitter_fix to 0 to disable the effects of the new code here.
It starts to be effective against the random physics jitter at around
0.02 to 0.05. at values greater than 1 it starts having ill effects on
the engine's ability to react sensibly to dropped frames and framerate
changes.
@zmanuel zmanuel force-pushed the timer_hysteresis_multiframe_pr1 branch from 98d63e8 to d5abd4e Compare April 9, 2018 20:36
@zmanuel
Copy link
Contributor Author

zmanuel commented Apr 9, 2018

Sure thing, done. Unsquashed version moved to https://github.com/zmanuel/godot/commits/timer_hysteresis_multiframe_pr1_unsquashed in case further edits are required.

@reduz
Copy link
Member

reduz commented May 7, 2018

In general it seems fine. I will merge it so people can test it. I will do some internal reorganization after merging though.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 8, 2020

@zmanuel The new property lacks documentation. I vaguely know what is it for, but what does setting lower or higher value do?

@zmanuel
Copy link
Contributor Author

zmanuel commented Jan 8, 2020

I'll try to explain it as precicely as possible:
Imagine you have an ingame clock that just adds the delta arguments to _process(). Imagine you also have a realtime clock. You can then sync them up with an offset such that
realtime_clock <= ingame_clock + offset <= realtime_clock + physics_jitter_fix/physics_fps

Within these bounds, the system bends the timer to avoid fluctuations in the number of physics frames per animation frame and the delta argument to _process.

(That is the end of the minimal precise description, the rest is elaboration/rambling. Do ask specific questions or give me other terms I can try and explain things in.)

At physics_jitter_fix = 0 (or < 0), the system is off, it is not allowed to bend the timer at all.
At physics_jitter_fix = 1, you can get a maximum deviation of game and real clock of one physics frame, so usually 16.67 ms.
Two reason to not go too high with physics_jitter_fix:

  1. Network games. They often rely on clocks being in sync, and if the only clock you can build ingame can have a high deviation from a real clock, the two ingame clocks between peers can deviate by twice that.
  2. If you really have load spikes that cause you to miss frames, a high physics_jitter_fix will hide them for a while, causing a weird slow motion effect for a bit, followed by a speedup when the load goes back to normal. Values below two should be safe here.

physics_jitter_fix/physics_fps is also another significant time: Assume you have a game that is running smoothly for the most part, but every second or so, you do blocking AI processing that takes the time T to complete, fast enough that the various render buffers (the one internal to Godot, the one in the graphics driver and on the GPU itself) can keep the screen refresh going at 60 fps the whole time. If T < physics_jitter_fix/physics_fps seconds, those processing spikes will be not be observable in the game timer and rendering output (Well, the first spike may be, but the followups won't). At low physics_jitter_fix, you would see regular jerks in the motion of objects, single frames where things are a bit ahead of where they should be. Sufficiently high physics_jitter_fix allows them to be smoothed away.

The following timing assumption that always existed continues to hold: Imagine you also have a physics clock, derived from summing up all the delta arguments to _physics_process(). After the call to _process() has updated your ingame clock, you always have
physics_clock <= ingame_clock <= physics_clock + 1/physics_fps

@KoBeWi
Copy link
Member

KoBeWi commented Jan 8, 2020

I summed it up to this description:

Controls how much physic frames are synchronized with real time. For 0 or less, the frames are synchronized . Such values are recommended for network games, where clock synchronization matters. Higher values cause higher deviation of in-game clock and real clock, but allows to smooth out framerate jitters.

Hope it's correct.

EDIT:
Probably forgot to mention that I needed it for documentation purposes.

@lawnjelly
Copy link
Member

Correct me if I'm wrong but is it not possible to address the same problem (of aliasing between input deltas and physics delta) with frame delta smoothing? Is this PR actually doing frame delta smoothing (effectively)?

I.e. if your smoothed frame rate is a constant 59, and your physics is 60 tps, then you would get 1 'glitch' every second, rather than the staircasing effect from aliasing?

@jasonswearingen
Copy link

This is in relation to the Physics Jitter Fix setting.

To anyone else (like me) searching for info on what it does....

@zmanuel
Copy link
Contributor Author

zmanuel commented Jan 9, 2020

@KoBeWi Yes, this is correct. I would add a note about the sensible range (even though the default value already gives a hint):

The default of 0.5 should be fine for most; values above 2 could cause the game to react to dropped frames with a noticeable delay and are not recommended.

@lawnjelly It is a form of frame delta smoothing; after all, it takes the input frame deltas and modifies them to be more smooth. The simple 'average over N frames'-algorithms you find when searching the web would not solve the problem I developed the patch for, however (see the linked defect). They only reduce jitter by smoothing it out. This solution is much better (I have to say that, of course :) ) because in many cases, it absolutely flattens the frame delta curve. The main condition is that the physics FPS and screen refresh rate need to be in a relatively neat fractional relation (60/60 will definitely do, but 60/120, 60/144, 60/100 also work fine). Your example (60/59) is no such relation. The only effect you get there is that the delta argument to _process will always be between 1/60 and 13 / (12*60), which should be narrow enough to make any jitter unnoticeable anyway. The 'glitch' you mention never happens for the _process() calls. It does happen for the physics process, because every second, you need to cram two physics steps into one animation frame. No way around that, unless you allow the game to adapt its speed slightly to the screen refresh (definitely not good for network games).

@lawnjelly
Copy link
Member

lawnjelly commented Jan 10, 2020

Your example (60/59) is no such relation.

Ok, let's say frame rate 59.9999. This depends on the variation in measured delta of course how similar they would need to be in order to get the staggering reported in your issue.

I guess what I'm getting at is that I'm not sure why it needs to involve physics ticks in the calculations. Providing you provide a rock-steady steady delta, whatever it is, you won't get the staggering effect you mention in the issue. You will as you say get regular 'glitch' occurrence of either zero, or 2 physics ticks instead of 1, but that is to be expected.

Some of the ratios you mention (60/144, 60/100) are so far off you will always get a staircasing effect (where it would be especially important to use fixed timestep interpolation).

Maybe we are just looking at the same problem two alternative ways - one way is to fudge the incoming delta (delta smoothing), and one way is to keep leave the delta as is and fudge the number of physics ticks (which I'm getting the impression is what this PR does, I'm presuming it doesn't alter the delta? or maybe it does, there seems to be some feedback between the calculated number of physics ticks and the delta?).

it absolutely flattens the frame delta curve

I wasn't clear on what this meant. Maybe a diagram? I understand it is difficult to explain this topic, but so far, for myself at least (and I know a few others) your explanations haven't quite hit the mark.

To summarise I see the problem as following this flow:

error in measured input delta -> error in number of physics ticks done in a particular frame

Given the direction of flow, if we can fix the error in physics ticks by fixing the input delta, that seems (on the face of it) a simpler approach than trying to predict the number of physics ticks expected (from past physics ticks), then doing an additional step to back calculate an input delta. They may ultimately achieve the same thing but I'm wondering whether the code / logic might be simpler to fix the input delta?

I guess I am wondering did you consider at the time fixing the input delta, but decide that predicting the ticks was better for some reason? (not criticising btw, I just genuinely don't see the advantage yet! 😃 )

@lawnjelly
Copy link
Member

lawnjelly commented Jan 10, 2020

I summed it up to this description:

Controls how much physic frames are synchronized with real time. For 0 or less, the frames are synchronized . Such values are recommended for network games, where clock synchronization matters. Higher values cause higher deviation of in-game clock and real clock, but allows to smooth out framerate jitters.

Hope it's correct.

EDIT:
Probably forgot to mention that I needed it for documentation purposes.

Just a suggestion, the terminology of using 'frame' for a physics iteration is confusing imo. There is a well established meaning to the word frame in games, to mean a rendered frame. A physics iteration need not be related to a rendered frame (and is explicitly not related in the case of Godot fixed timestep), so a different word should be used, hence 'tick' or 'iteration' would be more sensible imo.

For instance in this paragraph you've used 'frame' to refer to a physics tick, and 'framerate' referring to render frames. Which is it? For a beginner they won't have a clue what you are on about. It is kind of like comparing apples to oranges, except comparing 'apples to apples'. 😄

@zmanuel
Copy link
Contributor Author

zmanuel commented Jan 10, 2020

Some of the ratios you mention (60/144, 60/100) are so far off you will always get a staircasing effect (where it would be especially important to use fixed timestep interpolation).

Yes, if given the choice, you should not run a game with a 60 physics updates per second at 100 or 144 hz refresh rate, especially if 120 or any other n*60 is available. The good this PR does in these cases is limited.

Maybe we are just looking at the same problem two alternative ways - one way is to fudge the incoming delta (delta smoothing), and one way is to keep leave the delta as is and fudge the number of physics ticks (which I'm getting the impression is what this PR does, I'm presuming it doesn't alter the delta? or maybe it does, there seems to be some feedback between the calculated number of physics ticks and the delta?).

Never change the delta argument to _physics_update(). There are perfectly valid numeric algorithms that only keep their promises for fixed time steps.You wouldn't want to break energy conservation :)
The primary purpose of this PR is to fudge the number of physics ticks per render frame, indeed.
But whenever you fudge that, you also need to fudge the delta argument to _update(); there is a relation between the sum of arguments to _update() and the total number of physics steps taken so far that the original code guaranteed.
And, well, once all that was in place, ironing the delta argument to _update() to be almost constant (given the right circumstances) was just 2% more code.

it absolutely flattens the frame delta curve

I wasn't clear on what this meant. Maybe a diagram?

Sure, the flattened diagram is the red dot line in the screenshot some posts up; the red dots show the delta argument to _process(). The pre-PR and WIP screenshots are in the linked issue.

To summarise I see the problem as following this flow:

error in measured input delta -> error in number of physics ticks done in a particular frame

Given the direction of flow, if we can fix the error in physics ticks by fixing the input delta, that seems (on the face of it) a simpler approach than trying to predict the number of physics ticks expected (from past physics ticks), then doing an additional step to back calculate an input delta.

Well, yeah. However, the property of the input data that triggers the issue is that the samples of total time (0s, 1/60 s, 2/60 s, 3/60 s,... ideally) fluctuate around a line of the form
sample[i] = offset + in/(mphysics_fps)
with some small integers n and m and some float offset, all constant over longer stretches of time, but any time the rendering situation changes (single frames get dropped, or performance drops from 60 fps to 30 for a bit), n,m and offset can change, too. Determining values of n and m you need to avoid is possible (this PR almost does it; it's only interested in close bounds, though), but you can't know offset without talking to the physics step calculator. If we're being really nitpicky, you can't even know physics_fps.
Dunno, I just found it more convenient to attack the problem where it is easiest do diagnose. Absolutely optimal would be to go to the real source of the timing you actually want: You should ask the GPU when the frames you gave it actually made it to the screen. Sadly, there doesn't seem to be an API for that :( (Fences come close.)

An approach along your lines of thinking that could work:
Make it so that the delta argument to _process() is always of the form n/(m*physics_fps) with integers n and m of yet to be determined constraints, while still tracking the total real time as close as possible given these constraints.

That would certainly be easier to implement, avoid the original problem and yield constant delta arguments to _process() for long stretches of time. It wouldn't work too well for the hypothetical fool who does not use the physics systems at all and sets physics_fps to 17 for some reason; with perfect 60 hz timer measurements, the system may fluctuate between updates of 1/51 and 1/68 s. The current implementation doesn't help that fool, but it also doesn't make it worse.

Anyway, that guy is only hypothetical, and his problems can be fixed with the same thing one probably should to to avoid real time and game time clock having a large deviation for a long time because no suitable delta can sync them up: adapt the limits on n and m depending on the situation. Allow larger m and n if there is a consistent timer discrepancy, or if the values you pick themselves fluctuate wildly.

You can probably guess that I could ramble on forever about this, I'd better stop here.

I grant you dips on implementing a simpler system. I admit my own fingers are itching now a bit, and it might be good to have a solution in the drawer in case anyone ever looks at this code here and decides to rip it out because nobody understands it any more.

I summed it up to this description:

Controls how much physic frames are synchronized with real time. For 0 or less, the frames are synchronized . Such values are recommended for network games, where clock synchronization matters. Higher values cause higher deviation of in-game clock and real clock, but allows to smooth out framerate jitters.

Hope it's correct.
EDIT:
Probably forgot to mention that I needed it for documentation purposes.

Just a suggestion, the terminology of using 'frame' for a physics iteration is confusing imo.

I agree with this nitpick, although the relevant setting is called physics_fps and not _ups or _tps. But anyway, for simplicity, it's better to just talk about the regular render frames here in the doc. Cut out the 'physic', and I think it should be clearer and more correct. The documentation reader does not care about this obscure physics update issue. Smoother rendering vs accurate time, that should be universally understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants