feat: Terrain conforming particles - #3245
Conversation
stephanmeesters
added
Enhancement
b0d2d9b to
4b2af0a
Compare
9491602 to
11b2e6b
Compare
11b2e6b to
60d17b0
Compare
PR Summary by QodoAdd terrain-conforming particle rendering
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
|
| Filename | Overview |
|---|---|
| Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainParticle.cpp | Implements terrain-bound calculation, recursive mesh generation, dynamic lookup sizing, batching, and rendering; the previous non-finite-size concern remains in its existing thread. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp | Routes conforming particle batches through the new renderer while preserving texture, shader, alignment, and capacity flush boundaries. |
| Core/GameEngine/Include/GameClient/ParticleSys.h | Adds the conforming alignment option and classifies it as a field-particle mode. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp | Adds a terrain-flatness query used to coarsen conforming particle meshes. |
| GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Displays terrain-particle triangle and draw-call statistics. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
PS[Particle system] --> B[Particle batch manager]
B -->|Billboard or planar| PG[PointGroup renderer]
B -->|Conforming| TP[Terrain particle renderer]
TP --> TB[Intersect particle and visible terrain bounds]
TB --> SR[Recursively subdivide non-flat regions]
SR --> M[Generate terrain-aligned mesh]
M --> DX[Submit dynamic vertex and index buffers]
Reviews (3): Last reviewed commit: "feat(terrainparticle): Add define guard ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| { | ||
| const Int gridLocation = (y - particle.bounds.lo.y) * particle.bounds.width() + x - particle.bounds.lo.x; | ||
| UnsignedShort& index = m_vertexLookup[gridLocation]; |
There was a problem hiding this comment.
When a conforming particle covers more than 32,768 terrain grid points, gridLocation can exceed the fixed 32,768-entry m_vertexLookup. Particle size has no upper bound, and the bounds are clipped only to the map and visible area, so a sufficiently large particle can access memory outside the lookup buffer, corrupt memory, or crash the renderer.
Knowledge Base Used: Rendering and video devices
Prompt To Fix With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainParticle.cpp
Line: 203-205
Comment:
**Lookup Buffer Overflow**
When a conforming particle covers more than 32,768 terrain grid points, `gridLocation` can exceed the fixed 32,768-entry `m_vertexLookup`. Particle size has no upper bound, and the bounds are clipped only to the map and visible area, so a sufficiently large particle can access memory outside the lookup buffer, corrupt memory, or crash the renderer.
**Knowledge Base Used:** [Rendering and video devices](https://app.greptile.com/thesuperhackers/-/custom-context/knowledge-base/thesuperhackers/generalsgamecode/-/docs/rendering-and-video-devices.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed. This would have given problems only for extremely large particles, over 180x180 terrain tiles,
| vertex.u1 = 0.5f - localX / (2.0f * particle.size); | ||
| vertex.v1 = 0.5f - localY / (2.0f * particle.size); |
There was a problem hiding this comment.
A zero-sized conforming particle can reach these UV calculations because particle parsing and runtime size updates do not require a positive value. If its bounds still form a quad, dividing by 2.0f * particle.size produces infinite or NaN texture coordinates, resulting in malformed or unpredictable rendering. Non-positive and non-finite sizes should be skipped before generating geometry.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainParticle.cpp
Line: 221-222
Comment:
**Invalid Particle Size Division**
A zero-sized conforming particle can reach these UV calculations because particle parsing and runtime size updates do not require a positive value. If its bounds still form a quad, dividing by `2.0f * particle.size` produces infinite or NaN texture coordinates, resulting in malformed or unpredictable rendering. Non-positive and non-finite sizes should be skipped before generating geometry.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Added a size check to render().
Would have expected a cull of zero-sized particles somewhere earlier but that does not appear to be the case.
There was a problem hiding this comment.
That’s the right place for the check: before calculating projectedRadius, terrain bounds, or UVs. There does not appear to be a guaranteed earlier cull for zero or invalid runtime-updated sizes, so guarding in render() is appropriate. A size <= 0.0f check resolves the reported division-by-zero case; non-finite values would need a separate finite-value check if they can reach the particle arrays.
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| vertex.u1 = 0.5f - localX / (2.0f * particle.size); | ||
| vertex.v1 = 0.5f - localY / (2.0f * particle.size); |
There was a problem hiding this comment.
2. Terrain particles render twice as large 🐞 Bug ≡ Correctness
W3DTerrainParticle treats Particle::getSize() as a half-width even though PointGroupClass treats the same value as the quad's full width. Every conforming system therefore covers twice the intended width and height, changing the appearance and area of existing particle definitions when they select the new alignment.
Agent Prompt
## Issue description
Terrain-conforming particles interpret the existing particle size as a half-width, making them twice as wide and tall as equivalent point-group particles.
## Fix Focus Areas
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainParticle.cpp[119-124]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainParticle.cpp[217-223]
## Recommended Fix
Treat size as the full quad width. Compute the rotated projected radius from `size * 0.5f`, and map texture coordinates so offsets of `-size/2` and `+size/2` correspond to the texture edges.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
It is rendered at the same size as the pointgroup flat version, so it looks as expected on flat terrain.
Interestingly billboarded particles interpret particle size differently, and are rendered at half the size compared to the same flat particle.
| { | ||
| PARTICLE_ALIGNMENT_BILLBOARD = 0, | ||
| PARTICLE_ALIGNMENT_XYPLANAR, | ||
| PARTICLE_ALIGNMENT_CONFORMING, |
There was a problem hiding this comment.
7. Editing effects removes terrain alignment 🐞 Bug ⚙ Maintainability
DebugWindowDialog::getSwitchFromSystem recognizes only XY-planar alignment and updateSwitchToSystem maps an unchecked alignment control to billboard mode. Once the new conforming enum value is loaded into the Particle Editor, editing any switches-dialog setting writes that unchecked state back and replaces conforming alignment with billboard.
Agent Prompt
## Issue description
The Particle Editor represents alignment as a boolean XY-plane checkbox. It cannot represent the new conforming enum value and writes the unchecked value back as billboard alignment during unrelated switch edits.
## Fix Focus Areas
- Core/GameEngine/Include/GameClient/ParticleSys.h[437-443]
- Core/Tools/ParticleEditor/ParticleEditorDialog.cpp[1085-1114]
- Core/Tools/ParticleEditor/CSwitchesDialog.cpp[74-127]
## Recommended Fix
Make the editor expose all alignment enum values, preferably with a three-value control, and write the selected enum value directly. At minimum, preserve conforming alignment when the legacy XY-plane switch is not changed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
ParticleEditor support for conforming particles should be handled in a later PR
There was a problem hiding this comment.
Dismissed — deferred.
60d17b0 to
e389227
Compare
e389227 to
4a54116
Compare
1 participant
Merge by rebase
Summary
Makes it possible to render particles correctly on uneven terrain.
IsGroundAligned = CONFORMING.ENABLE_TERRAIN_CONFORMING_PARTICLES.Method
W3DTerrainParticleis a combination of (an evolved version of)W3DScorchandPointGroupClass.Testing
How-to
Using a test build
See this test build.
Using data patch
Building Patch2 with a modification to particle systems see the data PR.
Using code
Alternatively, you can cherry-pick this commit which force-enables the right particles:
Performance impact
The test build was used to test a moderately heavy scene (3 nuke cannons + 3 nukes shooting at a hill in Alpine Assault) using a Macbook Pro 2013 base spec aka Potato, which showed a 5.5% average FPS loss.
Image gallery
More
Todo