bugfix(render2d): Flush batched 2D draws before they overflow the dynamic buffers

[tintinhamans]

• Follow up for perf(gui): Significantly reduce cost of 2d render elements #2514

Render2DClass::Render passes Indices.Count() to DynamicIBAccessClass, which takes an unsigned short. 65538 indices become 2, the index buffer is locked for 2, and the copy loop writes all 65538.

Before batching, W3DDisplay called Render after every draw call. With batching, setup2DRenderState previously only flushed when the texture, mode or grayscale changed, so identical draws could accumulate beyond the dynamic buffer limits.

Drawable::drawAmmo hits this: it draws one pip image per round in the clip, and ClipSize is a plain INI::parseInt. A weapon with ShowsAmmoPips and a clip of 10923 produces 65538 indices, and selecting or hovering the unit can write up to 128 KB past the buffer.

DynamicVBAccessClass and DynamicIBAccessClass now define MAX_VERTEX_COUNT and MAX_INDEX_COUNT. Render2DClass::Can_Add checks whether the batch has room for the requested vertices and indices.

Each draw supplies its actual geometry counts to setup2DRenderState, including rotated images and each clock branch. The batch flushes before the next draw would exceed either limit, rather than at half capacity.

Render keeps an assertion to flag an oversized batch in diagnostic builds and a runtime guard to prevent an unsafe buffer copy.

The similar narrowing in drawRiverWater is handled separately.

[coderabbitai]

Note

Reviews paused

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.

• Last reviewed commit: 6b298b2

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d9c703bf-2840-46ee-a756-2cebb099bd5f

📥 Commits

Reviewing files that changed from the base of the PR and between 1714002 and 6b298b2.

📒 Files selected for processing: 2

• Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DDisplay.h
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Walkthrough

The Generals and GeneralsMD 2D renderers now check vertex and index counts against dynamic-buffer limits. W3DDisplay passes expected draw counts when it decides whether to reuse or flush a batch.

Changes

2D renderer capacity checks

Layer / File(s)	Summary
Capacity contract and render guard Core/Libraries/Source/WWVegas/WW3D2/dx8indexbuffer.h, Core/Libraries/Source/WWVegas/WW3D2/dx8vertexbuffer.h, Generals/Code/Libraries/Source/WWVegas/WW3D2/render2d.h, Generals/Code/Libraries/Source/WWVegas/WW3D2/render2d.cpp, GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.h, GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.cpp	The buffer classes define 65,535-count limits. Both Render2DClass variants add Can_Add and assert and return without rendering when existing geometry exceeds the limits.
Batch capacity check Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DDisplay.h, Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp	setup2DRenderState receives expected vertex and index counts. It reuses the current batch only when capacity and render state match; otherwise, it flushes and reinitializes the batch. Draw calls provide counts for their geometry, including branch-dependent clock geometry and rotated images.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: stephanmeesters

Merge Risk: ⚪ Minimal · up to 6b298

The capacity checks address the reported 2D batching overflow. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
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.
Title check		The title clearly and concisely describes the main change: flushing batched 2D draws before dynamic-buffer overflow.
Description check		The description directly explains the buffer-overflow bug, the affected code paths, and the implemented safeguards.

• 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues were found.

Summary

The PR flushes batched 2D draws before they exceed the dynamic buffers’ count limits.

• The 2D display flushes a batch before the next draw would exceed its limits.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Draw[Next 2D draw] --> Check{Same drawing state and enough room?}
    Check -->|Yes| Add[Add geometry]
    Check -->|No| Flush[Render and clear current batch]
    Flush --> State[Set drawing state]
    State --> Add
    Add --> Next[Continue batch]

Loading

_{Reviews (4) · Last reviewed commit: "fix(render2d): flush batched draws befor..." · Reviewed by Greptile}

[Mauller]

LGTM

[xezon]

Is this meant to be an assert or runtime condition? This now implies that it cannot hit on runtime but also that it can hit on runtime. Which one is true?

[tintinhamans]

The assert flags a broken capacity assumption. The runtime guard prevents an unsafe buffer copy if it still happens.