Skip to content

Commit

Permalink
Change how com_frameTime is updated
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
DanielGibson committed Jun 29, 2024
1 parent 295ef9b commit 74f40bd
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 29 deletions.
72 changes: 46 additions & 26 deletions neo/framework/Common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -279,7 +296,8 @@ idCommonLocal::idCommonLocal( void ) {
config_compressor = NULL;
#endif

async_timer = 0;
memset( &asyncThread, 0, sizeof(asyncThread) );
runAsyncThread = false;
}

/*
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -2616,6 +2635,7 @@ void idCommonLocal::Async( void ) {
SingleAsyncTic();
lastTicMsec += ticMsec;
}
nextTicTargetMsec = lastTicMsec + ticMsec;
}

/*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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" );
}


Expand All @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion neo/framework/Session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ void idSessionLocal::CompleteWipe() {
}
}

extern void Com_UpdateFrameTime();

/*
================
idSessionLocal::ShowLoadingGui
Expand All @@ -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 );
}
Expand Down
4 changes: 2 additions & 2 deletions neo/idlib/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit 74f40bd

Please sign in to comment.