Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Cellcote wants to merge 1 commit into
base: main
Choose a base branch
from
+8 −9
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Core/GameEngineDevice/Include/W3DDevice/GameClient/BaseHeightMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class BaseHeightMapRenderObjClass : public RenderObjClass, public DX8_CleanupHoo
void updateMacroTexture(AsciiString textureName);
void doTextures(Bool flag) {m_disableTextures = !flag;};
/// Update the diffuse value from static light info for one vertex.
void doTheLight(VERTEX_FORMAT *vb, Vector3*light, Vector3*normal, RefRenderObjListIterator *pLightsIterator, UnsignedByte alpha);
void doTheLight(VERTEX_FORMAT *vb, const Vector3*light, Vector3*normal, RefRenderObjListIterator *pLightsIterator, UnsignedByte alpha);
void addScorch(Vector3 location, Real radius, Scorches type);
void addTree(DrawableID id, Coord3D location, Real scale, Real angle,
Real randomScaleAmount, const W3DTreeDrawModuleData *data);
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ static lights into account as well. It is possible to just use the normal in th
vertex and let D3D do the lighting, but it is slower to render, and can only
handle 4 lights at this point. */
//=============================================================================
void BaseHeightMapRenderObjClass::doTheLight(VERTEX_FORMAT *vb, Vector3*light, Vector3*normal, RefRenderObjListIterator *pLightsIterator, UnsignedByte alpha)
void BaseHeightMapRenderObjClass::doTheLight(VERTEX_FORMAT *vb, const Vector3*light, Vector3*normal, RefRenderObjListIterator *pLightsIterator, UnsignedByte alpha)
{
#ifdef USE_NORMALS
vb->nx = normal->X;
Expand Down
13 changes: 6 additions & 7 deletions Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ Int HeightMapRenderObjClass::updateVB(DX8VertexBufferClass *pVB, VERTEX_FORMAT *
{
Int i,j;
Vector3 lightRay[MAX_GLOBAL_LIGHTS];
const Coord3D *lightPos;
Int xCoord, yCoord;
Int vn0,un0,vp1,up1;
Vector3 l2r,n2f,normalAtTexel;
Expand All @@ -320,6 +319,12 @@ Int HeightMapRenderObjClass::updateVB(DX8VertexBufferClass *pVB, VERTEX_FORMAT *
assert(x0 >= originX && y0 >= originY && x1>x0 && y1>y0 && x1<=originX+VERTEX_BUFFER_TILE_LENGTH && y1<=originY+VERTEX_BUFFER_TILE_LENGTH);
#endif

for (Int lightIndex=0; lightIndex < TheGlobalData->m_numGlobalLights; lightIndex++)
Copy link
Copy Markdown

@Mauller Mauller Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown

@Caball009 Caball009 Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

@Caball009 Caball009 Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue for it.

{
const Coord3D& lightPos = TheGlobalData->m_terrainLightPos[lightIndex];
lightRay[lightIndex].Set(-lightPos.x, -lightPos.y, -lightPos.z);
}

DX8VertexBufferClass::WriteLockClass lockVtxBuffer(pVB);
VERTEX_FORMAT *vbHardware = (VERTEX_FORMAT*)lockVtxBuffer.Get_Vertex_Array();
VERTEX_FORMAT *vBase = data;
Expand Down Expand Up @@ -362,12 +367,6 @@ Int HeightMapRenderObjClass::updateVB(DX8VertexBufferClass *pVB, VERTEX_FORMAT *
pMap->getUVData(mapX, mapY, U, V);
pMap->getAlphaUVData(mapX, mapY, UA, VA, alpha, &flipForBlend);

for (Int lightIndex=0; lightIndex < TheGlobalData->m_numGlobalLights; lightIndex++)
{
lightPos=&TheGlobalData->m_terrainLightPos[lightIndex];
lightRay[lightIndex].Set(-lightPos->x,-lightPos->y, -lightPos->z);
}

//top-left sample
l2r.Set(2*MAP_XY_FACTOR,0,MAP_HEIGHT_SCALE*(pMap->getDisplayHeight(mapX+cellOffset, mapY) - pMap->getDisplayHeight(un0, mapY)));
n2f.Set(0,2*MAP_XY_FACTOR,MAP_HEIGHT_SCALE*(pMap->getDisplayHeight(mapX, (mapY+cellOffset)) - pMap->getDisplayHeight(mapX, vn0)));
Expand Down
Loading