perf(terrain): Hoist global light ray computation out of per-vertex loop

[Cellcote]

In HeightMapRenderObjClass::updateVB, the light ray directions were recomputed from TheGlobalData->m_terrainLightPos for every terrain quad in the nested i,j loops. These values are constant for the entire update region, so move the computation before the loops.

Eliminates (width * height - 1) redundant recomputations per update tile.

[greptile-apps]

Greptile Summary

This PR optimizes terrain vertex-buffer updates in HeightMapRenderObjClass::updateVB by hoisting the global-light ray computation out of the inner quad loop. Previously, TheGlobalData->m_terrainLightPos was read and negated for every cell in the i,j loop; the values are constant for the entire update tile, so moving them before the loop is semantically equivalent and eliminates (width × height − 1) redundant iterations per tile update.

• HeightMap.cpp: Light-ray loop moved before the vertex-buffer lock and outer row loop; the lightPos pointer local is removed in favour of a const Coord3D& scoped to the hoisted loop.
• BaseHeightMap.h / BaseHeightMap.cpp: doTheLight's light parameter tightened from Vector3* to const Vector3*, correctly reflecting that the function never modifies the array. All existing call sites pass non-const arrays, which implicitly convert to const Vector3* without issue.

The change is a pure performance optimisation with no functional impact. The single Core/ header is the only BaseHeightMap.h in the repository, so the signature change covers all callers.

Confidence Score: 5/5

• Safe to merge — pure loop-invariant hoisting with a correct const-correctness fix and no functional changes.
• No P0/P1 findings. The optimization is semantically equivalent to the original code: the hoisted values are read-only globals that don't change during a single updateVB call. The const Vector3* signature change is backward-compatible with all existing callers. All three changed files score 5/5 individually.
• No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp	Hoists the per-light-index lightRay computation from the inner i,j quad loop to just before the outer loop; removes the now-unnecessary lightPos pointer local variable.
Core/GameEngineDevice/Include/W3DDevice/GameClient/BaseHeightMap.h	Declaration of doTheLight updated to take const Vector3* for the light parameter — a correct const-correctness improvement.
Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp	Definition of doTheLight updated to match the header's new const Vector3* parameter; implementation reads but never writes through the pointer, so const is appropriate.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["updateVB(pVB, data, x0, y0, x1, y1, ...)"] --> B{"m_vertexBufferTiles && pMap?"}
    B -- No --> Z["return 0"]
    B -- Yes --> C["Hoist: for each lightIndex in m_numGlobalLights\nRead TheGlobalData->m_terrainLightPos[lightIndex]\nlightRay[lightIndex] = negated position"]
    C --> D["Lock vertex buffer\n(WriteLockClass)"]
    D --> E["for j = y0 .. y1  (outer row loop)"]
    E --> F["for i = x0 .. x1  (inner cell loop)"]
    F --> G["Compute UVs, alpha, normals\nfor 4 quad vertices"]
    G --> H["doTheLight(vb, lightRay, normal, ...)\n×4 vertices — reads precomputed lightRay"]
    H --> F
    F -- done --> E
    E -- done --> I["Copy memory VB → hardware VB"]
    I --> J["return vertex count"]

Loading

_{Reviews (3): Last reviewed commit: "perf(terrain): Hoist global light ray co..." | Re-trigger Greptile}

[Caball009]

This is outside the scope of this PR, but may need to be addressed at some point. GlobalData::m_numGlobalLights can be read from INI, and it's used in a couple of places to iterate over fixed size arrays, but there's no check anywhere to prevent potential OOB access if m_numGlobalLights is greater than the array sizes.

[Cellcote]

This is outside the scope of this PR, but may need to be addressed at some point. GlobalData::m_numGlobalLights can be read from INI, and it's used in a couple of places to iterate over fixed size arrays, but there's no check anywhere to prevent potential OOB access if m_numGlobalLights is greater than the array sizes.

I guess this would apply to more global settings, right? and that would require a more generic approach to fix for all overridable settings.

[Mauller]

I would also add a check that we are smaller than MAX_GLOBAL_LIGHTS for safety.

Int maxLights = min(TheGlobalData->m_numGlobalLights, MAX_GLOBAL_LIGHTS);
for (Int lightIndex=0; lightIndex < maxLights; lightIndex++)
{
// loop gubbins
}

etc

[Caball009]

This should be done during / after the INI loading, not throughout the code base imo.

Edit: I'd suggest making a separate issue for it and ignore it for this PR. I can create the issue if needed.

[Caball009]

I created an issue for it.

[Caball009]

Looks good to me.