From 9eab5317f3787bd61d26fd646541b9582bc1947f Mon Sep 17 00:00:00 2001 From: James R Date: Tue, 4 Apr 2023 04:29:47 -0700 Subject: [PATCH 1/2] Stop calling P_NullPrecipThinker In levels with tens of thousands of precipmobjs, the overhead of running a thinker for all of them is too much, no matter how small the thinker is. ALL thinking is done inside the renderer now, where it can be limited by distance. Precipmobjs track the last leveltime they thunk, so interpolated frames don't think twice. --- src/hardware/hw_main.c | 13 +++++------ src/m_perfstats.c | 3 +-- src/p_local.h | 9 +++++++- src/p_mobj.c | 52 ++++++++++++++++++++++++++---------------- src/p_mobj.h | 6 +++-- src/p_spec.c | 6 +++-- src/p_tick.c | 27 ++++++++++++++++------ src/r_things.c | 18 +++++++-------- 8 files changed, 83 insertions(+), 51 deletions(-) diff --git a/src/hardware/hw_main.c b/src/hardware/hw_main.c index ca53980cf..32f2afb43 100644 --- a/src/hardware/hw_main.c +++ b/src/hardware/hw_main.c @@ -5653,6 +5653,12 @@ static void HWR_ProjectPrecipitationSprite(precipmobj_t *thing) // uncapped/interpolation interpmobjstate_t interp = {0}; + // okay... this is a hack, but weather isn't networked, so it should be ok + if (!P_PrecipThinker(thing)) + { + return; + } + // do interpolation if (R_UsingFrameInterpolation() && !paused) { @@ -5752,13 +5758,6 @@ static void HWR_ProjectPrecipitationSprite(precipmobj_t *thing) vis->precip = true; vis->bbox = false; - - // okay... this is a hack, but weather isn't networked, so it should be ok - if (!(thing->precipflags & PCF_THUNK)) - { - P_PrecipThinker(thing); - thing->precipflags |= PCF_THUNK; - } } #endif diff --git a/src/m_perfstats.c b/src/m_perfstats.c index 53014a5ef..f3e92fe30 100644 --- a/src/m_perfstats.c +++ b/src/m_perfstats.c @@ -51,7 +51,7 @@ precise_t ps_playerthink_time = 0; precise_t ps_botticcmd_time = 0; precise_t ps_thinkertime = 0; -precise_t ps_thlist_times[NUM_THINKERLISTS]; +precise_t ps_thlist_times[NUM_ACTIVETHINKERLISTS]; precise_t ps_acs_time = 0; int ps_checkposition_calls = 0; @@ -367,7 +367,6 @@ static void M_DrawTickStats(void) {"main ", "Main: ", &ps_thlist_times[THINK_MAIN]}, {"mobjs ", "Mobjs: ", &ps_thlist_times[THINK_MOBJ]}, {"dynslop", "Dynamic slopes: ", &ps_thlist_times[THINK_DYNSLOPE]}, - {"precip ", "Precipitation: ", &ps_thlist_times[THINK_PRECIP]}, {0} }; diff --git a/src/p_local.h b/src/p_local.h index 51f946572..aba75cbb8 100644 --- a/src/p_local.h +++ b/src/p_local.h @@ -72,7 +72,13 @@ typedef enum THINK_MAIN, THINK_MOBJ, THINK_DYNSLOPE, - THINK_PRECIP, + + // Lists after this may exist but they do not call an + // action in P_RunThinkers + NUM_ACTIVETHINKERLISTS, + + THINK_PRECIP = NUM_ACTIVETHINKERLISTS, + NUM_THINKERLISTS } thinklistnum_t; /**< Thinker lists. */ extern thinker_t thlist[]; @@ -80,6 +86,7 @@ extern thinker_t thlist[]; void P_InitThinkers(void); void P_AddThinker(const thinklistnum_t n, thinker_t *thinker); void P_RemoveThinker(thinker_t *thinker); +void P_UnlinkThinker(thinker_t *thinker); // // P_USER diff --git a/src/p_mobj.c b/src/p_mobj.c index 4322e8478..7d35de413 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -546,7 +546,7 @@ static boolean P_SetPrecipMobjState(precipmobj_t *mobj, statenum_t state) if (state == S_NULL) { // Remove mobj - P_RemovePrecipMobj(mobj); + P_FreePrecipMobj(mobj); return false; } st = &states[state]; @@ -4174,25 +4174,33 @@ void P_RecalcPrecipInSector(sector_t *sector) // // P_NullPrecipThinker // -// For "Blank" precipitation +// Just the identification of a precip thinker. The thinker +// should never actually be called! // void P_NullPrecipThinker(precipmobj_t *mobj) { - //(void)mobj; - mobj->precipflags &= ~PCF_THUNK; - R_ResetPrecipitationMobjInterpolationState(mobj); + (void)mobj; + I_Assert("P_NullPrecipThinker should not be called" == 0); } -void P_PrecipThinker(precipmobj_t *mobj) +boolean P_PrecipThinker(precipmobj_t *mobj) { boolean flip = (mobj->precipflags & PCF_FLIP); + if (mobj->lastThink == leveltime) + return true; // already thinked this tick + + mobj->lastThink = leveltime; + + R_ResetPrecipitationMobjInterpolationState(mobj); P_CycleStateAnimation((mobj_t *)mobj); if (mobj->state == &states[S_RAINRETURN]) { // Reset to ceiling! - P_SetPrecipMobjState(mobj, mobj->info->spawnstate); + if (!P_SetPrecipMobjState(mobj, mobj->info->spawnstate)) + return false; + mobj->z = (flip) ? (mobj->floorz) : (mobj->ceilingz); mobj->momz = FixedMul(-mobj->info->speed, mapobjectscale); mobj->precipflags &= ~PCF_SPLASH; @@ -4213,20 +4221,23 @@ void P_PrecipThinker(precipmobj_t *mobj) // HACK: sprite changes are 1 tic late, so you would see splashes on the ceiling if not for this state. // We need to use the settings from the previous state, since some of those are NOT 1 tic late. INT32 frame = (mobj->frame & ~FF_FRAMEMASK); - P_SetPrecipMobjState(mobj, S_RAINRETURN); + + if (!P_SetPrecipMobjState(mobj, S_RAINRETURN)) + return false; + mobj->frame = frame; - return; + return true; } else { if (!P_SetPrecipMobjState(mobj, mobj->state->nextstate)) - return; + return false; } } } if (mobj->precipflags & PCF_SPLASH) - return; + return true; mobj->z += mobj->momz; @@ -4240,12 +4251,16 @@ void P_PrecipThinker(precipmobj_t *mobj) } else { - P_SetPrecipMobjState(mobj, mobj->info->deathstate); + if (!P_SetPrecipMobjState(mobj, mobj->info->deathstate)) + return false; + mobj->z = (flip) ? (mobj->ceilingz) : (mobj->floorz); mobj->precipflags |= PCF_SPLASH; R_ResetPrecipitationMobjInterpolationState(mobj); } } + + return true; } static void P_RingThinker(mobj_t *mobj) @@ -11253,7 +11268,7 @@ boolean P_MobjWasRemoved(mobj_t *mobj) return true; } -void P_RemovePrecipMobj(precipmobj_t *mobj) +void P_FreePrecipMobj(precipmobj_t *mobj) { // unlink from sector and block lists P_UnsetPrecipThingPosition(mobj); @@ -11265,7 +11280,9 @@ void P_RemovePrecipMobj(precipmobj_t *mobj) } // free block - P_RemoveThinker((thinker_t *)mobj); + // Precipmobjs don't actually think using their thinker, + // so the free cannot be delayed. + P_UnlinkThinker((thinker_t*)mobj); } // Clearing out stuff for savegames @@ -11301,12 +11318,7 @@ void P_RemoveSavegameMobj(mobj_t *mobj) // free block // Here we use the same code as R_RemoveThinkerDelayed, but without reference counting (we're removing everything so it shouldn't matter) and without touching currentthinker since we aren't in P_RunThinkers - { - thinker_t *thinker = (thinker_t *)mobj; - thinker_t *next = thinker->next; - (next->prev = thinker->prev)->next = next; - Z_Free(thinker); - } + P_UnlinkThinker((thinker_t*)mobj); } static CV_PossibleValue_t respawnitemtime_cons_t[] = {{1, "MIN"}, {300, "MAX"}, {0, NULL}}; diff --git a/src/p_mobj.h b/src/p_mobj.h index c084b245c..b6eddadea 100644 --- a/src/p_mobj.h +++ b/src/p_mobj.h @@ -493,6 +493,8 @@ struct precipmobj_t INT32 tics; // state tic counter state_t *state; UINT32 flags; // flags from mobjinfo tables + + tic_t lastThink; }; struct actioncache_t @@ -540,9 +542,9 @@ void P_RemoveFloorSpriteSlope(mobj_t *mobj); boolean P_BossTargetPlayer(mobj_t *actor, boolean closest); boolean P_SupermanLook4Players(mobj_t *actor); void P_DestroyRobots(void); -void P_PrecipThinker(precipmobj_t *mobj); +boolean P_PrecipThinker(precipmobj_t *mobj); void P_NullPrecipThinker(precipmobj_t *mobj); -void P_RemovePrecipMobj(precipmobj_t *mobj); +void P_FreePrecipMobj(precipmobj_t *mobj); void P_SetScale(mobj_t *mobj, fixed_t newscale); void P_XYMovement(mobj_t *mo); void P_RingXYMovement(mobj_t *mo); diff --git a/src/p_spec.c b/src/p_spec.c index 32ca12dd7..2d1acc726 100644 --- a/src/p_spec.c +++ b/src/p_spec.c @@ -1833,16 +1833,18 @@ void P_SwitchWeather(preciptype_t newWeather) if (purge == true) { thinker_t *think; + thinker_t *next; precipmobj_t *precipmobj; - for (think = thlist[THINK_PRECIP].next; think != &thlist[THINK_PRECIP]; think = think->next) + for (think = thlist[THINK_PRECIP].next; think != &thlist[THINK_PRECIP]; think = next) { if (think->function.acp1 != (actionf_p1)P_NullPrecipThinker) continue; // not a precipmobj thinker precipmobj = (precipmobj_t *)think; - P_RemovePrecipMobj(precipmobj); + next = think->next; + P_FreePrecipMobj(precipmobj); } } else if (swap != MT_NULL) // Rather than respawn all that crap, reuse it! diff --git a/src/p_tick.c b/src/p_tick.c index be1efb337..fd24f37e4 100644 --- a/src/p_tick.c +++ b/src/p_tick.c @@ -295,8 +295,6 @@ static thinker_t *currentthinker; // void P_RemoveThinkerDelayed(thinker_t *thinker) { - thinker_t *next; - if (thinker->references != 0) { #ifdef PARANOIA @@ -327,15 +325,30 @@ void P_RemoveThinkerDelayed(thinker_t *thinker) return; } - /* Remove from main thinker list */ - next = thinker->next; + R_DestroyLevelInterpolators(thinker); + /* Note that currentthinker is guaranteed to point to us, * and since we're freeing our memory, we had better change that. So * point it to thinker->prev, so the iterator will correctly move on to * thinker->prev->next = thinker->next */ - (next->prev = currentthinker = thinker->prev)->next = next; + currentthinker = thinker->prev; - R_DestroyLevelInterpolators(thinker); + /* Remove from main thinker list */ + P_UnlinkThinker(thinker); +} + +// +// P_UnlinkThinker() +// +// Actually removes thinker from the list and frees its memory. +// +void P_UnlinkThinker(thinker_t *thinker) +{ + thinker_t *next = thinker->next; + + I_Assert(thinker->references == 0); + + (next->prev = thinker->prev)->next = next; Z_Free(thinker); } @@ -437,7 +450,7 @@ static void P_RunThinkers(void) { size_t i; - for (i = 0; i < NUM_THINKERLISTS; i++) + for (i = 0; i < NUM_ACTIVETHINKERLISTS; i++) { ps_thlist_times[i] = I_GetPreciseTime(); for (currentthinker = thlist[i].next; currentthinker != &thlist[i]; currentthinker = currentthinker->next) diff --git a/src/r_things.c b/src/r_things.c index f9b488684..337b9c8cf 100644 --- a/src/r_things.c +++ b/src/r_things.c @@ -2465,6 +2465,12 @@ static void R_ProjectPrecipitationSprite(precipmobj_t *thing) // uncapped/interpolation interpmobjstate_t interp = {0}; + // okay... this is a hack, but weather isn't networked, so it should be ok + if (!P_PrecipThinker(thing)) + { + return; + } + // do interpolation if (R_UsingFrameInterpolation() && !paused) { @@ -2554,7 +2560,7 @@ static void R_ProjectPrecipitationSprite(precipmobj_t *thing) if (thing->subsector->sector->cullheight) { if (R_DoCulling(thing->subsector->sector->cullheight, viewsector->cullheight, viewz, gz, gzt)) - goto weatherthink; + return; } // Determine the blendmode and translucency value @@ -2565,7 +2571,7 @@ static void R_ProjectPrecipitationSprite(precipmobj_t *thing) trans = (thing->frame & FF_TRANSMASK) >> FF_TRANSSHIFT; if (trans >= NUMTRANSMAPS) - goto weatherthink; // cap + return; // cap } // store information in a vissprite @@ -2623,14 +2629,6 @@ static void R_ProjectPrecipitationSprite(precipmobj_t *thing) // Fullbright vis->colormap = colormaps; - -weatherthink: - // okay... this is a hack, but weather isn't networked, so it should be ok - if (!(thing->precipflags & PCF_THUNK)) - { - P_PrecipThinker(thing); - thing->precipflags |= PCF_THUNK; - } } // R_AddSprites From c76fc45de95ae230392ab1bb9592756a4951fd04 Mon Sep 17 00:00:00 2001 From: toaster Date: Sat, 8 Apr 2023 21:55:28 +0100 Subject: [PATCH 2/2] P_SwitchWeather: Fix potential infinite loop if non-NullPrecipThinker thinker existed in THINK_PRECIP --- src/p_spec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/p_spec.c b/src/p_spec.c index 2d1acc726..2947638cc 100644 --- a/src/p_spec.c +++ b/src/p_spec.c @@ -1838,12 +1838,12 @@ void P_SwitchWeather(preciptype_t newWeather) for (think = thlist[THINK_PRECIP].next; think != &thlist[THINK_PRECIP]; think = next) { + next = think->next; + if (think->function.acp1 != (actionf_p1)P_NullPrecipThinker) continue; // not a precipmobj thinker precipmobj = (precipmobj_t *)think; - - next = think->next; P_FreePrecipMobj(precipmobj); } }