You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
perf(particlesys): Improve particle rendering performance using texture atlas - #3557
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
The changes add texture-atlas packing, premultiplied-alpha processing, and per-particle UV mapping for particle rendering. Particle batches use selected shaders and atlas textures. The sorting renderer groups consecutive triangles with compatible render states.
Adds portable, SSE2, and x86 implementations of premultiply_alpha. Shared tests check channel results, alpha preservation, and row padding.
Texture atlas packing and padding Core/Libraries/Source/WWVegas/WW3D2/textureatlas.*, Core/Libraries/Source/WWVegas/WW3D2/texture.*, Core/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt, Tests/Google/Core/Libraries/WWVegas/WW3D2/TextureAtlasTest.cpp, Tests/Google/Core/CMakeLists.txt
Adds TextureAtlasClass page allocation, texture caching, mip-level copying, and padding. Texture accessors gain const support or default mip-level arguments. Tests cover coordinate wrapping, clamping, and padding values.
Particle batching with atlas UVs Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DParticleSys.h, Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp, Core/Libraries/Source/WWVegas/WW3D2/pointgr.*
Particle batches select shaders, resolve atlas textures, store per-particle UV rectangles, and pass those rectangles to PointGroupClass for UV remapping. The particle manager reset also resets the atlas.
Sorting Renderer Batching
Layer / File(s)
Summary
Group draws by compatible render state Core/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp
Adds render-state compatibility checks and groups consecutive compatible nodes within each index-buffer chunk into a draw using the first node’s render state.
sequenceDiagram
participant W3DParticleSystemManager
participant TextureAtlasClass
participant PointGroupClass
W3DParticleSystemManager->>TextureAtlasClass: Find_Or_Insert particle texture and receive atlas UV rectangle
W3DParticleSystemManager->>PointGroupClass: Set_Texture_UV_Rects for particle rendering
PointGroupClass->>PointGroupClass: Remap generated UVs during Render
LoadingMerge Risk: Low · up to 34300
The change is mergeable. On devices that cannot create the atlas page, texture insertion may repeatedly allocate a large texture. This costs performance but does not break behavior. Consider caching the incompatibility result as a follow-up.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title clearly identifies the main change: improving particle rendering performance through a texture atlas.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description directly matches the changes. It covers texture-atlas integration, sorting-renderer batching, ParticleSys shared blending, and the reported performance improvement.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash# Check where Int/UnsignedInt come from and whether the deps test target gets them via PCH/forced include.
rg -nP '\btypedef\b.*\b(UnsignedInt|Int)\s*;' -g '*.h'
fd CMakeLists.txt Tests/Google --exec sh -c 'echo "== $1"; cat "$1"' _ {}
rg -n 'precompile_headers|FI|/FI|-include' -g 'CMakeLists.txt' -g '*.cmake'
Repository: TheSuperHackers/GeneralsGameCode
Length of output: 19476
🏁 Script executed:
set -o pipefail
printf'%s\n''== Dependencies/Utility/CMakeLists.txt =='
sed -n '1,220p' Dependencies/Utility/CMakeLists.txt
printf'%s\n''== Dependencies/Precompiled/CMakeLists.txt =='
sed -n '1,140p' Dependencies/Precompiled/CMakeLists.txt
printf'%s\n''== Dependencies/Precompiled/precompiled.h =='
sed -n '1,220p' Dependencies/Precompiled/precompiled.h
printf'%s\n''== BaseTypes include references =='
rg -n -F --glob '*.h' -e 'BaseTypes.h' Dependencies Core/Libraries/Source/Lib/Include/Precompiled | head -80
printf'%s\n''== premultiply test includes =='forfin Tests/Google/Dependencies/Utility/premultiply_alpha_common.h Tests/Google/Dependencies/Utility/premultiply_alpha_portable_test.cpp Tests/Google/Dependencies/Utility/premultiply_alpha_sse2_test.cpp Tests/Google/Dependencies/Utility/premultiply_alpha_x86asm_test.cpp;doecho"== $f =="
sed -n '1,100p'"$f"done
Repository: TheSuperHackers/GeneralsGameCode
Length of output: 8953
🏁 Script executed:
set -o pipefail
printf'%s\n''== depsi_always and deps_precompiled definitions =='
rg -n -F --glob 'CMakeLists.txt' --glob '*.cmake' -e 'depsi_always' -e 'deps_precompiled'.printf'%s\n''== candidate precompiled sources =='
rg --files Dependencies/Precompiled | sort
printf'%s\n''== forced header =='
sed -n '1,240p' Dependencies/Precompiled/Precompiled/precompiled.h
printf'%s\n''== type declarations in dependency precompiled sources =='
rg -n -F --glob '*.h' -e 'typedef int Int' -e 'typedef uint32_t UnsignedInt' -e 'BaseTypes.h' -e 'using Int' -e 'using UnsignedInt' Dependencies/Precompiled Dependencies
Repository: TheSuperHackers/GeneralsGameCode
Length of output: 3479
🏁 Script executed:
set -o pipefail
printf'%s\n''== Dependencies/Precompiled/Precompiled/CppTypes.h =='
sed -n '1,240p' Dependencies/Precompiled/Precompiled/CppTypes.h
printf'%s\n''== declarations of Int and UnsignedInt in dependency and test sources =='
rg -n -P --glob '*.{h,hpp,inl,cpp,cxx}''\b(typedef|using|#define)\b[^;\n]*(Int|UnsignedInt)|\b(Int|UnsignedInt)\s+(Int|UnsignedInt)\b' Dependencies Tests/Google
printf'%s\n''== direct type references around dependency test build =='
sed -n '1,100p' Dependencies/CMakeLists.txt
sed -n '1,100p' Tests/Google/Dependencies/CMakeLists.txt
Repository: TheSuperHackers/GeneralsGameCode
Length of output: 3232
Use types available to the test target.
The dependency precompiled header does not define the engine typedefs Int and UnsignedInt. Replace them with int and unsigned throughout this helper.
🐛 Suggested fix
-inline void testPremultiplyAlpha(void (*premultiply)(unsigned char*, Int, Int, Int))+inline void testPremultiplyAlpha(void (*premultiply)(unsigned char*, int, int, int))
{
- constexpr Int width = 259;- constexpr Int stride = width + 5;- constexpr Int height = 256;- constexpr UnsignedInt untouched = 0x80FF8000u;- std::vector<UnsignedInt> pixels(stride * height, untouched);- for (Int y = 0; y < height; ++y)+ constexpr int width = 259;+ constexpr int stride = width + 5;+ constexpr int height = 256;+ constexpr unsigned untouched = 0x80FF8000u;+ std::vector<unsigned> pixels(stride * height, untouched);+ for (int y = 0; y < height; ++y)
{
- const UnsignedInt alpha = y;- UnsignedInt* row = &pixels[y * stride];- for (UnsignedInt color = 0; color < 256; ++color)+ const unsigned alpha = y;+ unsigned* row = &pixels[y * stride];+ for (unsigned color = 0; color < 256; ++color)
@@
- const std::vector<UnsignedInt> source = pixels;+ const std::vector<unsigned> source = pixels;
@@
- Int mismatches = 0;- for (Int i = 0; i < stride * height; ++i)+ int mismatches = 0;+ for (int i = 0; i < stride * height; ++i)
{
- UnsignedInt expected = source[i];+ unsigned expected = source[i];
@@
- const UnsignedInt alpha = source[i] >> 24;+ const unsigned alpha = source[i] >> 24;
@@
- for (UnsignedInt shift = 0; shift < 24; shift += 8)+ for (unsigned shift = 0; shift < 24; shift += 8)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
DirectX 8 D3DXCreateTexture behavior when texture width or height exceeds MaxTextureWidth or non-power-of-two textures are unsupported; does it fail, round dimensions, or assert?
Result:
<source_evidence>
<source kind="search">
<title>D3DXCreateTexture</title>
<location>https://library.thedatadungeon.com/directx81-c/directx_cpp/Graphics/Reference/CPP/D3DX/Functions/Texture/D3DXCreateTexture.content.htm</location>
<excerpt>citeturn1search0 [wordlim: 200] Crawled: 1.8 years ago; Microsoft DirectX 8.1 (C++) ... Therefore, calls to D3DXCreateTexture will often succeed where calls to IDirect3DDevice8::CreateTexture would fail. ... If only Height or Width is set to D3DX_DEFAULT and the other parameter is set to a numeric value, the texture is square, with the height and width of the texture equal to the numeric value.
Microsoft DirectX 8.1 (C++)
# D3DXCreateTexture
Creates an empty texture, adjusting the calling parameters as needed.
HRESULT D3DXCreateTexture(
LPDIRECT3DDEVICE8 pDevice,
UINT Width,
UINT Height,
UINT MipLevels,
DWORD Usage,
D3DFORMAT Format,
D3DPOOL Pool,
LPDIRECT3DTEXTURE8* ppTexture
);
#### Parameters
pDevice
[in] Pointer to an IDirect3DDevice8 interface, representing the device to be associated with the texture.
Width
[in] Width in pixels. If this value is 0, then a value of 1 is used. See Remarks.
Height
[in] Height in pixels. If this value is 0, then a value of 1 is used. See Remarks.
MipLevels
[in] Number of mip levels requested. If this value is zero or D3DX_DEFAULT, a complete mipmap chain is created.
Usage
[in] 0, D3DUSAGE_RENDERTARGET, or D3DUSAGE_DYNAMIC. Setting this flag to D3DUSAGE_RENDERTARGET indicates that the surface is to be used as a render target. The resource can then be passed to the pNewRenderTarget parameter of the SetRenderTarget method. If either D3DUSAGE_RENDERTARGET or D3DUSAGE_DYNAMNIC is specified, the application should check that the device supports this operation by calling IDirect3D8::CheckDeviceFormat. D3DUSAGE_DYNAMIC indicates that the surface should be handled dynamically. For more information about using dynamic textures, see Using Dynamic Textures.
Format
[in] Member of the D3DFORMAT enumerated type, describing the requested pixel format for the texture. The returned texture might have a different format from that specified by Format. Applications should check the format of the returned texture.
Pool
[in] Member of the D3DPOOL enumerated type, describing the memory class into which the texture should be placed.
ppTexture
[out] Address of a pointer to an IDirect3DTexture8 interface, representing the created texture object.
#### Return Values
If the function succeeds, the return value is D3D_OK.
If the function fails, the return value can be one of the following values.
D3DERR_NOTAVAILABLE
D3DERR_OUTOFVIDEOMEMORY
D3DERR_INVALIDCALL
E_OUTOFMEMORY
#### Remarks
Internally, D3DXCreateTexture uses D3DXCheckTextureRequirements to adjust the calling parameters. Therefore, calls to D3DXCreateTexture will often succeed where calls to IDirect3DDevice8::CreateTexture would fail.
If both Height and Width are set to D3DX_DEFAULT, a value of 256 is used for both parameters. If only Height or Width is set to D3DX_DEFAULT and the other parameter is set to a numeric value, the texture is square, with the height and width of the texture equal to the numeric value.
#### Requirements
Header: Declared in D3dx8tex.h.
Import Library: Use D3dx8.lib.</excerpt>
</source>
<source kind="search">
<title>D3DXCreateTexture function (D3dx9tex.h) - Win32 apps | Microsoft Learn</title>
<location>https://learn.microsoft.com/en-us/windows/win32/direct3d9/d3dxcreatetexture</location>
<excerpt>citeturn0search0 [wordlim: 200] Crawled: 2 weeks ago; # D3DXCreateTexture function ... If both Height and Width are set to D3DX_DEFAULT, a value of 256 is used for both parameters.
</excerpt>
</source>
<source kind="search">
<title>DirectX C++</title>
<location>https://documentation.help/directx8_c/documentation.pdf</location>
<excerpt>citeturn0search12 [wordlim: 200] Published: 8.7 years ago; D3DPTEXTURECAPS_CUBEMAP_POW2 ... A texture that is not a power of two cannot be set at a stage that will be read based on a shader computation (such as the bem, beml, or texm3x3 instructions in pixel shaders versions 1.0 to 1.3). ... This means that a texture with dimensions that are not powers of two cannot be addressed or sampled using texture coordinates computed within the shader.
</excerpt>
</source>
<source kind="search">
<title>D3DCAPS8 | DirectX8 Documentation</title>
<location>https://documentation.help/directx8_c/D3DCAPS8.htm</location>
<excerpt>citeturn0search1 [wordlim: 200] Crawled: 3 months ago; When the D3DCAPS2_NO2DDURING3DSCENE capability is set by the driver, it means that 2-D operations cannot be performed between calls to IDirect3DDevice8::BeginScene and IDirect3DDevice8::EndScene. ... A texture that is not a power of two cannot be set at a stage that will be read based on a shader computation (such as the bem, beml, or texm3x3 instructions in pixel shaders versions 1.0 to 1.3). ... This means that a texture with dimensions that are not powers of two cannot be addressed or sampled using texture coordinates computed within the shader. ... Some hardware reports the cap D3DPTEXTURECAPS_TEXREPEATNOTSCALEDBYSIZE. ... In the DirectX 8.x programmable pipeline, this corresponds to the number of instructions supported by pixel shaders on this particular implementation.
</excerpt>
</source>
<source kind="search">
<title>DirectX-Wrappers/DirectX81SDK/include/d3d8caps.h at master · elishacloud/DirectX-Wrappers · GitHub</title>
<location>https://github.com/elishacloud/DirectX-Wrappers/blob/master/DirectX81SDK/include/d3d8caps.h</location>
<excerpt>citeturn0search2 [wordlim: 200] Crawled: 3 weeks ago; #define D3DPTEXTURECAPS_POW2 0x00000002L /* Power-of-2 texture dimensions are required - applies to non-Cube/Volume textures only.
</excerpt>
</source>
<omitted_sources count="39">Additional source records were omitted from this bounded result.</omitted_sources>
</source_evidence>
Citations:
- 1: https://library.thedatadungeon.com/directx81-c/directx_cpp/Graphics/Reference/CPP/D3DX/Functions/Texture/D3DXCreateTexture.content.htm?utm_source=openai
- 2: https://library.thedatadungeon.com/directx81-c/directx_cpp/Graphics/Reference/CPP/D3DX/Functions/Texture/D3DXCreateTexture.content.htm?utm_source=openai
Avoid repeated allocation after a persistent page incompatibility.
For an eligible uncached texture, Find_Or_Insert can reach Create_Page again on each permitted packing attempt while Pages is empty. D3DXCreateTexture adjusts parameters when needed, and Create_Page rejects the resulting texture when its dimensions, format, or mip count differ. This can allocate and release another large page texture for each new texture.
Record persistent page incompatibility and skip later attempts. Clear that state in Reset(). Do not treat every null texture as permanent, because the wrapper also reports transient out-of-video-memory failures. Propagate the failure reason or use another distinction before suppressing those retries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GenRelates to GeneralsMajorSeverity: Minor < Major < Critical < BlockerPerformanceIs a performance concernZHRelates to Zero Hour
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
stephanmeesters
added
Major
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Adds 77% performance to a busy scene.