bugfix(rendering): Fix Microwave Heat Haze blackout on forced AA

[githubawn]

This PR aims to resolve the black screen bug caused by driver-forced MSAA, continuing and refining the work started in #1073.

The Bug: When users force MSAA via their driver control panel, the driver secretly upgrades the depth buffer to be multisampled. However, the Direct3D 8 API still reports MultiSampleType=NONE for created textures. When ScreenDefaultFilter attempts to use Render-To-Texture (RTT), it binds a non-MSAA texture to this secretly-MSAA depth buffer. This surface mismatch is a D3D API violation that silently breaks depth testing, resulting in a black screen.

This PR fixes the issue by permanently disabling the RTT path inside ScreenDefaultFilter::preRender (falling back to the CopyRects smudge path).

[greptile-apps]

Greptile Summary

This PR fixes the driver-forced MSAA black screen by permanently disabling the RTT path in ScreenDefaultFilter::preRender (unconditionally returns FALSE) and routing all heat-haze/smudge rendering through the CopyRects path. As defence-in-depth, MSAA is detected early in W3DShaderManager::init() by inspecting D3DSURFACE_DESC.MultiSampleType, and GetRenderTarget is now properly null-checked before dereferencing. The SAFE_RELEASE macro is centralised from W3DWater.cpp into WWCommon.h, and all #ifdef USE_COPY_RECTS guards are removed.

Key strengths:

• Core fix is logically sound: the copy path fallback is correct and bypasses the MSAA mismatch
• The early MSAA detection in init() + runtime failsafe in startRenderToTexture() provide layered defence
• All RTT resource cleanup is now unified via SAFE_RELEASE

Remaining concerns:

• ScreenDefaultFilter::postRender is now permanently unreachable dead code but retains a DEBUG_ASSERTCRASH that would fire if ever called, creating a maintenance trap
• SAFE_RELEASE macro lacks a redefinition guard, risking macro conflicts in future includes

Confidence Score: 3/5

• The core MSAA fix is logically sound, but two maintenance issues require attention: a dead code function with a live crash assertion, and a macro lacking redefinition protection.
• The MSAA blackout fix is well-reasoned with proper defence-in-depth: early detection, null-checks, and a fallback path. However, confidence is reduced to 3/5 due to two issues: (1) ScreenDefaultFilter::postRender is now permanently unreachable but retains a DEBUG_ASSERTCRASH that creates a maintenance trap if the RTT path is ever re-enabled, and (2) the SAFE_RELEASE macro centralisation lacks a redefinition guard, risking future include conflicts. Both are actionable but not blockers.
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DShaderManager.cpp (postRender dead code with live assertion) and Core/Libraries/Source/WWVegas/WWLib/WWCommon.h (macro guard missing).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[W3DView: filterPreRender] --> B{ScreenDefaultFilter::preRender}
    B -- "always returns FALSE\n(new behaviour)" --> C[preRenderResult = FALSE\npostRender never called]

    A2[W3DShaderManager::init] --> D{GetRenderTarget}
    D -- "hr fail or null" --> E[return early\nno RTT resources]
    D -- "hr OK" --> F{desc.MultiSampleType\n== D3DMULTISAMPLE_NONE?}
    F -- "Yes: no MSAA" --> G[CreateTexture\nfor RTT]
    F -- "No: MSAA detected" --> H[hr = E_FAIL\nskip texture creation]
    G --> I{CreateTexture OK?}
    H --> I
    I -- "fail" --> J[SAFE_RELEASE oldRenderSurface\nRTT disabled]
    I -- "OK" --> K[RTT resources ready\nbut never used by\nScreenDefaultFilter]

    A3[W3DSmudgeManager::render] --> L{testHardwareSupport}
    L -- "RTT null or inactive +\nm_backgroundTexture exists" --> M[SMUDGE_SUPPORT_YES\nuse CopyRects path]
    L -- "RTT null + no background" --> N[SMUDGE_SUPPORT_NO\nskip render]
    M --> O[Copy backbuffer → m_backgroundTexture]
    O --> P[Render smudge quads\nusing background texture]

Loading

Comments Outside Diff (1)

1. Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DShaderManager.cpp, line 183-205 (link)

   ScreenDefaultFilter::postRender is now unreachable dead code due to preRender unconditionally returning FALSE, yet it contains a live DEBUG_ASSERTCRASH that will fire if this function is ever reached:

   IDirect3DTexture8 * tex = W3DShaderManager::endRenderToTexture();
   DEBUG_ASSERTCRASH(tex, ("Require rendered texture."));

   Since preRender returns FALSE, startRenderToTexture() is never called, so m_renderingToTexture is never set to true. If this function becomes reachable again in the future (e.g., if the RTT path is re-enabled), endRenderToTexture() will return nullptr and the assert will immediately crash.

   The function should either be removed or clearly annotated as deliberately inert. At minimum, the DEBUG_ASSERTCRASH should be removed or conditioned to prevent this maintenance trap.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DShaderManager.cpp
   Line: 183-205
   
   Comment:
   `ScreenDefaultFilter::postRender` is now unreachable dead code due to `preRender` unconditionally returning `FALSE`, yet it contains a live `DEBUG_ASSERTCRASH` that will fire if this function is ever reached:
   
   ```cpp
   IDirect3DTexture8 * tex = W3DShaderManager::endRenderToTexture();
   DEBUG_ASSERTCRASH(tex, ("Require rendered texture."));
   ```
   
   Since `preRender` returns `FALSE`, `startRenderToTexture()` is never called, so `m_renderingToTexture` is never set to `true`. If this function becomes reachable again in the future (e.g., if the RTT path is re-enabled), `endRenderToTexture()` will return `nullptr` and the assert will immediately crash.
   
   The function should either be removed or clearly annotated as deliberately inert. At minimum, the `DEBUG_ASSERTCRASH` should be removed or conditioned to prevent this maintenance trap.
   
   How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: e888dbe}

