perf(smudge): Early return W3DSmudgeManager::render when no smudges are present

[stephanmeesters]

In #2484 a global smudge set was introduced through which all smudge particles were rendered.

As a result there is always at least one smudge set in the list, which meant that W3DSmudgeManager::render would always do some work before discovering there are no smudges in the set. This PR adds an early return to skip some of these calculations including a call to SortingRendererClass::Flush().

Performance gain: ~7.5µs per frame

Todo

• Get some estimate of the performance gain

[qodo-free-for-open-source-projects]

PR Summary by Qodo

perf(smudge): Skip W3DSmudgeManager::render work when no smudges exist

✨ Enhancement 🕐 Less than 10 minutes

AI Description

• Add an early return in smudge rendering when the used-set list is empty or global set has zero
 smudges.
• Avoid unnecessary hardware-support checks and an otherwise unconditional
 SortingRendererClass::Flush().

Diagram

graph TD
  A["Game client frame"] --> B["W3DSmudgeManager::render"] --> C{"Any smudges?"} -->|"No"| D["Return (skip work)"]
  C -->|"Yes"| E["testHardwareSupport"] --> F["SortingRendererClass::Flush"] --> G["Render smudges"]

  subgraph Legend
    direction LR
    _proc["Process"] ~~~ _dec{"Decision"}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Scan sets for first non-empty before flushing

• ➕ Does not rely on the invariant that the global smudge set is always the front element
• ➕ Still avoids Flush() when all sets are empty
• ➖ Adds a small O(n) scan each frame (typically tiny, but still extra work)
• ➖ Slightly more code than a single front()/count check

2. Maintain a global total-smudge counter/dirty flag

• ➕ O(1) check without relying on container ordering
• ➕ Can be reused for other early-outs/telemetry
• ➖ Requires careful bookkeeping on every add/remove/clear path
• ➖ Higher risk of counter desync bugs than the current minimal change

Recommendation: The current early-return is the best minimal fix for the regression described, as long as the ordering invariant holds (global smudge set always at front). If that invariant isn’t formally guaranteed, prefer scanning for a non-empty set (or maintaining a total counter) to avoid future subtle correctness/perf regressions.

Files changed (1) +7 / -5

Enhancement (1) +7 / -5

W3DSmudge.cppEarly-exit smudge rendering when no smudges exist; always flush once rendering proceeds +7/-5

Early-exit smudge rendering when no smudges exist; always flush once rendering proceeds

• Adds an early return at the start of W3DSmudgeManager::render when there are no used smudge sets or the global set has zero smudges, avoiding downstream work including SortingRendererClass::Flush(). Removes a now-redundant conditional around Flush(), since the function only continues when rendering is expected to happen.

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

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. Stale smudge count ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new early-return path in W3DSmudgeManager::render exits before updating m_smudgeCountLastFrame,
so SmudgeManager::getSmudgeCountLastFrame() can return a previous-frame value when there are no
smudges/sets. This breaks the contract of a “last frame” counter on frames where the global smudge
set is empty.

Code

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[R315-316]

+	if(m_usedSmudgeSetList.empty() || m_usedSmudgeSetList.front()->getUsedSmudgeCount() == 0)
+		return;

Evidence

The early return occurs before the only assignment to m_smudgeCountLastFrame, and the value is
exposed via a public accessor; therefore frames that hit the early return will not update the
last-frame count.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[311-316]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[429-436]
Core/GameEngine/Include/GameClient/Smudge.h[104-108]

Agent prompt

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

## Issue description
`W3DSmudgeManager::render` now returns early when there are no smudge sets or no smudges in the global set, but it does so before updating `m_smudgeCountLastFrame`. This leaves `SmudgeManager::getSmudgeCountLastFrame()` returning a stale non-zero value on subsequent empty frames.
## Issue Context
`m_smudgeCountLastFrame` is only assigned later in `render` after the visibility pass. With the new early return, that assignment is skipped entirely.
## Fix Focus Areas
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[311-317]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[429-436]
## Proposed fix
Set `m_smudgeCountLastFrame = 0;` before returning from the new early-return condition (or otherwise ensure it is updated on all return paths that represent “no smudges rendered this frame”).

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

---

2. Support check never runs ✓ Resolved 🐞 Bug ☼ Reliability

Description

The early return can prevent testHardwareSupport() from ever being called when the global smudge set
remains empty, leaving m_hardwareSupportStatus stuck at SMUDGE_SUPPORT_UNKNOWN. Since
getHardwareSupport() treats UNKNOWN as supported, upstream code may keep smudge-related processing
enabled even on unsupported hardware until at least one smudge exists.

Code

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[R315-320]

+	if(m_usedSmudgeSetList.empty() || m_usedSmudgeSetList.front()->getUsedSmudgeCount() == 0)
+		return;
+
//Verify that the card supports the effect.
if (!testHardwareSupport())
return;

Evidence

The early return precedes the support test. The support flag defaults to UNKNOWN and
getHardwareSupport() treats UNKNOWN as supported; upstream uses getHardwareSupport() to decide
whether to run smudge setup/marking, and testHardwareSupport() is where UNKNOWN is resolved to
YES/NO.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[311-320]
Core/GameEngine/Source/GameClient/System/Smudge.cpp[36-40]
Core/GameEngine/Include/GameClient/Smudge.h[106-113]
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp[3006-3015]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[140-145]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp[371-375]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[203-221]

Agent prompt

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

## Issue description
`W3DSmudgeManager::render` now returns before calling `testHardwareSupport()` when the smudge-set list is empty or the global set has zero smudges. This can leave `m_hardwareSupportStatus` as `SMUDGE_SUPPORT_UNKNOWN` indefinitely, while `getHardwareSupport()` reports support for all states except `SMUDGE_SUPPORT_NO`.
## Issue Context
- `SmudgeManager` initializes `m_hardwareSupportStatus` to `SMUDGE_SUPPORT_UNKNOWN`.
- `getHardwareSupport()` returns true for UNKNOWN.
- Multiple call sites gate smudge work on `getHardwareSupport()`.
- `testHardwareSupport()` is the codepath that transitions UNKNOWN -> YES/NO.
## Fix Focus Areas
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[311-320]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp[203-221]
- Core/GameEngine/Include/GameClient/Smudge.h[106-113]
- Core/GameEngine/Source/GameClient/System/Smudge.cpp[36-40]
## Proposed fix
Preserve the perf win (skipping Flush/visibility pass/backbuffer copy) while still allowing capability detection by:
1) Calling `testHardwareSupport()` before the early-return check (it is effectively one-time work due to internal caching), or
2) Calling `testHardwareSupport()` only when `m_hardwareSupportStatus == SMUDGE_SUPPORT_UNKNOWN` even if returning early.
Either approach avoids leaving hardware support permanently UNKNOWN in smudge-free sessions.

ⓘ 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 ↗

[Mauller]

Another optimisation could be to move TestHardwareSupport into the init function instead of constantly calling it every render frame.

[stephanmeesters]

Another optimisation could be to move TestHardwareSupport into the init function instead of constantly calling it every render frame.

Perhaps but the hardware test only has a cost the first time and the result is cached afterward, and a benefit of this lazy check is that it can easily rerun after a device reset (actually it's not currently doing that, seems like a bug).

[Mauller]

Another optimisation could be to move TestHardwareSupport into the init function instead of constantly calling it every render frame.

Perhaps but the hardware test only has a cost the first time and the result is cached afterward, and a benefit of this lazy check is that it can easily rerun after a device reset (actually it's not currently doing that, seems like a bug).

Ah yes i see it now, the function is architected in an annoying way and could have had an early return.

[bobtista]

after DoParticles() returns, RTS3DScene::Flush() immediately calls SortingRendererClass::Flush() again.
Before: the inner call flushes the particle queue and the outer call is empty
After: the outer call flushes the queue instead.
So this removes one empty Flush() invocation, then the backbuffer/surface and visibility setup, right?

[stephanmeesters]

So this removes one empty Flush() invocation, then the backbuffer/surface and visibility setup, right?

Yeah that sounds correct, so the expected performance gain is small (but every bit helps)