bugfix(w3dwater): Skip rivers that overflow the dynamic index buffer

[tintinhamans]

• Follow up for bugfix(render2d): Flush batched 2D draws before they overflow the dynamic buffers #3487

drawRiverWater asks DynamicIBAccessClass for three indices per river point, and the count is an unsigned short. 21846 points wrap to 2, and the loop still writes 65532 indices.

The point count comes straight from the map file, so one river polygon is enough to write up to 128 KB past the index buffer.

Rivers that do not fit in 16 bits are now skipped. The largest river in the 537 retail and custom maps I tested has 374 points.

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

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: fb82e607-6a81-48d6-b1ab-94eea96b93f9

📥 Commits

Reviewing files that changed from the base of the PR and between cbfb063 and ac29be1.

📒 Files selected for processing: 1

• Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.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 river water draw path now checks the computed index count against the dynamic index buffer limit before allocation. If the count exceeds 65,535, it asserts and returns without drawing. Rivers within the limit continue through the existing rendering path.

Changes

River water rendering

Layer / File(s)	Summary
Check river index count Core/Libraries/Source/WWVegas/WW3D2/dx8indexbuffer.h, Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp	DynamicIBAccessClass defines MAX_INDEX_COUNT as 0xffff. drawRiverWater asserts and returns when the computed index count exceeds that limit, before setting m_drawingRiver or allocating the dynamic index buffer.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: mauller, stephanmeesters

Merge Risk: ⚪ Minimal · up to ac29b

Rivers exceeding the 16-bit index limit are skipped before allocation, and accepted rivers fit the allocated buffer. No material merge-blocking risk from this change remains.

Pre-merge checks: 4

Passed checks: 4 passed

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.
Description check		The description clearly explains the river index-count overflow, the resulting buffer overrun risk, and the behavior change that skips oversized rivers.
Title check		The title clearly and concisely describes the main change: skipping rivers that exceed the dynamic index-buffer limit.

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

Summary

The PR skips rivers whose index count exceeds the dynamic buffer’s 16-bit limit.

• Rivers that exceed the index limit are skipped before drawing.

_{Reviews (5) · Last reviewed commit: "bugfix(w3d): Skip rivers that overflow t..." · Reviewed by Greptile}

[stephanmeesters]

Looks like the river rendering is super inefficient right now, it puts the entire maps river's polygons into the buffer instead of culling to just the part that's on screen.

Made an item for that: #3496

[Mauller]

looks good

[xezon]

Why is this labeled as "ThisProject" ?

[tintinhamans]

Why is this labeled as "ThisProject" ?

My bad, removed the label - had this one open when I was labeling the one for fixing 2d batching issue I think.