Skip to content

bugfix(heightmap): Reduce ray cast lengths and fix ray casting in BaseHeightMapRenderObjClass::Cast_Ray#2836

Open
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/fix-view-raycast
Open

bugfix(heightmap): Reduce ray cast lengths and fix ray casting in BaseHeightMapRenderObjClass::Cast_Ray#2836
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/fix-view-raycast

Conversation

@xezon

@xezon xezon commented Jun 28, 2026

Copy link
Copy Markdown

This change reduces the ray cast lengths from sqr(farZ) to farZ*2 and fixes ray casting in BaseHeightMapRenderObjClass::Cast_Ray.

The sqr(farZ) produced too large ray casts which somehow broke BaseHeightMapRenderObjClass::Cast_Ray and caused infinite loops.

@xezon xezon added Critical Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour ThisProject The issue was introduced by this project, or this task is specific to this project Crash This is a crash, very bad labels Jun 28, 2026
*rayEnd -= *rayStart; //vector camera to world space point
rayEnd->Normalize(); //make unit vector
*rayEnd *= sqr(m_3DCamera->Get_Depth()); //adjust length to reach far clip plane and beyond
*rayEnd *= m_3DCamera->Get_Depth() * 2; //adjust length to reach far clip plane and beyond

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I cannot recall why I picked sqr(farZ) before. farZ*2 appears to also be sufficient.

result.ComputeContactPoint=true;

Int p;
for (p=0; p<3; p++) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have no clue why there was this loop here. It does not do anything. I removed it and it works.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I now see why this loop is there and I put it back and fixed the looping logic as far as I could tell.

@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug where ray cast lengths were computed as sqr(farZ) (far clip distance squared) instead of farZ * 2, producing astronomically large rays that caused excessive cell-iteration performance issues in Cast_Ray. A companion rewrite of the Cast_Ray loop logic replaces the old "early exit on no intersection" pattern with a convergence criterion that breaks when the clipped endpoints stop changing.

  • W3DView.cpp: Both getPickRay and lookAt are updated to use Get_Depth() * 2 as the ray length, which is sufficient to reach the terrain from any valid camera position.
  • BaseHeightMap.cpp: The Cast_Ray refinement loop is restructured with hasP0/hasP1 guards, newP0/newP1 convergence flags, and a post-loop early-return when either clipped endpoint was never established.

Confidence Score: 5/5

Safe to merge; the root cause of the infinite-loop regression is clearly identified and both fix sites are correct.

The W3DView.cpp change is a straightforward arithmetic correction. The Cast_Ray rewrite in BaseHeightMap.cpp is more involved but the logic is sound: the convergence-based break criterion is a correct replacement for the original no-intersection-break exit, and the hasP0/hasP1 guards properly gate the triangle-scan phase. The only quality issue is that P0 and P1 are compared before being initialized on the first iteration, which is technically undefined behaviour but poses no practical risk given the terrain bounding box geometry.

BaseHeightMap.cpp — the first-iteration comparison of uninitialised P0/P1 against the collision contact point.

Important Files Changed

Filename Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Two identical one-line fixes replace sqr(Get_Depth()) with Get_Depth() * 2 for ray length in getPickRay and lookAt; the change is minimal and clearly correct.
Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp Cast_Ray loop logic rewritten with convergence-based break condition; P0/P1 are compared against result.ContactPoint before being initialized on the first iteration (Vector3 default constructor leaves X/Y/Z uninitialized), which is technically UB.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Cast_Ray called"] --> B{"m_map valid?"}
    B -- No --> RETF1["return false"]
    B -- Yes --> C["Init terrain hbox\nlineseg = raytest.Ray"]
    C --> LOOP["Loop p = 0..2"]
    LOOP --> D["result.Reset()\nComputeContactPoint = true\nnewP0 = false, newP1 = false"]
    D --> E{"Forward Collide\nlineseg intersect hbox"}
    E -- No intersection --> BREAK
    E -- Intersects --> F{"result.StartBad?"}
    F -- false --> G["newP0 = P0 != ContactPoint\nhasP0 = true\nP0 = ContactPoint"]
    F -- true --> H["skip P0 update"]
    G --> I["Reverse lineseg2\nCollide lineseg2 intersect hbox"]
    H --> I
    I --> J{"result.StartBad?"}
    J -- false --> K["newP1 = P1 != ContactPoint\nhasP1 = true\nP1 = ContactPoint"]
    J -- true --> BREAK
    K --> BREAK{"!newP0 or !newP1?"}
    BREAK -- true --> POSTLOOP
    BREAK -- false --> M["Compute cell range\nstartCell/endCell from P0,P1"]
    M --> N["Scan heights\nRefine hbox to actual Z range"]
    N --> LOOP
    POSTLOOP["Post-loop check"] --> O{"hasP0 and hasP1?"}
    O -- No --> RETF2["return false"]
    O -- Yes --> P["Adjust cell indices\nby borderSize"]
    P --> Q["Iterate offset 1, 3\nTest every triangle in range"]
    Q --> R["return hit"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Cast_Ray called"] --> B{"m_map valid?"}
    B -- No --> RETF1["return false"]
    B -- Yes --> C["Init terrain hbox\nlineseg = raytest.Ray"]
    C --> LOOP["Loop p = 0..2"]
    LOOP --> D["result.Reset()\nComputeContactPoint = true\nnewP0 = false, newP1 = false"]
    D --> E{"Forward Collide\nlineseg intersect hbox"}
    E -- No intersection --> BREAK
    E -- Intersects --> F{"result.StartBad?"}
    F -- false --> G["newP0 = P0 != ContactPoint\nhasP0 = true\nP0 = ContactPoint"]
    F -- true --> H["skip P0 update"]
    G --> I["Reverse lineseg2\nCollide lineseg2 intersect hbox"]
    H --> I
    I --> J{"result.StartBad?"}
    J -- false --> K["newP1 = P1 != ContactPoint\nhasP1 = true\nP1 = ContactPoint"]
    J -- true --> BREAK
    K --> BREAK{"!newP0 or !newP1?"}
    BREAK -- true --> POSTLOOP
    BREAK -- false --> M["Compute cell range\nstartCell/endCell from P0,P1"]
    M --> N["Scan heights\nRefine hbox to actual Z range"]
    N --> LOOP
    POSTLOOP["Post-loop check"] --> O{"hasP0 and hasP1?"}
    O -- No --> RETF2["return false"]
    O -- Yes --> P["Adjust cell indices\nby borderSize"]
    P --> Q["Iterate offset 1, 3\nTest every triangle in range"]
    Q --> R["return hit"]
Loading

Reviews (2): Last reviewed commit: "Refine fixes to BaseHeightMapRenderObjCl..." | Re-trigger Greptile

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp Outdated
result.StartBad=false;
lineseg2.Set(lineseg.Get_P1(),lineseg.Get_P0()); //reverse line segment
if (CollisionMath::Collide(lineseg2,hbox,&result))
{ if (!result.StartBad) //check if end point inside terrain

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When the ray length was very large, then this collision here failed. I am not sure why but I suspect because of floating point precision loss at very large numbers.

When it failed, then P1 was a very large number, and the cells loop further below would then iterate over very large integer numbers which practically meant the application hung up.

With this change the ray cast now aborts early if this collision test failed, which avoids all this trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Crash This is a crash, very bad Critical Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ThisProject The issue was introduced by this project, or this task is specific to this project ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant