Skip to content

perf(view): Cache only latest screen-to-terrain result#2992

Open
CryoTheRenegade wants to merge 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/W3DViewm-locationRequests-single-cache-conversion
Open

perf(view): Cache only latest screen-to-terrain result#2992
CryoTheRenegade wants to merge 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/W3DViewm-locationRequests-single-cache-conversion

Conversation

@CryoTheRenegade

Copy link
Copy Markdown

Summary

In #2815 I profiled W3DView::m_locationRequests and found that only the newest entry was reused out of the 40 entry cache. The movement (moving camera in replays) test produced 33,590 newest-entry hits, while older cached entries had no hits.

This change replaces the 40 entry cache with a single entry cache that tracks bridge and seismic additions, deletions, and damage-state changes persistently so we can invalidate the entry when a change occurs.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR changes screen-to-terrain caching to keep only the latest result. The main changes are:

  • Replaced the W3DView request vector with one cached screen/world pair.
  • Added cache invalidation for viewport, camera, terrain, seismic, and bridge changes.
  • Added bridge-change counters to both Generals and Zero Hour terrain logic.
  • Removed the old position-request typedefs from the common STL typedef header.

Confidence Score: 5/5

This looks safe to merge after a small cache-invalidation cleanup.

  • The main cache rewrite is internally consistent.
  • Bridge damage polling can invalidate the new cache even when no bridge changed.
  • No security issues were found in the changed code.

TerrainLogic.cpp in both Generals and GeneralsMD.

Important Files Changed

Filename Overview
Core/GameEngine/Include/Common/STLTypedefs.h Removed the old position-request cache typedefs.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h Replaced the vector cache members with scalar fields for one cached request.
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Implements the single-entry cache and invalidates it around view, terrain, seismic, and bridge changes.
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h Adds the bridge-change counter field and accessor.
Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp Adds bridge-change counter updates, with one over-invalidation issue in damage-state polling.
GeneralsMD/Code/GameEngine/Include/GameLogic/TerrainLogic.h Mirrors the bridge-change counter API for Zero Hour.
GeneralsMD/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp Mirrors the bridge-change counter implementation, including the same damage-polling over-invalidation issue.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp:1866
**Bridge Counter Always Changes**

When `updateBridgeDamageStates()` runs, this increment advances the cache generation even if the bridge scan found no real damage-state change. `W3DView::screenToTerrain()` now treats any generation change as an invalidation, so maps with bridges can throw away the new one-entry cache every frame and lose the intended reuse.

### Issue 2 of 2
GeneralsMD/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp:1866
**Bridge Counter Always Changes**

This mirrored Zero Hour path also advances `m_bridgeChangeCounter` every time bridge damage states are polled, even when no bridge state changed. Since `screenToTerrain()` invalidates its cached result on any counter difference, bridge maps can miss the new cache every frame instead of reusing the latest result.

Reviews (1): Last reviewed commit: "bugfix(w3d): Invalidate terrain cache on..." | Re-trigger Greptile

Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant