perf(particlesys): Batch same type particles to improve particle rendering performance by 15 - 30% by Mauller · Pull Request #3155 · TheSuperHackers/GeneralsGameCode

Mauller · 2026-08-15T16:48:29Z

This can be squash merged

This PR is separated into two commits to aid reviewing.
The initial commit is a small refactor to make the diff slightly cleaner on the second commit.
The second commit implements the particle batching created by Ronin and cleaned up by myself.

When testing we see a 15-20% performance improvement on average. But this may be higher in some scenarios.
EDIT: An early particle visibility test has added 2 - 5% more performance on top of the original.

The batching works by creating a common texture that the particle effects are drawn to before being sent to the GPU. This reduces the number of draw calls, thus improving rendering performance.

Only particles with the same material and shader type can be batched to the texture, so once particles of a different type are observed. The system will flush the prior batch and start a new one based on the new particle type.

As draw order is preserved, particles adhere to their original layering.

Some performance comparison images

Using the chemical spray as the source of particles, all tractors are using their AoE spray ability when these images were taken.

Before:

After:

Using the firewall and flamethrower effect of the flame tank, a circle of flame tanks are creating firewalls in the centre while extra flamers spray fire into the centre of the inferno.

Before:

After:

qodo-free-for-open-source-projects · 2026-08-15T16:49:43Z

PR Summary by Qodo

perf(particlesys): Batch consecutive particle systems to reduce draw calls

✨ Enhancement 🕐 40+ Minutes

AI Description

• Batch consecutive particle systems sharing texture/shader/billboard state into a single draw.
• Flush the batch on material/shader changes, streak/volume particles, or buffer full.
• Make particle type name access const-ref to reduce copies during render setup.

Diagram

graph TD
  A["doParticles(): iterate systems"] --> B{"Batchable?"} -->|"no"| C["flushParticleBatch()"] --> D["Render immediately (streak/volume)"]
  B -->|"yes"| E["Append to shared buffers"] --> F{"Type/state change or full?"} -->|"yes"| C
  F -->|"no"| G["Keep pending batch"]
  H["End of doParticles()"] --> C

High-Level Assessment

The following are alternative approaches to this PR:

1. Stable bucket + late flush (collect then render)

➕ Potentially batches across non-consecutive systems by grouping same texture/shader.
➕ Can keep batching logic separate from per-system particle extraction.
➖ Hard to preserve strict draw order without additional depth/layer bucketing.
➖ Higher memory use (must retain per-system particle data until end).
➖ Riskier change to ordering semantics compared to current incremental flush.

2. GPU instancing / dynamic vertex-buffer aggregation

➕ Avoids render-state churn while keeping direct sprite rendering.
➕ Can scale better than CPU-side batching if engine supports it.
➖ May require deeper WW3D pipeline changes (new shaders/vertex formats).
➖ Higher implementation and compatibility risk across existing particle types.

Recommendation: Current approach (incremental batching with immediate flush on state change) is a good fit for preserving draw order while reducing draw calls with minimal engine-wide disruption. The main review focus should be correctness of flush boundaries (texture/shader/billboard changes, streak/volume exclusions, buffer-full mid-system) and texture ref-count handling across all paths.

Files changed (4) +172 / -54

Enhancement (3) +171 / -53

W3DParticleSys.cppBatch compatible particle systems and add flushParticleBatch() +155/-53

Batch compatible particle systems and add flushParticleBatch()

• Introduces batching state (texture/shader/billboard) and accumulates particles from consecutive compatible systems into shared buffers tracked by m_pointCount. Adds flushParticleBatch() to render the accumulated batch and resets state; flushes on state changes, non-batchable systems (streak/volume), buffer-full mid-system, and at the end of the frame.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp

W3DParticleSys.hAdd batching state fields and flush helper declaration +8/-0

Add batching state fields and flush helper declaration
• Adds flushParticleBatch() declaration and new member fields (batch toggles, shader type, point count, batch texture) to support deferred batched rendering.
Generals/Code/GameEngineDevice/Include/W3DDevice/GameClient/W3DParticleSys.h

W3DParticleSys.hMirror batching state fields and flush helper for MD build +8/-0

