Skip to content

Commit

Permalink
Fix UBSan error at early startup about converting infinity to int.
Browse files Browse the repository at this point in the history
This happens because during early startup, the FPS value returned by the
stats is zero, and 1 / 0 = inf, and static_cast<int>(inf) is undefined
behavior.

Also, I removed one case as it's dead code (measurements_cnt <= 0 cannot
happen as the function itself increments it), and riddled the timing code
with asserts and comments explaining why each assert is always true.
  • Loading branch information
divVerent authored and tobbi committed Jun 17, 2020
1 parent bfd11ac commit 7249298
Showing 1 changed file with 29 additions and 24 deletions.
53 changes: 29 additions & 24 deletions src/supertux/screen_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ struct ScreenManager::FPS_Stats
auto time_now = std::chrono::steady_clock::now();
int dtime_us = static_cast<int>(std::chrono::duration_cast<
std::chrono::microseconds>(time_now - time_prev).count());
assert(dtime_us >= 0); // Steady clock.
if (dtime_us == 0)
return;
time_prev = time_now;
Expand All @@ -148,15 +149,15 @@ struct ScreenManager::FPS_Stats
if (expired_seconds < 0.5f)
return;
// Update values to be printed every 0.5 s
if (measurements_cnt > 0) {
last_fps = static_cast<float>(measurements_cnt) / expired_seconds;
last_fps_min = 1000000.0f / static_cast<float>(max_us);
last_fps_max = 1000000.0f / static_cast<float>(min_us);
} else {
last_fps = 0;
last_fps_min = 0;
last_fps_max = 0;
}
assert(measurements_cnt > 0); // ++measurements_cnt above.
last_fps = static_cast<float>(measurements_cnt) / expired_seconds;
assert(last_fps > 0); // measurements_cnt > 0 and expired_seconds >= 0.5f.
assert(max_us > 0); // dtime_us > 0.
last_fps_min = 1000000.0f / static_cast<float>(max_us);
assert(last_fps_min > 0); // max_us > 0.
assert(min_us > 0); // initialization to 1000000 and dtime_us > 0.
last_fps_max = 1000000.0f / static_cast<float>(min_us);
assert(last_fps_max > 0); // min_us > 0.
measurements_cnt = 0;
acc_us = 0;
min_us = 1000000;
Expand Down Expand Up @@ -477,21 +478,25 @@ ScreenManager::run()
// Do not calculate more than a few steps at once
// The maximum number of steps executed before drawing a frame is
// adjusted to the current average frame rate
float seconds_per_frame = 1.0f / fps_statistics.get_fps();
int max_steps_per_frame = static_cast<int>(
ceilf(seconds_per_frame / seconds_per_step));
if (max_steps_per_frame < 2)
// max_steps_per_frame is very negative when the fps value is zero
// Furthermore, the game should always be able to execute
// up to two steps before drawing a frame
max_steps_per_frame = 2;
if (max_steps_per_frame > 4)
// When the game is very laggy, it should slow down instead of calculating
// lots of steps at once so that the player can still control Tux
// reasonably;
// four steps per frame approximately corresponds to a 16 FPS gameplay
max_steps_per_frame = 4;
steps = std::min<int>(steps, max_steps_per_frame);
float fps = fps_statistics.get_fps();
if (fps != 0) {
// Skip if fps not ready yet (during first 0.5 seconds of startup).
float seconds_per_frame = 1.0f / fps_statistics.get_fps();
int max_steps_per_frame = static_cast<int>(
ceilf(seconds_per_frame / seconds_per_step));
if (max_steps_per_frame < 2)
// max_steps_per_frame is very negative when the fps value is zero
// Furthermore, the game should always be able to execute
// up to two steps before drawing a frame
max_steps_per_frame = 2;
if (max_steps_per_frame > 4)
// When the game is very laggy, it should slow down instead of
// calculating lots of steps at once so that the player can still
// control Tux reasonably;
// four steps per frame approximately corresponds to a 16 FPS gameplay
max_steps_per_frame = 4;
steps = std::min<int>(steps, max_steps_per_frame);
}

for (int i = 0; i < steps; ++i) {
// Perform a logical game step; seconds_per_step is set to a fixed value
Expand Down

0 comments on commit 7249298

Please sign in to comment.