Skip to content

Commit

Permalink
Fix negative delta arguments
Browse files Browse the repository at this point in the history
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below zero. That could happen
   on physics_jitter_fix changes or heavily fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes #26887
  • Loading branch information
zmanuel committed Feb 15, 2020
1 parent 53cf289 commit c0c900c
Showing 1 changed file with 62 additions and 1 deletion.
63 changes: 62 additions & 1 deletion main/main_timer_sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@

#include "main_timer_sync.h"

#ifdef DEBUG_ENABLED
// enable to get extra diagnostics from here
// #define SYNC_TIMER_DEBUG_ENABLED
#endif

void MainFrameTime::clamp_idle(float min_idle_step, float max_idle_step) {
if (idle_step < min_idle_step) {
idle_step = min_idle_step;
Expand Down Expand Up @@ -57,7 +62,7 @@ int MainTimerSync::get_average_physics_steps(float &p_min, float &p_max) {
const float typical_lower = typical_physics_steps[i];
const float current_min = typical_lower / (i + 1);
if (current_min > p_max)
return i; // bail out of further restrictions would void the interval
return i; // bail out if further restrictions would void the interval
else if (current_min > p_min)
p_min = current_min;
const float current_max = (typical_lower + 1) / (i + 1);
Expand Down Expand Up @@ -101,6 +106,15 @@ MainFrameTime MainTimerSync::advance_core(float p_frame_slice, int p_iterations_
max_typical_steps = steps_left_to_match_typical + 1;
}

#ifdef DEBUG_ENABLED
if (max_typical_steps < 0) {
WARN_PRINT_ONCE("max_typical_steps is negative");
}
if (min_typical_steps < 0) {
WARN_PRINT_ONCE("min_typical_steps is negative");
}
#endif

// try to keep it consistent with previous iterations
if (ret.physics_steps < min_typical_steps) {
const int max_possible_steps = floor((time_accum)*p_iterations_per_second + get_physics_jitter_fix());
Expand All @@ -120,6 +134,15 @@ MainFrameTime MainTimerSync::advance_core(float p_frame_slice, int p_iterations_
}
}

if (ret.physics_steps < 0) {
#ifdef SYNC_TIMER_DEBUG_ENABLED
// negative steps can only happen if either the real clock runs backwards (caught there)
// or the jitter_fix setting gets changed on the fly.
WARN_PRINT_ONCE("negative physics step calculated");
#endif
ret.physics_steps = 0;
}

time_accum -= ret.physics_steps * p_frame_slice;

// keep track of accumulated step counts
Expand All @@ -143,6 +166,10 @@ MainFrameTime MainTimerSync::advance_core(float p_frame_slice, int p_iterations_

// calls advance_core, keeps track of deficit it adds to animaption_step, make sure the deficit sum stays close to zero
MainFrameTime MainTimerSync::advance_checked(float p_frame_slice, int p_iterations_per_second, float p_idle_step) {
if (p_idle_step <= 0) {
WARN_PRINT_ONCE("p_idle_step not positive");
}

if (fixed_fps != -1)
p_idle_step = 1.0 / fixed_fps;

Expand Down Expand Up @@ -172,9 +199,43 @@ MainFrameTime MainTimerSync::advance_checked(float p_frame_slice, int p_iteratio
// last clamping: make sure time_accum is between 0 and p_frame_slice for consistency between physics and idle
ret.clamp_idle(idle_minus_accum, idle_minus_accum + p_frame_slice);

// all the operations above may have turned ret.idle_step negative, clamp to zero
if (ret.idle_step < 0) {
#ifdef SYNC_TIMER_DEBUG_ENABLED
WARN_PRINT_ONCE("negative animation timestep calculated");
#endif
ret.idle_step = 0;
}

// restore time_accum
time_accum = ret.idle_step - idle_minus_accum;

// forcing ret.idle_step to be positive may trigger a violation of the
// promise that time_accum is between 0 and p_frame_slice
#ifdef DEBUG_ENABLED
if (time_accum < -1E-7) {
WARN_PRINT_ONCE("time_accum negative");
}
#endif

if (time_accum > p_frame_slice) {
#ifdef SYNC_TIMER_DEBUG_ENABLED
WARN_PRINT_ONCE("extra physics steps required to avoid negative timesteps");
#endif
const int extra_physics_steps = floor(time_accum * p_iterations_per_second);
time_accum -= extra_physics_steps * p_frame_slice;
ret.physics_steps += extra_physics_steps;
}

#ifdef DEBUG_ENABLED
if (time_accum < -1E-7) {
WARN_PRINT_ONCE("time_accum negative");
}
if (time_accum > p_frame_slice + 1E-7) {
WARN_PRINT_ONCE("time_accum larger than p_frame_slice");
}
#endif

// track deficit
time_deficit = p_idle_step - ret.idle_step;

Expand Down

0 comments on commit c0c900c

Please sign in to comment.