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 1/8th of the input step.
   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

Co-authored-by: Hugo Locurcio <[email protected]>
  • Loading branch information
zmanuel and Calinou committed Mar 21, 2021
1 parent 07f076f commit 7f39c8f
Showing 1 changed file with 43 additions and 2 deletions.
45 changes: 43 additions & 2 deletions main/main_timer_sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,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;
}
Expand Down Expand Up @@ -105,6 +105,12 @@ MainFrameTime MainTimerSync::advance_core(float p_physics_step, int p_physics_fp
}
}

#ifdef DEBUG_ENABLED
if (max_typical_steps < 0) {
WARN_PRINT_ONCE("`max_typical_steps` is negative. This could hint at an engine bug or system timer misconfiguration.");
}
#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_physics_fps + get_physics_jitter_fix());
Expand All @@ -124,6 +130,10 @@ MainFrameTime MainTimerSync::advance_core(float p_physics_step, int p_physics_fp
}
}

if (ret.physics_steps < 0) {
ret.physics_steps = 0;
}

time_accum -= ret.physics_steps * p_physics_step;

// keep track of accumulated step counts
Expand Down Expand Up @@ -151,6 +161,9 @@ MainFrameTime MainTimerSync::advance_checked(float p_physics_step, int p_physics
p_process_step = 1.0 / fixed_fps;
}

float min_output_step = p_process_step / 8;
min_output_step = MAX(min_output_step, 1E-6);

// compensate for last deficit
p_process_step += time_deficit;

Expand All @@ -170,16 +183,44 @@ MainFrameTime MainTimerSync::advance_checked(float p_physics_step, int p_physics
}
}

// second clamping: keep abs(time_deficit) < jitter_fix * frame_slise
// second clamping: keep abs(time_deficit) < jitter_fix * p_physics_step
float max_clock_deviation = get_physics_jitter_fix() * p_physics_step;
ret.clamp_process_step(p_process_step - max_clock_deviation, p_process_step + max_clock_deviation);

// last clamping: make sure time_accum is between 0 and p_physics_step for consistency between physics and process
ret.clamp_process_step(process_minus_accum, process_minus_accum + p_physics_step);

// all the operations above may have turned ret.p_process_step negative or zero, keep a minimal value
if (ret.process_step < min_output_step) {
ret.process_step = min_output_step;
}

// restore time_accum
time_accum = ret.process_step - process_minus_accum;

// forcing ret.process_step to be positive may trigger a violation of the
// promise that time_accum is between 0 and p_physics_step
#ifdef DEBUG_ENABLED
if (time_accum < -1E-7) {
WARN_PRINT_ONCE("Intermediate value of `time_accum` is negative. This could hint at an engine bug or system timer misconfiguration.");
}
#endif

if (time_accum > p_physics_step) {
const int extra_physics_steps = floor(time_accum * p_physics_fps);
time_accum -= extra_physics_steps * p_physics_step;
ret.physics_steps += extra_physics_steps;
}

#ifdef DEBUG_ENABLED
if (time_accum < -1E-7) {
WARN_PRINT_ONCE("Final value of `time_accum` is negative. It should always be between 0 and `p_physics_step`. This hints at an engine bug.");
}
if (time_accum > p_physics_step + 1E-7) {
WARN_PRINT_ONCE("Final value of `time_accum` is larger than `p_physics_step`. It should always be between 0 and `p_physics_step`. This hints at an engine bug.");
}
#endif

// track deficit
time_deficit = p_process_step - ret.process_step;

Expand Down

0 comments on commit 7f39c8f

Please sign in to comment.