Skip to content

unify(ww3d2): Merge SortingRenderer and move to Core#2772

Merged
xezon merged 2 commits into
TheSuperHackers:mainfrom
stephanmeesters:unify/sorting-renderer
Jun 7, 2026
Merged

unify(ww3d2): Merge SortingRenderer and move to Core#2772
xezon merged 2 commits into
TheSuperHackers:mainfrom
stephanmeesters:unify/sorting-renderer

Conversation

@stephanmeesters
Copy link
Copy Markdown

Merge by rebase

Added to Generals

A refactor that moves separate arrays to a struct, as well as performance and memory improvements.

In detail:

  • Merged z depth into TempIndexStruct — removed separate polygon_z_array, node_id_array, polygon_index_array and their allocators; z, node ID, and triangle indices now live together in one struct
  • Added comparison operators (<, <=, >, >=, ==) to TempIndexStruct keyed on z
  • Replaced generic template sort (QuickSort/InsertionSort/Sort<T,K>) with a specialized sort on TempIndexStruct* using median-of-three partitioning and sorting smaller subarray first
  • Optimized vertex copy — uses single memcpy instead of per-vertex loop with matrix transform
  • Fast path for particle systems — when the world-view matrix is effectively identity apart from z-offset, skips full z computation and uses raw vertex z
  • Added NaN guards (_isnan/_finite) on computed triangle z-depth with debug assert
  • Wrapped Flush_Sorting_Pool with _Enable_Triangle_Draw to respect the enable/disable state
  • Removed unused ShortVectorIStruct constructors (default and parameterized)

Sorting renderer is involved with rendering particles in the right order, so I did this test with a bunch of particles (alpha and additive), they looked the same before and after so I think we're good:

Screenshot From 2026-06-06 19-44-15

@stephanmeesters stephanmeesters changed the title Unify/sorting renderer unify(ww3d2): Merge SortingRenderer and move to Core Jun 6, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 6, 2026

Greptile Summary

This PR unifies the sorting renderer by promoting the improved GeneralsMD implementation to Core, removing separate Generals and GeneralsMD copies, and updating all three CMakeLists.txt files accordingly. The Core version carries several refactors on top of the original: z-depth is merged into TempIndexStruct (eliminating several separate arrays and their allocators), the sort routine is replaced with a median-of-three quicksort, vertex copying switches to a single memcpy, a fast path is added for particle systems with an identity-like world-view matrix, and Flush_Sorting_Pool is now wrapped by _EnableTriangleDraw.

  • Struct consolidation: z, node-ID, and triangle indices now live together in TempIndexStruct; polygon_z_array, node_id_array, polygon_index_array, and vertex_z_array (and their separate allocators) are all gone, reducing both memory overhead and allocator bookkeeping.
  • Sort replacement: The generic two-array QuickSort/InsertionSort template pair is replaced by a self-contained median-of-three sort on TempIndexStruct* that avoids the extra key array and sorts the smaller sub-partition first to bound stack depth.
  • Guarding and fast path: Flush_Sorting_Pool is now gated by _EnableTriangleDraw; the identity-matrix fast path skips the full per-vertex z projection for particle systems where the world-view matrix is effectively identity.

Confidence Score: 4/5

The change is safe to merge; a pre-existing inconsistency in draw-call suppression (else branch not gated by _EnableTriangleDraw) was already flagged in the previous review round and is unchanged here.

The z-computation refactor is mathematically equivalent to the original (the transpose-then-row-access from before matches the column-access used now), the memcpy vertex copy is a straightforward size-times-sizeof replacement, and the new sort has been manually audited as correct. The only unresolved issue from a prior review — Draw_Triangles in the non-pooled else branch remaining ungated by _EnableTriangleDraw — is still present in the diff.

Core/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp — specifically the Flush() else branch around line 617 that calls Draw_Triangles without checking _EnableTriangleDraw.

Important Files Changed

Filename Overview
Core/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp Moved from GeneralsMD and refactored: TempIndexStruct now carries z, specialized Sort with median-of-three, fast particle path, memcpy-based vertex copy, and _EnableTriangleDraw wrapping around Flush_Sorting_Pool — the else branch that calls Draw_Triangles directly is still unguarded (pre-existing comment).
Core/Libraries/Source/WWVegas/WW3D2/sortingrenderer.h Moved from GeneralsMD to Core unchanged; public interface is identical to both Generals and GeneralsMD versions.
Core/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt Uncomments sortingrenderer.cpp and sortingrenderer.h so Core now builds both files.
Generals/Code/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt Comments out sortingrenderer entries so Generals no longer builds its own copy.
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt Comments out sortingrenderer entries so GeneralsMD no longer builds its own copy.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["SortingRendererClass::Flush()"] --> B["while sorted_list not empty"]
    B --> C{"sorting buffer\ntype?"}
    C -- "SORTING / DYNAMIC_SORTING" --> D["Insert_To_Sorting_Pool(state)"]
    C -- "other" --> E["DX8Wrapper::Draw_Triangles (unguarded)"]
    D --> F["overlapping_nodes[]\noverlapping counts updated"]
    B -- "list empty" --> G["Save old _EnableTriangleDraw\nSet _EnableTriangleDraw"]
    G --> H["Flush_Sorting_Pool()"]
    H --> I["Get_Temp_Index_Array()\nmemcpy vertices to dynamic VB"]
    I --> J{"identity-like\nworld-view?"}
    J -- "yes (fast path)" --> K["tis.z = raw vertex z avg"]
    J -- "no" --> L["tis.z = full matrix transform avg"]
    K --> M["Sort(tis, tis + count)\nmedian-of-three quicksort"]
    L --> M
    M --> N["Fill dynamic IB\nApply_Render_State per group\nDraw_Triangles"]
    N --> O["Release refs / clean list"]
    O --> P["Restore old _EnableTriangleDraw"]
    H --> P
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp:135
**`if` body on same line as condition**

The body of this `if` is on the same line as the condition. Per the codebase convention, statement bodies should go on the next line to allow precise debugger breakpoint placement. Prefer:

```
if (right < left)
    break;
```

Reviews (3): Last reviewed commit: "unify(ww3d2): Move SortingRenderer to Co..." | Re-trigger Greptile

@xezon xezon added Gen Relates to Generals Unify Unifies code between Generals and Zero Hour labels Jun 7, 2026
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bot has a complain about something. Commit titles need pull ids.

@stephanmeesters
Copy link
Copy Markdown
Author

   Callers that invoke `SortingRendererClass::_Enable_Triangle_Draw(false)` expecting to suppress all sorting-renderer draws will still see non-pooled triangles rendered.

This is fine, _EnableTriangleDraw is meant to debug the sorting renderer, so it makes sense that it does not affect things that bypass the sorting renderer.

@greptileai

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 7, 2026

Fair point — if it's a debug tool scoped to the sorting renderer's pooled path, that behavior is correct by design. I'll withdraw that concern.

@stephanmeesters stephanmeesters force-pushed the unify/sorting-renderer branch from a660cf2 to f898980 Compare June 7, 2026 09:24
@xezon xezon merged commit a1358a9 into TheSuperHackers:main Jun 7, 2026
32 of 33 checks passed
xezon pushed a commit that referenced this pull request Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals Unify Unifies code between Generals and Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants