fix(heightmap): Prevent per-frame full terrain rebuild with DrawEntireTerrain

[sailro]

Problem

With DrawEntireTerrain=Yes (and likewise StretchTerrain=Yes) the engine rebuilds the entire terrain — all vertex buffers and lighting — on every single frame, which is the massive performance degradation reported in #2743.

Root cause: there are two sources of truth for the terrain draw size.

• WorldHeightMap::createDrawArea() force-overrides the stored draw size to the full map when DrawEntireTerrain=Yes (or to the stretch size when StretchTerrain=Yes).
• W3DView::updateTerrain() calls HeightMapRenderObjClass::setTerrainDrawSize() with the normal draw size every frame (introduced by fix(heightmap): Fix up view and heightmap to work better with low camera pitch #2677).

setTerrainDrawSize() early-returns when the requested size equals the stored size, but the requested normal size never matches the stored full-map size. So every frame it re-runs initHeightData() + a full-map updateBlock(), and sets m_needFullUpdate, causing updateCenter() to do a second full-map update. Roughly two full terrain rebuilds per frame.

This regressed in #2677, which added the per-frame setTerrainDrawSize() call; before that, updateTerrain() only called updateCenter().

Fix

Two parts that work together:

1. Stop the per-frame rebuild. Resolve the effective draw size inside setTerrainDrawSize() the same way createDrawArea() does, by applying the StretchTerrain / DrawEntireTerrain overrides up front (same precedence: DrawEntireTerrain wins). The unchanged-check now matches, so the terrain is no longer reinitialized and rebuilt every frame.

2. Keep event-driven repaints working. That per-frame rebuild was, however, the only thing repainting the terrain when the vertex grid already covers the whole map: HeightMapRenderObjClass::updateCenter() early-returns in that case (small maps, StretchTerrain or DrawEntireTerrain) and so never reaches its m_needFullUpdate handling. Once the per-frame rebuild is removed, pending full updates requested by staticLightingChanged(), ReAcquireResources() after a device reset, time-of-day changes, or late texture loads would never be applied — leaving undrawn (black) and untextured (grey) tiles, most visibly on the shell map. updateCenter() now honors a pending m_needFullUpdate in that early-return path with a single full updateBlock, so those repaints still happen, but only when actually requested instead of every frame.

Steady-state behavior returns to a one-time full update plus the normal full-map draw (i.e. the pre-#2677 cost), while terrain still updates correctly on lighting, device-reset and texture changes. The change is scoped to HeightMapRenderObjClass (terrainLOD == 7); FlatHeightMapRenderObjClass::setTerrainDrawSize/oversizeTerrain are no-ops and are unaffected.

Testing

• Built Release / Win32 and tested in Zero Hour with DrawEntireTerrain=Yes.
• In-game (Whiteout): smooth where the unpatched build was heavily laggy — the per-frame rebuild is gone.
• Shell map / intro menu and in-game terrain: renders correctly (no black holes, no untextured grey tiles), confirming lighting / device-reset / late-texture repaints still occur.
• Verified DrawEntireTerrain=No (default) is unaffected: the overrides only apply when the corresponding setting is on, and the new updateCenter branch only runs when the grid already covers the whole map.

AI disclosure

Per the contribution guidelines: the code in this PR was written with the help of an LLM (GitHub Copilot CLI) and then reviewed, iterated on, and verified by a human. The root cause (two competing sources of truth for the draw size defeating the unchanged-check, regressed by #2677) was traced deliberately, and a first iteration that only fixed setTerrainDrawSize() was caught in testing to regress shell-map rendering — which led directly to the second part of the fix. The change was validated in-game (performance and rendering) with DrawEntireTerrain=Yes. The diff is intentionally small and self-contained: one file, two narrowly-scoped changes.

Fixes #2743

cc @xezon

[greptile-apps]

Greptile Summary

This PR fixes a per-frame full terrain rebuild that regressed in #2677 when DrawEntireTerrain=Yes or StretchTerrain=Yes is active. The change is confined to two tightly-scoped additions in HeightMapRenderObjClass inside HeightMap.cpp.

• setTerrainDrawSize(): Applies the StretchTerrain/DrawEntireTerrain size overrides (with proper clamping, matching createDrawArea()) before the unchanged-check, so the normal-size request from updateTerrain() now matches the stored full-map draw size and the early-return fires correctly — eliminating the per-frame initHeightData + updateBlock double rebuild.
• updateCenter(): The existing early-return path (triggered when the vertex grid already covers the whole map) now processes a pending m_needFullUpdate before returning, ensuring lighting, device-reset, and late-texture repaints are still applied on-demand without reintroducing the every-frame cost.

Confidence Score: 5/5

Safe to merge — the fix is self-contained, the clamping in the StretchTerrain path matches createDrawArea(), and the updateCenter early-return now correctly drains pending full updates without reintroducing the per-frame rebuild.

Both changes are narrow and well-bounded: setTerrainDrawSize() gains deterministic override logic that mirrors the existing createDrawArea() precedence rules, and the updateCenter() addition is guarded by the same m_needFullUpdate flag that the rest of the engine uses for on-demand repaints. The previous reviewer concern about missing clamp on the StretchTerrain path was addressed in this revision. No pre-existing patterns are disturbed.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp	Two narrowly-scoped additions: StretchTerrain/DrawEntireTerrain overrides with correct clamping added to setTerrainDrawSize(), and m_needFullUpdate honored in the updateCenter() full-map early-return path. Logic is consistent with createDrawArea() and the existing update guards.

_{Reviews (5): Last reviewed commit: "fix(heightmap): Apply the extent clamp o..." | Re-trigger Greptile}

[stephanmeesters]

Could you try making the comments more concise? Also don't make them refer to other code because that can get outdated (would need maintenance) and comments also don't typically refer to issues id's.

[sailro]

Could you try making the comments more concise? Also don't make them refer to other code because that can get outdated (would need maintenance) and comments also don't typically refer to issues id's.

Thanks for the review! @stephanmeesters, I've pushed an update:

• More concise — both comments are roughly half the length now.
• No references to other code — dropped the mentions of other functions (createDrawArea(), updateTerrain(), etc.) that could go stale.
• No issue IDs in the comments.

I did keep the StretchTerrain / DrawEntireTerrain names. These aren't internal code references but user-facing settings exposed in the INI, so they're stable, documented names rather than something likely to be renamed. They're also the whole reason this code path exists and appear right on the following lines, so naming them makes the intent clearer than a purely abstract description. That said, I'm happy to drop them too if you'd prefer the comments fully free of identifiers — just let me know.

[sailro]

Hello @xezon @stephanmeesters

I'd be happy to discuss or modify anything in this PR that you think is worth changing.

I'm just trying to fix a display issue that I think is crucial given today's high resolutions.

Thanks!

[xezon]

This Pull will be superseded.

[sailro]

This Pull will be superseded.

Thanks @xezon. Understood on the supersession. Just in case it's useful in the meantime, I've addressed all of @Skyaero42's review comments in a separate follow-up commit (the extent clamp is now applied only in the StretchTerrain branch, and I added the blank line between the two overrides).

Whatever works best for you -folding this into your terrain rework or keeping it as a reference until then- I'm happy to help however is most convenient. Just let me know.

[xezon]

I have created new change #2846 to address the issue.

The fix presented in this pull is small scope, but not ideal because it applies a hack on already confusing code. First, the streched terrain does nothing and can be removed. Second, the ownership of terrain sizing is not well delegated and needs to be simplified. While setting it in HeightMapRenderObjClass::setTerrainDrawSize can work, we can do better.

[sailro]

I have created new change #2846 to address the issue.

The fix presented in this pull is small scope, but not ideal because it applies a hack on already confusing code. First, the streched terrain does nothing and can be removed. Second, the ownership of terrain sizing is not well delegated and needs to be simplified. While setting it in HeightMapRenderObjClass::setTerrainDrawSize can work, we can do better.

That is perfect thanks, I made a humble review, it is clearly cleaner than mine.

Thank you !