Skip to content

perf: reduce water nuke terrain update spikes - #5011

Open
Aotumuri wants to merge 1 commit into
mainfrom
perf-water-nuke-terrain-deltas
Open

perf: reduce water nuke terrain update spikes#5011
Aotumuri wants to merge 1 commit into
mainfrom
perf-water-nuke-terrain-deltas

Conversation

@Aotumuri

Copy link
Copy Markdown
Member

Description

Water nukes could cause noticeable frame and simulation spikes when large terrain deltas were applied.

This PR reduces those spikes in two ways:

  • Coalesces adjacent terrain row spans into bounded-overdraw rectangles before uploading them to WebGL.
  • Prevents terrain finalization and water-path graph rebuilding from running in the same game tick.

Previously, each changed row produced a separate texSubImage2D call for each of the three terrain textures. Compact crater-shaped deltas can now be uploaded as a single rectangle. Unchanged texels inside a merged rectangle are uploaded using their current terrain value, so the rendered result remains unchanged.

Rectangles are only merged when the additional upload area is bounded:

  • merged area is at most 1.5× the original span area; or
  • the merge adds at most 4,096 texels.

Performance

Synthetic WebGL2 benchmark using Giant World Map-sized textures (4108×1948). The benchmark mirrors the production terrain-delta path: RGBA terrain encoding followed by uploads to the terrain, railroad, and shared terrain-byte textures.

Each result is the median of 11 samples, with 50 uploads per sample. Results were reproduced across three browser runs.

Scenario GL calls before → after Upload size before → after Median before → after Speedup
Atom bomb 183 → 3 2.8 KB → 3.7 KB 0.84ms → 0.014–0.016ms ~56×
Hydrogen bomb 603 → 3 31.4 KB → 40.4 KB 2.76ms → 0.112–0.116ms ~24×
5× atom barrage 915 → 15 14.1 KB → 18.6 KB 5.03–5.14ms → 0.088–0.094ms ~56×
40× MIRV 555 → 15 626.6 KB → 628.4 KB 2.18–2.19ms → 1.43ms ~1.53×

For a hydrogen-bomb-shaped delta, the number of WebGL upload calls is reduced by 99.5%, from 603 to 3.

On Giant World Map, separating terrain finalization from water-graph rebuilding also reduced the measured worst water-nuke simulation tick:

Before After Improvement
80.8ms 47.3ms ~41% lower peak

Correctness

  • Merged rectangles contain the current terrain byte for every texel, including unchanged gap tiles.
  • Excessively sparse adjacent rows are kept separate to avoid unbounded overdraw.
  • Water-graph rebuilding is delayed by only one tick when terrain conversion occurs. The existing rebuild throttle already permits the graph to remain stale between rebuilds.

Tests

  • Added coverage for compact multi-row rectangle merging.
  • Added coverage ensuring sparse rows are not merged when overdraw is excessive.
  • Added coverage ensuring water-graph rebuilding occurs on the tick after terrain conversion.
  • npx vitest run tests/client/render/frame/derive/terrain-row-spans.test.ts tests/nukes/WaterNukes.test.ts
  • npx tsc --noEmit

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

aotumuri

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 545d26a6-32e8-4090-81e5-03c1a628e726

📥 Commits

Reviewing files that changed from the base of the PR and between b6c194e and ed085d1.

📒 Files selected for processing (4)
  • src/client/render/frame/derive/TerrainRowSpans.ts
  • src/core/game/WaterManager.ts
  • tests/client/render/frame/derive/terrain-row-spans.test.ts
  • tests/nukes/WaterNukes.test.ts

Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.


Walkthrough

Terrain row spans now merge into bounded rectangles and include unchanged gap tiles in uploads. Water graph rebuilding is deferred for one tick after water terrain conversions.

Changes

Terrain and water update behavior

Layer / File(s) Summary
Bounded terrain rectangle batching
src/client/render/frame/derive/TerrainRowSpans.ts, tests/client/render/frame/derive/terrain-row-spans.test.ts
Adjacent rows merge when overdraw and extra-texel limits allow it. Rectangle buffers include unchanged tiles. Tests cover merged 3×3 rectangles and sparse rows.
Deferred water graph rebuilding
src/core/game/WaterManager.ts, tests/nukes/WaterNukes.test.ts
WaterManager.tick records terrain conversions and skips graph rebuilding during that tick. The test verifies rebuilding on the following tick.

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

Merge Risk: ⚪ Minimal · up to ed085

This PR reduces terrain-update and water-simulation spikes while preserving terrain results and adding focused coverage; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

Suggested labels: small-fix

Suggested reviewers: evanpelle, flopinguin

Sequence Diagram(s)

sequenceDiagram
  participant WaterManager
  participant TerrainConversionQueue
  participant WaterGraph
  WaterManager->>TerrainConversionQueue: Process pending conversions
  TerrainConversionQueue-->>WaterManager: Report conversion occurred
  WaterManager-->>WaterGraph: Skip rebuild on conversion tick
  WaterManager->>WaterGraph: Rebuild on next eligible tick
Loading

Poem

Rows join hands in tiled arrays,
Gap bytes travel through the squares.
Water waits, then graphs renew,
One quiet tick before they do.
Bounded shapes keep uploads bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main performance improvement for water nuke terrain updates.
Description check ✅ Passed The description directly explains the terrain upload and water-graph changes, performance results, correctness, and tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

2 participants