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 severe per-frame full terrain rebuild that occurred when DrawEntireTerrain=Yes or StretchTerrain=Yes, regressed by #2677. The root cause was a mismatch between the normal draw size passed by updateTerrain() each frame and the full-map draw size already stored by createDrawArea(), defeating the unchanged-check early-return in setTerrainDrawSize().

• setTerrainDrawSize (HeightMap.cpp:1210–1225): Applies the same StretchTerrain/DrawEntireTerrain overrides that createDrawArea() uses (including a post-override clamp to getXExtent()/getYExtent()), so the equality check now matches the stored size and the per-frame initHeightData + full updateBlock no longer fires.
• updateCenter early-return path (HeightMap.cpp:1692–1702): When the vertex grid already covers the whole map (the code path that previously had no m_needFullUpdate handling), a pending full update is now flushed with a single updateBlock, ensuring lighting/device-reset/texture repaints still occur without reintroducing the per-frame rebuild.

Confidence Score: 5/5

Safe to merge — the two changes are self-contained within HeightMapRenderObjClass, mirror the override logic already present in createDrawArea(), and the updateCenter m_needFullUpdate flush correctly reuses the existing m_updating guard.

Both change sites are narrow and well-matched to the root cause. setTerrainDrawSize now applies the same StretchTerrain/DrawEntireTerrain overrides and post-override clamp that createDrawArea uses, so the equality check resolves correctly for all cases including small maps with StretchTerrain. The updateCenter early-return path now handles pending full updates without reintroducing per-frame rebuilds, and the m_updating recursion guard is set and cleared correctly around the new updateBlock call.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp	Two narrowly-scoped changes: adds StretchTerrain/DrawEntireTerrain override logic plus a post-override clamp in setTerrainDrawSize, and adds m_needFullUpdate handling in the updateCenter early-return path. Logic matches the existing createDrawArea behaviour and the m_updating guard is correctly managed in both change sites.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["W3DView::updateTerrain() — every frame"] -->|"calls with normal draw size"| B["setTerrainDrawSize(width, height)"]
    B --> C{"m_oversizeDrawWidth != 0?"}
    C -->|Yes| D["width = m_oversizeDrawWidth"]
    C -->|No| E["width = min(width, XExtent)"]
    D --> F{"StretchTerrain?"}
    E --> F
    F -->|Yes| G["width = STRETCH_DRAW_WIDTH\nheight = STRETCH_DRAW_HEIGHT"]
    F -->|No| H{"DrawEntireTerrain?"}
    G --> H
    H -->|Yes| I["width = getXExtent()\nheight = getYExtent()"]
    H -->|No| J["clamp to XExtent/YExtent"]
    I --> J
    J --> K{"width == drawWidth\n&& height == drawHeight?"}
    K -->|"Yes (no change)"| L["return — no rebuild"]
    K -->|"No (size changed)"| M["initHeightData + m_needFullUpdate = true"]

    N["updateCenter() — every frame"] --> O{"m_x >= XExtent\n&& m_y >= YExtent?"}
    O -->|"Yes (full-map grid)"| P{"m_needFullUpdate?"}
    P -->|Yes| Q["updateBlock full map\nm_needFullUpdate = false"]
    P -->|No| R["return — nothing to do"]
    Q --> R
    O -->|No| S["Normal scroll / partial update path"]
    S --> T{"m_needFullUpdate?"}
    T -->|Yes| U["setDrawArea + updateBlock full"]
    T -->|No| V["Incremental scroll updates"]

Loading

_{Reviews (3): Last reviewed commit: "fix(heightmap): Prevent per-frame full t..." | 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.