From f8e62424c58ecca2f013d1bb3e601f2527efdd84 Mon Sep 17 00:00:00 2001 From: Manuel Moos Date: Sat, 15 Feb 2020 23:50:25 +0100 Subject: [PATCH] Fix negative delta arguments 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 Cherry-Pick from 6be702bace06604281b3358ce7015086bcb7e2fb. --- main/main_timer_sync.cpp | 43 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/main/main_timer_sync.cpp b/main/main_timer_sync.cpp index da708d97cc8..508c3dc26d4 100644 --- a/main/main_timer_sync.cpp +++ b/main/main_timer_sync.cpp @@ -304,7 +304,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; } @@ -352,6 +352,12 @@ MainFrameTime MainTimerSync::advance_core(float p_frame_slice, int p_iterations_ } } +#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_iterations_per_second + get_physics_jitter_fix()); @@ -371,6 +377,10 @@ MainFrameTime MainTimerSync::advance_core(float p_frame_slice, int p_iterations_ } } + if (ret.physics_steps < 0) { + ret.physics_steps = 0; + } + time_accum -= ret.physics_steps * p_frame_slice; // keep track of accumulated step counts @@ -398,6 +408,9 @@ MainFrameTime MainTimerSync::advance_checked(float p_frame_slice, int p_iteratio p_idle_step = 1.0 / fixed_fps; } + float min_output_step = p_idle_step / 8; + min_output_step = MAX(min_output_step, 1E-6); + // compensate for last deficit p_idle_step += time_deficit; @@ -424,9 +437,37 @@ 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 or zero, keep a minimal value + if (ret.idle_step < min_output_step) { + ret.idle_step = min_output_step; + } + // 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("Intermediate value of `time_accum` is negative. This could hint at an engine bug or system timer misconfiguration."); + } +#endif + + if (time_accum > p_frame_slice) { + 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("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_frame_slice + 1E-7) { + WARN_PRINT_ONCE("Final value of `time_accum` is larger than `p_frame_slice`. It should always be between 0 and `p_frame_slice`. This hints at an engine bug."); + } +#endif + // track deficit time_deficit = p_idle_step - ret.idle_step;