[greptile-apps]

_{3 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[stephanmeesters]

Maybe we can accurately tell if MSAA was forced with some of the changes in #1073 like below, and use that in place of desc.MultiSampleType

static D3DFORMAT _Get_D3D_Back_Buffer_Format() { return DisplayFormat; }
...
WWINLINE unsigned int DX8Wrapper::Bytes_Per_Pixel(D3DFORMAT format)
{
	switch (format)
	{
	// 8-bit formats
	..
	// Depth / stencil
	case D3DFMT_D16_LOCKABLE:
	case D3DFMT_D16:
	case D3DFMT_D15S1:
	case D3DFMT_D24S8:
	case D3DFMT_D24X8:
	case D3DFMT_D24X4S4:
	case D3DFMT_D32:
		return 0;
		...
}

Do you know if the RTT path is much faster when MSAA is off compared to copyRects? Is it worth the extra complexity to support both?

[githubawn]

Maybe we can accurately tell if MSAA was forced with some of the changes in #1073 like below, and use that in place of desc.MultiSampleType

To find this bug I tested a benchmark, but the test ran differently on different vendors, is this needed?
https://github.com/githubawn/GeneralsGameCode/tree/fix/nvidia-microwave-aa-clean/benchmark

Do you know if the RTT path is much faster when MSAA is off compared to copyRects? Is it worth the extra complexity to support both?

I tried benchmarking them, but the game lacks robust benchmarking tools. Using CapFrameX with a DX8 bridge means we’re mostly benchmarking the wrapper’s translation overhead; there’s too much fluctuation between runs to see a real difference between RTT and copyRects. It might be worth keeping both around in case we move to a native DX9 base where RTT isn't broken?

[stephanmeesters]

I tried benchmarking them, but the game lacks robust benchmarking tools. Using CapFrameX with a DX8 bridge means we’re mostly benchmarking the wrapper’s translation overhead; there’s too much fluctuation between runs to see a real difference between RTT and copyRects. It might be worth keeping both around in case we move to a native DX9 base where RTT isn't broken?

If you're up for it you could give #2202 a try; add a zone in the smudge code, then do a testrun on a replay or something that's kinda reproduceable. Tracy can compare histograms of zones between two different captures

[githubawn]

I actually tried to set this up by bringing the Tracy profiling (#2202) into this branch to compare them.

Looking at the traces, the overall render zone copy is almost invisible below PassDefault in both pathways. The smudge rendering is incredibly fast either way. I added the detail zones and did a direct comparison between the RTT and CopyRects (smudge copy) pathways.

A true 1:1 benchmark would require reverting to a pre-safety commit, reapplying Tracy, and resolving build conflicts, which I've partially done but found overly time intensive.

Given the minimal frame time impact, I think CopyRects is sufficient here, but if you have ideas for a cleaner benchmark setup, I'm happy to iterate.

These are the results:

[stephanmeesters]

Very cool! Seems that CopyRects is the way to go.

You can inspect the histogram of a zone by going to Statistics and then clicking on your zone which gives an image like this:

By pressing Ctrl + Numpad+ you can increase the max render FPS so that would maybe make the differences easier to see (if any)

[stephanmeesters]

To find this bug I tested a benchmark, but the test ran differently on different vendors, is this needed? https://github.com/githubawn/GeneralsGameCode/tree/fix/nvidia-microwave-aa-clean/benchmark

I will take a look at this soon

[xezon]

I can confirm that it also fixes the Smudge on my machine (AMD Video Card). Very nice.

I did a very basic performance comparison between before and after fix with 35 Microwave tanks in an otherwise empty scene.

Before: ~230 FPS
After: ~230 FPS

I think we need to not worry about performance for now. The Microwave tank is also not a unit that is spammed much.

[xezon]

Title needs updating because it is not Nvidia specific bug.

[greptile-apps]

SAFE_RELEASE macro is moved from its local definition in W3DWater.cpp to the global WWCommon.h header without a redefinition guard. If any other header file defines SAFE_RELEASE before this one is processed, it will trigger a macro-redefinition warning or error (especially with -Werror). Add a guard:

Suggested change

	#define SAFE_RELEASE(p) { if(p) { (p)->Release(); (p)=nullptr; } }
	#ifndef SAFE_RELEASE
	#define SAFE_RELEASE(p) { if(p) { (p)->Release(); (p)=nullptr; } }
	#endif

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/Libraries/Source/WWVegas/WWLib/WWCommon.h
Line: 26

Comment:
`SAFE_RELEASE` macro is moved from its local definition in `W3DWater.cpp` to the global `WWCommon.h` header without a redefinition guard. If any other header file defines `SAFE_RELEASE` before this one is processed, it will trigger a macro-redefinition warning or error (especially with `-Werror`). Add a guard:

```suggestion
#ifndef SAFE_RELEASE
#define SAFE_RELEASE(p) { if(p) { (p)->Release(); (p)=nullptr; } }
#endif
```

How can I resolve this? If you propose a fix, please make it concise.