From 74f40bd0adc76808409ff12f52230547d9471e08 Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Sat, 29 Jun 2024 05:25:30 +0200 Subject: [PATCH] Change how com_frameTime is updated It used to be set with just com_frameTime = com_ticNumber * USERCMD_MSEC; where com_ticNumber is incremented in each idCommonLocal::SingleAsyncTic(). That worked well enough when USERCMD_MSEC was a fixed constant, but now changing com_gameHz could cause com_frameTime to decrease, which leads to all kinds of funny glitches like fading into the main menu when switching mods taking way too long, resulting in a black screen (when the mod has higher com_gameHz => lower USERCMD_MSEC). So now this is done in a function with slightly more logic that adds to com_frameTime instead of recalculating it with a multiplication. I also changed how SingleAsyncTic() (or idCommonLocal::Async()) is called, by using a proper thread instead of an SDL_Timer. This gives me more control over it so it can be more precise (and, as far as I can tell, it is) --- neo/framework/Common.cpp | 72 +++++++++++++++++++++++++-------------- neo/framework/Session.cpp | 4 ++- neo/idlib/Parser.cpp | 4 +-- 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/neo/framework/Common.cpp b/neo/framework/Common.cpp index 6d8b6b02d..cc1a84daa 100644 --- a/neo/framework/Common.cpp +++ b/neo/framework/Common.cpp @@ -122,7 +122,7 @@ int time_gameDraw; int time_frontend; // renderSystem frontend time int time_backend; // renderSystem backend time -int com_frameTime; // time for the current frame in milliseconds - TODO: DG: make it double? +int com_frameTime; // time (since start) for the current frame in milliseconds - TODO: DG: make it double? int com_frameNumber; // variable frame number volatile int com_ticNumber; // 60 hz tics int com_editors; // currently opened editor(s) @@ -250,12 +250,29 @@ class idCommonLocal : public idCommon { idCompressor * config_compressor; #endif - SDL_TimerID async_timer; + static int AsyncThread(void* arg); + xthreadInfo asyncThread; + volatile bool runAsyncThread; }; idCommonLocal commonLocal; idCommon * common = &commonLocal; +// DG: updates com_frameTime based on the current tic number and USERCMD_MSEC (com_gameFrameTime == 1000/com_gameHz) +void Com_UpdateFrameTime() { + // It used to be just com_frameTime = com_ticNumber * USERCMD_MSEC; + // But now that USERCMD_MSEC isn't fixed to 16 for fixed 60fps anymore (thanks to com_gameHz), + // that doesn't work anymore (com_frameTime would decrease when setting com_gameHz to a lower value!) + // So I moved updating it into a function (it's done in 3 places) that has just slightly more logic + // to ensure com_frameTime never decreases (well, until it overflows :-p) + static int lastTicNum = 0; + int ticNum = com_ticNumber; + int ticDiff = ticNum - lastTicNum; + assert(ticDiff >= 0); + com_frameTime += ticDiff * USERCMD_MSEC; + lastTicNum = ticNum; +} + /* ================== idCommonLocal::idCommonLocal @@ -279,7 +296,8 @@ idCommonLocal::idCommonLocal( void ) { config_compressor = NULL; #endif - async_timer = 0; + memset( &asyncThread, 0, sizeof(asyncThread) ); + runAsyncThread = false; } /* @@ -2456,7 +2474,7 @@ void idCommonLocal::Frame( void ) { // DG: prepare new ImGui frame - I guess this is a good place, as all new events should be available? D3::ImGuiHooks::NewFrame(); - com_frameTime = com_ticNumber * USERCMD_MSEC; // TODO: can we continue using 60hz tics and still run at higher fps? com_frameNumber vs com_ticNumber suggests that.. + Com_UpdateFrameTime(); // DG: put updating com_frameTime into a function idAsyncNetwork::RunFrame(); @@ -2502,7 +2520,7 @@ idCommonLocal::GUIFrame void idCommonLocal::GUIFrame( bool execCmd, bool network ) { Sys_GenerateEvents(); eventLoop->RunEventLoop( execCmd ); // and execute any commands - com_frameTime = com_ticNumber * USERCMD_MSEC; + Com_UpdateFrameTime(); // DG: put updating com_frameTime into a function if ( network ) { idAsyncNetwork::RunFrame(); } @@ -2539,8 +2557,8 @@ typedef struct { static const int MAX_ASYNC_STATS = 1024; asyncStats_t com_asyncStats[MAX_ASYNC_STATS]; // indexed by com_ticNumber -int prevAsyncMsec; -int lastTicMsec; +static int lastTicMsec; +static int nextTicTargetMsec; // when (according to Sys_Milliseconds()) the next async tic should start void idCommonLocal::SingleAsyncTic( void ) { // main thread code can prevent this from happening while modifying @@ -2591,6 +2609,7 @@ void idCommonLocal::Async( void ) { if ( !com_preciseTic.GetBool() ) { // just run a single tic, even if the exact msec isn't precise SingleAsyncTic(); + nextTicTargetMsec = msec + USERCMD_MSEC; return; } @@ -2616,6 +2635,7 @@ void idCommonLocal::Async( void ) { SingleAsyncTic(); lastTicMsec += ticMsec; } + nextTicTargetMsec = lastTicMsec + ticMsec; } /* @@ -2805,21 +2825,22 @@ void idCommonLocal::SetMachineSpec( void ) { } } -static unsigned int AsyncTimer(unsigned int interval, void *) { - common->Async(); - Sys_TriggerEvent(TRIGGER_EVENT_ONE); +int idCommonLocal::AsyncThread(void* arg) +{ + idCommonLocal* self = (idCommonLocal*)arg; + + while ( self->runAsyncThread ) { - // calculate the next interval to get as close to 60fps as possible - unsigned int now = SDL_GetTicks(); - unsigned int tick = com_ticNumber * USERCMD_MSEC; - // FIXME: this is pretty broken and basically always returns 1 because now now is much bigger than tic - // (probably com_tickNumber only starts incrementing a second after engine starts?) - // only reason this works is common->Async() checking again before calling SingleAsyncTic() + self->Async(); - if (now >= tick) - return 1; + Sys_TriggerEvent(TRIGGER_EVENT_ONE); - return tick - now; + // TODO: -1 is so we don't sleep too long - would -2 be better, or can we have a more precise sleep? + // IIRC especially on Windows sleeping is imprecise by at least on MS + int sleepTime = Max( 0, nextTicTargetMsec - (int)Sys_Milliseconds() - 1 ); + Sys_Sleep( sleepTime ); + } + return 0; } #ifdef _WIN32 @@ -3084,10 +3105,8 @@ void idCommonLocal::Init( int argc, char **argv ) { Sys_Error( "Error during initialization" ); } - async_timer = SDL_AddTimer(USERCMD_MSEC, AsyncTimer, NULL); // TODO: maybe rework the whole async timer thing, might be too imprecise - - if (!async_timer) - Sys_Error("Error while starting the async timer: %s", SDL_GetError()); + runAsyncThread = true; + Sys_CreateThread( AsyncThread, this, asyncThread, "AsyncThread" ); } @@ -3097,9 +3116,10 @@ idCommonLocal::Shutdown ================= */ void idCommonLocal::Shutdown( void ) { - if (async_timer) { - SDL_RemoveTimer(async_timer); - async_timer = 0; + if ( asyncThread.threadHandle != NULL ) { + runAsyncThread = false; + Sys_DestroyThread( asyncThread ); + memset( &asyncThread, 0, sizeof(asyncThread) ); } idAsyncNetwork::server.Kill(); diff --git a/neo/framework/Session.cpp b/neo/framework/Session.cpp index 837f7f522..991f04ab5 100644 --- a/neo/framework/Session.cpp +++ b/neo/framework/Session.cpp @@ -524,6 +524,8 @@ void idSessionLocal::CompleteWipe() { } } +extern void Com_UpdateFrameTime(); + /* ================ idSessionLocal::ShowLoadingGui @@ -541,7 +543,7 @@ void idSessionLocal::ShowLoadingGui() { int stop = Sys_Milliseconds() + 1000; int force = 10; while ( Sys_Milliseconds() < stop || force-- > 0 ) { - com_frameTime = com_ticNumber * USERCMD_MSEC; + Com_UpdateFrameTime(); // DG: put updating com_frameTime into a function session->Frame(); session->UpdateScreen( false ); } diff --git a/neo/idlib/Parser.cpp b/neo/idlib/Parser.cpp index bd4fe7b76..a8455eb42 100644 --- a/neo/idlib/Parser.cpp +++ b/neo/idlib/Parser.cpp @@ -1153,7 +1153,7 @@ int idParser::Directive_define( void ) { int line = token.line; // change "#define GAME_FPS 60" to "#define GAME_FPS sys.getTicsPerSecond()" - // (or equivalent for GAME_FRAMETIME and sys.getFrameTime()) + // (or equivalent for GAME_FRAMETIME and sys.getRawFrameTime()) hackTokens[0] = createToken( "sys", TT_NAME, 3, line ); hackTokens[1] = createToken( ".", TT_PUNCTUATION, P_REF, line ); hackTokens[2] = createToken( funName, TT_NAME, strlen(funName), line ); // getTicsPerSecond or getFrameTime @@ -1206,7 +1206,7 @@ int idParser::Directive_define( void ) { if ( numHackTokens != 0 ) { // skip rest of the line, inject our hackTokens instead and return while ( idParser::ReadLine( &token ) ) - {}; + {} define->tokens = hackTokens[0]; last = hackTokens[0];