Mirror batching state fields and flush helper for MD build
• Keeps the GeneralsMD header in sync by adding the same batching members and flushParticleBatch() declaration used by the core W3D implementation.
GeneralsMD/Code/GameEngineDevice/Include/W3DDevice/GameClient/W3DParticleSys.h

Refactor (1) +1 / -1

ParticleSys.hReturn particle type name as const reference (const-correct) +1/-1

Return particle type name as const reference (const-correct)
• Changes getParticleTypeName() to return a const AsciiString& and marks it const. This avoids string copies in render code paths and allows calling through const contexts.
Core/GameEngine/Include/GameClient/ParticleSys.h

qodo-free-for-open-source-projects · 2026-08-15T16:54:32Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Batch texture ref leak~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

W3DParticleSystemManager retains an extra reference to m_batchTexture via Add_Ref() during batching,
but the destructor never releases it, so destroying the manager with a pending batch can leak a
TextureClass ref and prevent proper cleanup. Normal end-of-frame flushing usually releases it, but
lifecycle paths that skip the final flush (shutdown/teardown) can still leak.

Code

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[R210-213]
+		if (canBatch && m_batchTexture == nullptr)
+		{
+			m_batchTexture = texture;
+			m_batchTexture->Add_Ref();

Evidence
The batching code explicitly increments the texture refcount for m_batchTexture, and
flushParticleBatch() decrements it, but the class destructor does not release m_batchTexture at
all, violating the refcount ownership rules for stored pointers.
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[195-216]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[256-282]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[441-478]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[75-90]
Core/Libraries/Source/WWVegas/WWLib/refcount.h[67-83]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`W3DParticleSystemManager` now owns a ref-counted `m_batchTexture` (via `Add_Ref()`), but its destructor doesn’t release that ownership. If the manager is destroyed while a batch is pending, the texture ref can leak.
### Issue Context
- `Get_Texture()` returns an add-ref’d `TextureClass*`, and any retained pointer must be released per `refcount.h` rules.
- `flushParticleBatch()` releases `m_batchTexture`, but destructor cleanup should not rely on a render-path being invoked.
### Fix Focus Areas
- Add destructor cleanup for the new owned member (`m_batchTexture`) and reset batch state.
- file/path[start_line-end_line]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[47-90]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[209-216]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[441-478]
### Suggested change
In `~W3DParticleSystemManager()`, add e.g.:
- `REF_PTR_RELEASE(m_batchTexture);`
- `m_pointCount = 0;`
(Optionally do this before/after deleting `m_pointGroup`; either is fine since both are ref-counted owners.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Unconditional texture lookup~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

doParticles now calls Get_Texture() before it knows whether any particles survive culling
(m_pointCount==startCount), causing unnecessary texture-hash lookup and refcount inc/dec work for
systems that render nothing. This adds avoidable per-system overhead on the empty/fully-culled path.

Code

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[R195-199]

+		// TheSuperHackers @perf 09/08/2026 Ronin/Mauller Implement batched rendering for similar particles
+		// Particles with the same blending will now be batched onto a single texture surface before being drawn
+		// If a different particle type appears before the batch is filled, the previous batch will be drawn first
+		TextureClass *texture = W3DDisplay::m_assetManager->Get_Texture( sys->getParticleTypeName().str() );
+		const Bool canBatch = !( m_streakLine && sys->isUsingStreak() ) && ( sys->getVolumeParticleDepth() <= 1 );

Evidence
The code fetches the texture (which includes an Add_Ref) before particle culling, and then
immediately releases it in the no-particles path, proving the extra work occurs even when nothing is
rendered.
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[195-206]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[285-289]
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DAssetManager.cpp[157-181]
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DAssetManager.cpp[221-229]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Get_Texture()` is invoked for every non-smudge particle system before culling determines whether the system contributes any renderable particles. For fully-culled/empty systems this work is wasted.
### Issue Context
`W3DAssetManager::Get_Texture()` performs a hash lookup and `Add_Ref()` before returning, so calling it for systems that end up not rendering does extra work and refcount churn.
### Fix Focus Areas
- Lazily acquire the texture only after the first particle passes culling (before writing into the batch buffers), so fully-culled systems never call `Get_Texture()`.
- Ensure batching flush/setup still happens before appending any particles into the shared buffers.
- file/path[start_line-end_line]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[195-218]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[231-290]
- Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DAssetManager.cpp[157-229]
### Suggested approach
- Initialize `TextureClass* texture = nullptr;` and postpone `Get_Texture()` until you encounter the first particle that passes the cull checks.
- At that moment, run the current batch-compatibility checks and potentially `flushParticleBatch(rinfo)` **before** writing that first particle into the buffers.
- If no particles pass culling, skip texture acquisition entirely.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

stephanmeesters · 2026-08-15T17:29:03Z

I think W3DParticleSys.h was supposed to have been moved to core in #3014 but wasn't...

Mauller · 2026-08-16T08:20:34Z

I think W3DParticleSys.h was supposed to have been moved to core in #3014 but wasn't...

Outside the scope of this change, but i noticed that too.

Mauller · 2026-08-16T09:25:49Z

Made another modification to perform an earlier visibility check, it can help prevent non visible particle systems from causing a batch flush. It also saves a bit of overhead later on.

It can give a bit of extra perf due to this, around 2 - 5% more.

Mauller · 2026-08-16T09:41:49Z

Ah stupid VC6 loop handling, will just fixing now

Mauller · 2026-08-16T09:47:48Z

Fixed VC6 build and issues mentioned by the bot

stephanmeesters · 2026-08-16T13:09:44Z

 W3DParticleSystemManager::W3DParticleSystemManager()
 {
+	m_batchBillboard = true;
+	m_batchParticleSystems = true;


Perhaps use a define instead as this is always true?

it's better to not use defines and to use constants instead if something is not changing.

It is always true. What us the point of this bool?

stephanmeesters · 2026-08-16T13:16:33Z

-				m_pointGroup->Set_Flag( PointGroupClass::TRANSFORM, true );	// transform to screen space
-
-				switch( sys->getShaderType() )
+				if ( sys->getVolumeParticleDepth() > 1 )


Would it make things simpler/more consistent if volume particles were batched as well?

volume particles work in a different way as they have multiple surfaces.
The batching only really works with billboarded / flat particles

Are they really that different though, they appear to use the same input arrays. The only difference is that they call a different render function and render more surfaces

Would need to find something that uses volume particles, all my current tests don't show any activity down that path

Found that the microwave tank uses them.

xezon

First review pass. Reference counting needs simplification.

xezon · 2026-08-16T16:19:11Z

 		if (sys->isUsingDrawables())
 			continue;

+		// TheSuperHackers @perf 16/08/2026 Mauller Test if particle system has any visible particles that can be drawn


date after author

fixed and changes @Perf to @performance

xezon · 2026-08-16T16:20:30Z

+			const Coord3D* pos = vp->getPosition();
+			Real psize = vp->getSize();
+
+			//Test if particle is at the screen or terrain edges.


This test exists more than once in this file. Can consolidate and simplify.

I tried by having the visibility test section put particles that are visible into a list, but the performance was lower than just testing again and running through all particles later on.

There is likely an element of memory locality to it which putting pointers to the particle system on a list loses.

Fixed by setting the culled variable on the particles, they always had the variable but it has not been used till now.

xezon · 2026-08-16T16:23:45Z

+
 	enum { MAX_POINTS_PER_GROUP = 512 };

+	TextureClass *m_batchTexture;								///< the texture used as the drawing surface for batched particle draws


RefCountPtr<TextureClass>

xezon · 2026-08-16T16:24:52Z

 W3DParticleSystemManager::W3DParticleSystemManager()
 {
+	m_batchBillboard = true;
+	m_batchParticleSystems = true;


It is always true. What us the point of this bool?

xezon · 2026-08-16T16:27:42Z

+		// TheSuperHackers @perf 09/08/2026 Ronin/Mauller Implement batched rendering for similar particles
+		// Particles with the same blending will now be batched onto a single texture surface before being drawn
+		// If a different particle type appears before the batch is filled, the previous batch will be drawn first
+		TextureClass *texture = W3DDisplay::m_assetManager->Get_Texture( sys->getParticleTypeName().str() );


RefCountPtr

xezon · 2026-08-16T16:32:40Z

+	Bool m_batchBillboard;
+	Bool m_batchParticleSystems;
+	ParticleSystemInfo::ParticleShaderType m_batchShaderType;
+	Int m_pointCount;


Maybe can be unsigned

made it unsigned and put it back within doParticles() outside of the particle system list loop

xezon · 2026-08-16T16:39:13Z

+	m_batchBillboard = true;
+	m_batchParticleSystems = true;
+	m_batchShaderType = ParticleSystemInfo::INVALID_SHADER;
 	m_pointCount = 0;


m_pointCount does not need to be a class member. Is used in one function. Can be passed as argument to the flush function. m_pointCount as class member also poses risk from early returns.

Yeah when looking at it again i forgot that we flush the last batch anyway and don't batch between calls to doParticles()

going to make it a function member again.

xezon · 2026-08-16T16:42:42Z

This function now does these culling tests 3 times. Can we optimize this?

Something else i could try is adding a flag to the particle which the first visibility test sets to say if the particle is visible on screen.

fixed by setting and using the culled variable on the particles.

… for further optimisations

Mauller · 2026-08-16T17:49:53Z

Partially addressed review comments, things still appear to give a good 15 - 30% more performance with the tweaks.

…visibility skip until the next upstream rebase

Mauller self-assigned this Aug 15, 2026

Mauller added Major Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Gen Relates to Generals ZH Relates to Zero Hour labels Aug 15, 2026

qodo-free-for-open-source-projects Bot reviewed Aug 15, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp Outdated

Caball009 reviewed Aug 15, 2026

View reviewed changes

Comment thread Generals/Code/GameEngineDevice/Include/W3DDevice/GameClient/W3DParticleSys.h Outdated

Mauller force-pushed the Mauller/perf-batch-particle-draws branch from 62747ca to 315c30c Compare August 16, 2026 09:26

Mauller changed the title ~~perf(particlesys): Batch same type particles to improve particle rendering performance by 15-20%~~ perf(particlesys): Batch same type particles to improve particle rendering performance by 20 - 30% Aug 16, 2026

Mauller changed the title ~~perf(particlesys): Batch same type particles to improve particle rendering performance by 20 - 30%~~ perf(particlesys): Batch same type particles to improve particle rendering performance by 15 - 30% Aug 16, 2026

Mauller force-pushed the Mauller/perf-batch-particle-draws branch from 315c30c to 0df2c07 Compare August 16, 2026 09:47

stephanmeesters reviewed Aug 16, 2026

View reviewed changes

Mauller force-pushed the Mauller/perf-batch-particle-draws branch from 0df2c07 to 0e8506d Compare August 16, 2026 14:06

xezon reviewed Aug 16, 2026

View reviewed changes

Mauller added 2 commits August 16, 2026 18:36

chore(particlesys): Refactor point count into a class variable, ready…

15b7f3c

… for further optimisations

perf(particlesys): Implement batched rendering for particles (#)

4b5ec70

Mauller force-pushed the Mauller/perf-batch-particle-draws branch from 0e8506d to 4b5ec70 Compare August 16, 2026 17:45

bobtista added a commit to bobtista/GeneralsGameCode that referenced this pull request Aug 17, 2026

chore(particlesys): Carry the PR TheSuperHackers#3155 early particle …

1e2bfd7

…visibility skip until the next upstream rebase


		enum { MAX_POINTS_PER_GROUP = 512 };

		TextureClass *m_batchTexture; ///< the texture used as the drawing surface for batched particle draws

Uh oh!

Conversation

Mauller commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 15, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

stephanmeesters commented Aug 15, 2026

Uh oh!

Uh oh!

Mauller commented Aug 16, 2026

Uh oh!

Mauller commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mauller commented Aug 16, 2026

Uh oh!

Mauller commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mauller Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Mauller commented Aug 15, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 15, 2026 •

edited

Loading

Mauller commented Aug 16, 2026 •

edited

Loading

Mauller commented Aug 16, 2026 •

edited

Loading

Mauller Aug 16, 2026 •

edited

Loading

Mauller Aug 16, 2026 •

edited

Loading