From d6556ee4b4d9d2c4c9ac48f998bd387f5416b75d Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Sun, 31 May 2015 04:29:15 -0400 Subject: Fix bug with frames being rendered twice. This actually (I believe) fixes three separate issues that are all aspects of the same bug: * Alexandre-Xavier reported that when running at full framerate, the single -devparm dot would flash (#374); * Linguica reported on Doomworld that Chocolate Doom appeared to be rendering each tic twice (see: http://www.doomworld.com/vb/post/1340374 ). * Harha reported performance problems when running in a VM, which may be related (http://www.doomworld.com/vb/post/1374315 ). Chocolate Doom long ago (41cdd5785305a) changed the main loop code so that it does not freeze in network games when tics stop being received; the idea is that it should always be possible to activate the menu to quit. Vanilla Doom allows this too, but only after 20 tics of waiting in TryRunTics() for network data to be received; the menu can be used but is deathly slow. So the loop was changed to wait for only 1 tic instead. However, there was an error in the logic for the check. In a single player game, when time has advanced to the point of being ready to execute another tic, NetUpdate() will build the new ticcmd_t; TryRunTics() was returning immediately; according to the timer, at the exact same time as tic generation, we had also been spinning in the loop for a whole tic and it was time to render a new frame. The end result was that each tic transition would trigger two frames to be rendered: the previous frame, and the new frame. Clearly this is not what is intended. To fix the problem: * Refactor the blocking loop in TryRunTics() so that we only bail out of the function after checking that the loop's exit condition has not just been satisfied (also to eliminate an unnecessary call to I_Sleep() between ticcmd_t generation and execution). * Increase the delay before we bail out to 5 tics rather than just 1. This is still much less than the 20 used in Vanilla Doom but is low enough to keep the menu responsive. A higher bar should ensure that this bug can't reoccur, even in multiplayer where the clocks can be adjusted for sync. This fixes #374. Thanks to everyone involved in reporting the different aspects of it. --- src/d_loop.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/d_loop.c b/src/d_loop.c index e7be7b30..ad9d4a41 100644 --- a/src/d_loop.c +++ b/src/d_loop.c @@ -48,6 +48,12 @@ typedef struct boolean ingame[NET_MAXPLAYERS]; } ticcmd_set_t; +// Maximum time that we wait in TryRunTics() for netgame data to be +// received before we bail out and render a frame anyway. +// Vanilla Doom used 20 for this value, but we use a smaller value +// instead for better responsiveness of the menu when we're stuck. +#define MAX_NETGAME_STALL_TICS 5 + // // gametic is the tic about to (or currently being) run // maketic is the tic that hasn't had control made for it yet @@ -746,7 +752,6 @@ void TryRunTics (void) counts = 1; // wait for new tics if needed - while (!PlayersInGame() || lowtic < gametic/ticdup + counts) { NetUpdate (); @@ -756,15 +761,19 @@ void TryRunTics (void) if (lowtic < gametic/ticdup) I_Error ("TryRunTics: lowtic < gametic"); - // Don't stay in this loop forever. The menu is still running, - // so return to update the screen - - if (I_GetTime() / ticdup - entertic > 0) - { - return; - } + // Still no tics to run? Sleep until some are available. + if (lowtic < gametic/ticdup + counts) + { + // If we're in a netgame, we might spin forever waiting for + // new network data to be received. So don't stay in here + // forever - give the menu a chance to work. + if (I_GetTime() / ticdup - entertic >= MAX_NETGAME_STALL_TICS) + { + return; + } - I_Sleep(1); + I_Sleep(1); + } } // run the count * ticdup dics -- cgit v1.2.3