feat(rendering): Add support for multi sample anti-aliasing (MSAA)#2454
Conversation
0df6acc to
ad46196
Compare
|
Had to push after initial as i accidentally left 4X MSAA enabled by default from my testing. |
|
| Filename | Overview |
|---|---|
| Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp | Adds MultiSampleAntiAliasing static member and wires it into Set_Render_Device: hardware capability is checked for both back-buffer and depth-stencil formats before applying the requested MSAA level, with a silent fallback to NONE. The D3DFMT_UNKNOWN depth-stencil fallback is now resolved before the capability check. Minor column-alignment inconsistency in the static member definition. |
| Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h | Exposes Set_MSAA_Mode / Get_MSAA_Mode as protected inline methods and declares the MultiSampleAntiAliasing static member. Minor tab-alignment issue on the new member declaration relative to the surrounding block. |
| Generals/Code/Libraries/Source/WWVegas/WW3D2/ww3d.cpp | Implements WW3D::Set_MSAA_Mode and WW3D::Get_MSAA_Mode as thin mapping layers between the public MultiSampleModeEnum and the DirectX D3DMULTISAMPLE_TYPE enum. Both switch statements now correctly use lowercase default: and handle all four exposed modes. |
| Generals/Code/Libraries/Source/WWVegas/WW3D2/ww3d.h | Adds the MultiSampleModeEnum enum (NONE / 2X / 4X / 8X) and declares the two new public static methods on WW3D. Clean, no issues. |
| Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Adds a single call to WW3D::Set_MSAA_Mode(WW3D::MULTISAMPLE_MODE_NONE) immediately before Set_Render_Device, preserving the retail default of no MSAA. Straightforward and correct. |
| GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp | Mirror of the Generals change. Same capability-check logic, same D3DFMT_UNKNOWN pre-resolution, same silent fallback to NONE. Same minor column-alignment inconsistency on the static member definition. |
| GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h | Mirror of the Generals header change. Same alignment issue on the new D3DMULTISAMPLE_TYPE member declaration. |
| GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/ww3d.cpp | Mirror of the Generals ww3d.cpp change. Correct default: keyword in both switch statements. |
| GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/ww3d.h | Mirror of the Generals ww3d.h change. Clean, no issues. |
| GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Mirror of the Generals W3DDisplay.cpp change. Same pre-device-creation MSAA default call. Correct and clean. |
Sequence Diagram
sequenceDiagram
participant Caller as W3DDisplay::init()
participant WW3D
participant DX8Wrapper
participant D3D8 as IDirect3D8
Caller->>WW3D: Set_MSAA_Mode(MULTISAMPLE_MODE_NONE)
WW3D->>DX8Wrapper: Set_MSAA_Mode(D3DMULTISAMPLE_NONE)
Note over DX8Wrapper: MultiSampleAntiAliasing = D3DMULTISAMPLE_NONE
Caller->>WW3D: Set_Render_Device(...)
WW3D->>DX8Wrapper: Set_Render_Device(...)
Note over DX8Wrapper: Resolve back-buffer & depth-stencil formats
Note over DX8Wrapper: Resolve D3DFMT_UNKNOWN depth fallback
alt MultiSampleAntiAliasing > NONE
DX8Wrapper->>D3D8: CheckDeviceMultiSampleType(BackBufferFormat)
D3D8-->>DX8Wrapper: hrBack
DX8Wrapper->>D3D8: CheckDeviceMultiSampleType(DepthStencilFormat)
D3D8-->>DX8Wrapper: hrDepth
alt FAILED(hrBack) || FAILED(hrDepth)
Note over DX8Wrapper: MultiSampleAntiAliasing = D3DMULTISAMPLE_NONE
end
end
Note over DX8Wrapper: _PresentParameters.MultiSampleType = MultiSampleAntiAliasing
DX8Wrapper->>D3D8: CreateDevice(...)
D3D8-->>DX8Wrapper: device
Caller->>WW3D: Get_MSAA_Mode()
WW3D->>DX8Wrapper: Get_MSAA_Mode()
DX8Wrapper-->>WW3D: MultiSampleAntiAliasing
WW3D-->>Caller: MultiSampleModeEnum (effective mode)
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp
Line: 114
Comment:
**Column alignment broken in static member definitions block**
The new `D3DMULTISAMPLE_TYPE` definition uses a single space between the type name and the class-qualified member name, breaking the tab-aligned column structure maintained by all surrounding definitions. Both the member-name column and the `=` column fall out of alignment.
Surrounding lines use tabs so that `DX8Wrapper::` always starts at the same visual column and `=` signs line up on the right. The new line should follow the same pattern:
```suggestion
D3DMULTISAMPLE_TYPE DX8Wrapper::MultiSampleAntiAliasing = DEFAULT_MSAA;
```
(Adjust the number of tabs to match the existing tab stops used by `D3DFORMAT` and the `int`/`bool` lines above.)
The same misalignment exists in `GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp` line 118.
**Rule Used:** Align columns in data-oriented blocks (like lookup... ([source](https://app.greptile.com/review/custom-context?memory=e61e83cf-a14f-4759-bed8-3bab008b25b5))
**Learnt From**
[TheSuperHackers/GeneralsGameCode#2067](https://github.com/TheSuperHackers/GeneralsGameCode/pull/2067#discussion_r2704192059)
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h
Line: 634
Comment:
**Column alignment broken in static member declarations block**
`D3DFORMAT` is followed by 5 tabs so that `DisplayFormat` aligns with the other member names in the block. `D3DMULTISAMPLE_TYPE` is followed by only 1 tab, so `MultiSampleAntiAliasing` lands several columns earlier and visually breaks the alignment.
```suggestion
static D3DMULTISAMPLE_TYPE MultiSampleAntiAliasing;
```
(Use enough tabs after `D3DMULTISAMPLE_TYPE` to place `MultiSampleAntiAliasing` in the same column as `DisplayFormat`, `IsWindowed`, etc.)
The same issue is present in `GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h` line 643.
**Rule Used:** Align columns in data-oriented blocks (like lookup... ([source](https://app.greptile.com/review/custom-context?memory=e61e83cf-a14f-4759-bed8-3bab008b25b5))
**Learnt From**
[TheSuperHackers/GeneralsGameCode#2067](https://github.com/TheSuperHackers/GeneralsGameCode/pull/2067#discussion_r2704192059)
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "feat(MSAA): Replicat..."
ad46196 to
f488e92
Compare
|
Some nice catches by greptile that VS completely missed lol |
f488e92 to
2df62c2
Compare
|
Updated implementation to remove 16X support as it is unlikely to be supported and added a check before device creation that checks if the requested MSAA mode is supported before applying it. If the requested MSAA mode is unsupported it then defaults to no MSAA. External code needs to read back the set MSAA mode after device creation to update the settings. |
2df62c2 to
42673ff
Compare
|
Updated to make sure we check the correct window and display formats |
42673ff to
99176b4
Compare
99176b4 to
5f4da8c
Compare
5f4da8c to
581d7f4
Compare
581d7f4 to
f211f9d
Compare
|
Fixed and should be ready to go for human review |
| /* | ||
| ** Check the devices support for the requested MSAA mode then setup the multi sample type | ||
| */ | ||
| if (MultiSampleAntiAliasing > DEFAULT_MSAA) { |
There was a problem hiding this comment.
This line has the expectation that DEFAULT_MSAA = D3DMULTISAMPLE_NONE
Perhaps make it explicit and use MultiSampleAntiAliasing != D3DMULTISAMPLE_NONE
There was a problem hiding this comment.
Thats reasonable, i guess if we altered the default MSAA it would break this.
| // IF we fail then disable MSAA entirely. | ||
| // External code needs to retrieve the configured MSAA mode after device creation | ||
| WWDEBUG_SAY(("Requested MSAA Mode Not Supported")); | ||
| MultiSampleAntiAliasing = DEFAULT_MSAA; |
There was a problem hiding this comment.
MultiSampleAntiAliasing = D3DMULTISAMPLE_NONE
| @@ -761,6 +761,9 @@ void W3DDisplay::init() | |||
| } | |||
| } | |||
|
|
|||
| // TheSuperHackers @feature Mauller 13/03/2026 Add native MSAA support, must be set before creating render device | |||
| WW3D::Set_MSAA_Mode(WW3D::MULTISAMPLE_MODE_NONE); | |||
There was a problem hiding this comment.
This is where #2375 will plug in right? Right now this line does nothing?
There was a problem hiding this comment.
Yes this will be where external code will set the MSAA level, likewise external code also needs to call get_MSAA_Mode after the display is created to check what mode was set etc.
f211f9d to
e22ee2a
Compare
|
Tweaked based on feedback. |
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
Show resolved
Hide resolved
e22ee2a to
8d67d37
Compare
| @@ -761,6 +761,9 @@ void W3DDisplay::init() | |||
| } | |||
| } | |||
|
|
|||
| // TheSuperHackers @feature Mauller 13/03/2026 Add native MSAA support, must be set before creating render device | |||
| WW3D::Set_MSAA_Mode(WW3D::MULTISAMPLE_MODE_NONE); | |||
There was a problem hiding this comment.
That's a Direct3D enumeration within directx wrapper, im going to follow this up soon with the changes needed to link it into options.ini.
i can also add in defaults for MSAA based on the presets too.
Medium 2x. High 4x seem reasonable? leave 8x as a custom option since it can be quite taxing.
|
Replicated to generals, this can just be squash merged. |
This PR adds new interfaces to DX8Wrapper and WW3D to be able to set multi sample anti aliasing. And to be able to get the currently selected mode.
The MSAA mode
MUST!!!be set before the rendering device is created, as it is used as an input when creating the rendering surface.I have left the retail games default setting of no MSAA within w3ddisplay, this can be expanded to enable MSAA support within the game options in a followup.
The modes supported are the most common which have been enumerated within WW3D as the following:
MULTISAMPLE_MODE_NONE
MULTISAMPLE_MODE_2X
MULTISAMPLE_MODE_4X
MULTISAMPLE_MODE_8X
MULTISAMPLE_MODE_16X- Removed 16X as it's unlikely to be supportedThere are more and intermediate modes that are selectable withing directx, but not all graphics adapters may support these. Therefore i chose the most common modes.
Edit - After adding the extra check to test if the requested MSAA mode is supported, calling code needs to check what the currently set MSAA mode is. IF the requested mode is unsupported then it defaults to no anti aliasing.
Directx8 does not support requesting the capability of the devices MSAA modes as an enumeration of all supported modes and requires you to call a function to test if the requested mode works.