tweak(scorch): Increase scorch buffers to 16-bit limit

[stephanmeesters]

Merge by rebase

• Relates to Terrain scorches have tight hardcoded limit #179
• Relates to World Builder Bug: Fix the issue where placed scorch marks disappear after a certain amount is reached #364

This PR upgrades the maximum number of scorches on screen (see table). This is done by bumping the index/vertex buffers capacity to the 16-bit limit. Since the index buffer is the bottleneck, the vertex buffer was resized to the nearest power of two that can safely hold all needed vertices.

Additionally, a fix was added to make scorch writes "atomic" by determining the number of vertices/indices it requires beforehand, otherwise we could see scorches being partially rendered, but this problem only appeared after increasing the buffer capacity.

Performance impact is very small: W3DScorch::updateScorches measures well below 0.1ms with a filled buffer. W3DScorch::updateScorches only runs when the scorches have changed, or if the terrain/lighting has changed.

Max number of scorches rendered, per scorch radius, before and after

Note: 500 is the current cap on the number of scorches.

Radius	Typical use	Grid	Vertices	Indices	Max rendered before	Max after
10	Mine disarming	4x4	16	54	500	500
15	Common combat FX	5x5	25	96	327	500
20	WorldBuilder default	6x6	36	150	227	436
35	Black napalm	9x9	81	384	101	170
50	Large explosions	12x12	144	726	56	90
75	Aurora bomb	17x17	289	1536	28	42
100	Firestorm effects	22x22	484	2646	16	24
250	MOAB/crater effects	52x52	2704	15606	3	4
320	Nuclear effects	66x66	4356	25350	1	2

Increasing capacity further

This PR applied a simple way to increase the capacity. Going for a higher capacity will need 32-bit buffers, or a different approach. To make "unlimited" scorches possible we could in the future switch to something like a baked terrain texture.

Todo

• Add pull IDs to commits
• Clean up commits

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Increase scorch buffers to 16-bit max and preflight per-scorch capacity

✨ Enhancement 🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Increase scorch vertex/index buffer capacities up to 16-bit limits to support more/larger
 scorches.
• Precompute per-scorch vertex/index requirements to avoid partially-written (partially-rendered)
 scorches.

Diagram

graph TD
  A["W3DScorch.drawScorches"] --> B["updateScorches"] --> C["Lock VB/IB"] --> D["writeScorchToBuffer"] --> E{"Fits remaining\nVB/IB space?"}
  E -- "no" --> F["Stop adding\nolder scorches"]
  E -- "yes" --> G["Append verts\n+ indices"] --> H[("DX8 VB/IB")]

  subgraph Legend
    direction LR
    _p["Process"] ~~~ _d{"Decision"} ~~~ _b[("Buffer")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Segment into multiple draw batches (multi-buffer or ring buffer)

• ➕ Avoids a hard stop when the current buffer fills; can draw more scorches by continuing in a second batch
• ➕ Keeps 16-bit indices while scaling total capacity
• ➖ More state changes/draw calls and more complex bookkeeping
• ➖ Must ensure correct blending/ordering between batches

2. Move to 32-bit indices (and larger vertex buffers)

• ➕ Removes 16-bit ceiling; simplest mental model for “just increase limits”
• ➕ Avoids careful sizing trade-offs between VB/IB capacity
• ➖ May not be supported/performant in all DX8-era paths or existing engine abstractions
• ➖ Higher memory bandwidth and potentially higher GPU cost

3. Bake scorches into a terrain overlay/texture

• ➕ Effectively “unlimited” historical scorches without growing dynamic geometry buffers
• ➕ Potentially cheaper at runtime once baked
• ➖ Requires new rendering path and update strategy (streaming tiles, blending rules, LOD)
• ➖ More engineering effort and higher risk of visual regressions

Recommendation: Current approach (maximizing within 16-bit limits + atomic per-scorch writes) is the best near-term fix: minimal engine disruption while increasing capacity and eliminating partial-render artifacts. If future requirements exceed this ceiling, batching is the least invasive next step; 32-bit buffers or texture baking are larger architectural moves.

Files changed (2) +18 / -6

Enhancement (1) +2 / -2

W3DScorch.hRaise scorch VB/IB capacities to 16-bit maximums +2/-2

Raise scorch VB/IB capacities to 16-bit maximums

• Updates scorch buffer sizing constants to use the 16-bit index ceiling (65535 indices) and a larger vertex capacity aligned with that limit. This increases the maximum number of drawable scorches for larger radii without changing the scorch count cap (MAX_SCORCH_MARKS).

Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScorch.h

Bug fix (1) +16 / -4

W3DScorch.cppPreflight vertex/index needs to make scorch writes all-or-nothing +16/-4

Preflight vertex/index needs to make scorch writes all-or-nothing

• Computes required vertex and index counts for a scorch before writing, returning false if it would exceed remaining buffer space. Removes in-loop overflow checks so a scorch can’t be partially written (and partially rendered) when buffers are near capacity.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (1) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Overflow bypasses buffer guard 🐞 Bug ⛨ Security

Description

writeScorchToBuffer computes requiredVertices/requiredIndices using 32-bit Int
multiplication, and then removes the incremental per-write bounds checks; if either multiplication
overflows, the capacity check can be bypassed and the subsequent loops will write past the end of
the locked vertex/index buffers (memory corruption). Map extents come directly from map files
without a hard cap, making oversized extents plausible for malformed/corrupt inputs.

Code

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[R207-210]

+	const Int requiredVertices = vertexCountX * vertexCountY;
+	const Int requiredIndices = 6 * (vertexCountX - 1) * (vertexCountY - 1);
+	if (requiredVertices > MAX_SCORCH_VERTEX - m_curNumScorchVertices ||
+	    requiredIndices > MAX_SCORCH_INDEX - m_curNumScorchIndices)

Evidence

The PR introduces 32-bit products for requiredVertices/requiredIndices and removes the older
per-iteration capacity checks, so an arithmetic overflow can cause the capacity gate to be skipped
and the loops to write unchecked into the fixed-size DX8 buffers. The heightmap extents are read
from file as Int without an explicit clamp, so unusually large extents are feasible for malformed
inputs.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[200-236]
Core/Libraries/Include/Lib/BaseTypeCore.h[114-120]
Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp[850-878]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`requiredVertices` and `requiredIndices` are computed using 32-bit `Int` products. If `vertexCountX`/`vertexCountY` are large enough, these products can overflow and become negative/small, letting the new capacity pre-check pass. Because the PR removed the old per-vertex/per-cell capacity checks, the subsequent write loops can overrun `curVb`/`curIb` and corrupt memory.
## Issue Context
- `Int` is `int32_t`, so overflow is possible.
- `WorldHeightMap` reads `m_width`/`m_height` from the file and uses them as extents with no explicit upper bound.
## Fix Focus Areas
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[200-213]
## Implementation notes
- Compute `requiredVertices` / `requiredIndices` in `Int64` (or `size_t`) and explicitly validate:
- `vertexCountX` and `vertexCountY` are within sane bounds.
- `requiredVertices <= (MAX_SCORCH_VERTEX - m_curNumScorchVertices)` and same for indices.
- guard intermediate multiplications (e.g., `(Int64)vertexCountX * vertexCountY`).
- Consider retaining a defensive per-loop check (even if redundant) to prevent any future arithmetic regression from becoming an OOB write.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Scorch limits still hardcoded 📎 Requirement gap ⚙ Maintainability

Description

MAX_SCORCH_VERTEX and MAX_SCORCH_INDEX are still compile-time constants (now set to 16-bit
maximums), and the write path hard-stops when those constants are reached. This keeps scorch
capacity non-configurable/non-scalable and requires code changes to adjust limits.

Code

Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScorch.h[R71-72]

+		MAX_SCORCH_VERTEX = 65535 / 2,
+		MAX_SCORCH_INDEX = 65535,

Evidence

PR Compliance ID 2 requires scorch buffer limits to be configurable/scalable rather than governed by
hardcoded constants. The PR explicitly sets the limits as fixed constants in W3DScorch.h and
enforces them via hardcoded comparisons in writeScorchToBuffer, meaning capacity is still not
adjustable via configuration/settings.

Terrain scorch buffer limits must not be tightly hardcoded; limits should be configurable/scalable
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScorch.h[69-76]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[200-213]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Terrain scorch vertex/index limits remain hardcoded (`MAX_SCORCH_VERTEX`, `MAX_SCORCH_INDEX`), so increasing/scaling capacity still requires a code change.
## Issue Context
PR Compliance ID 2 requires scorch limits to be configurable and/or scalable by settings rather than fixed constants.
## Fix Focus Areas
- Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScorch.h[69-76]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[200-213]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Oversized scorch drops all 🐞 Bug ≡ Correctness

Description

If a single scorch requires more vertices/indices than the buffers allow, the new pre-check returns
false before writing anything, and updateScorches() immediately aborts rendering of all
remaining scorches even if many would fit. This can cause “no scorches render” when the newest
scorch is oversized.

Code

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[R209-213]

+	if (requiredVertices > MAX_SCORCH_VERTEX - m_curNumScorchVertices ||
+	    requiredIndices > MAX_SCORCH_INDEX - m_curNumScorchIndices)
+	{
+		return false;
+	}

Evidence

The PR adds the early return false on capacity pre-check; updateScorches() treats any false
from writeScorchToBuffer() as a hard stop and returns immediately, so a scorch that can’t fit will
abort further processing (including scorches that would fit).

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[162-170]
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[207-213]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When a scorch can never fit (its `requiredVertices` > `MAX_SCORCH_VERTEX` or `requiredIndices` > `MAX_SCORCH_INDEX`), `writeScorchToBuffer` currently returns `false`, which causes `updateScorches()` to return immediately and render nothing further.
## Issue Context
`updateScorches()` iterates scorches newest-to-oldest and stops on the first `false`. With the new early pre-check, an oversized newest scorch can prevent all rendering (buffer remains empty).
## Fix Focus Areas
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[162-170]
- Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp[207-213]
## Implementation notes
- Differentiate between:
1) “doesn’t fit remaining space” (should stop, return `false`), and
2) “can’t ever fit even in an empty buffer” (should skip scorch and continue, return `true` without modifying counters).
- Concretely: add an early check like:
- if (`requiredVertices > MAX_SCORCH_VERTEX` || `requiredIndices > MAX_SCORCH_INDEX`) return `true`; // ignore scorch
- else if (`requiredVertices > MAX_SCORCH_VERTEX - m_curNumScorchVertices` || ...) return `false`;

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[xezon]

Can you explain why the ratio is 2 to 1 ?

[stephanmeesters]

The precise ratio is a maximum of 22844 vertices needed for 65535 indices (more indices than vertices because of vertex sharing), and 32768 is the nearest power of two up from 22844.

Actually 65535/2 is wrong and will be off by one due to rounding down, will correct that.

[xezon]

I would have expected 3 indices for 1 vertex.

[stephanmeesters]

It depends on the size of the scorch, for large scorches an inside vertex of the mesh is typically shared by 6 surrounding triangles and we get an average ratio of 6:1, while for small scorches the boundaries become more important where we have less vertex sharing (max 2 triangles in corners, max 4 at edges) and we see the ratio shrinking towards 3:1. The 32768 vertex limit gives a safe value with some margin.

[xezon]

Under what conditions does this hit? Maybe it should be an assert instead?

[stephanmeesters]

In ordinary gameplay I would not expect scorches outside the map, but it can happen in World Builder. This is just an early return, without it it would run through the for loops and end up writing nothing to the buffers.

[xezon]

Maybe return enum values to make the return meaning more explicit?