Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stephanmeesters wants to merge 4 commits into
base: main
Choose a base branch
from
+33 −13
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
73f46a9
fix(scorch): Early return if scorch does not fit in buffer
stephanmeesters Aug 21, 2026
8e9e304
tweak(scorch): Increase scorch buffer limits to 16-bit maximum
stephanmeesters Aug 17, 2026
3ebf358
Address review comment
stephanmeesters Aug 21, 2026
2482a6f
Correct power of two. Add scorch result enum
stephanmeesters Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,24 @@ class W3DScorch : public W3DScorchInterface

enum
{
MAX_SCORCH_VERTEX = 8194,
MAX_SCORCH_INDEX = 6 * 8194,
MAX_SCORCH_VERTEX = 32768,
MAX_SCORCH_INDEX = 65535,

@xezon xezon Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you explain why the ratio is 2 to 1 ?

@stephanmeesters stephanmeesters Aug 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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 xezon Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would have expected 3 indices for 1 vertex.

@stephanmeesters stephanmeesters Aug 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

MAX_SCORCH_MARKS = 500,
SCORCH_MARKS_IN_TEXTURE = 9,
SCORCH_PER_ROW = 3
};

enum WriteScorchResult
{
SCORCH_WRITTEN,
SCORCH_SKIPPED,
SCORCH_BUFFER_FULL
};

Bool isDuplicate(const TScorch& scorch) const;
void updateScorches(WorldHeightMap& map); ///< Update m_vertexScorch and m_indexScorch so all scorches will be drawn.
Bool writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map, UnsignedInt diffuse,
VertexFormatXYZDUV1* curVb, UnsignedShort* curIb);
WriteScorchResult writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map, UnsignedInt diffuse,
VertexFormatXYZDUV1* curVb, UnsignedShort* curIb);

DX8VertexBufferClass* m_vertexScorch; ///< Scorch vertex buffer.
DX8IndexBufferClass* m_indexScorch; ///< indices defining a triangles for the scorch drawing.
Expand Down
31 changes: 22 additions & 9 deletions Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,17 @@ void W3DScorch::updateScorches(WorldHeightMap& map)
// TheSuperHackers @info Scorches are written in reverse order to ensure that the last added scorches fit in the buffers.
for (std::deque<TScorch>::reverse_iterator it = m_scorches.rbegin(); it != m_scorches.rend(); ++it)
{
if (!writeScorchToBuffer(*it, map, diffuse,
vb + m_curNumScorchVertices, ib + m_curNumScorchIndices))
if (writeScorchToBuffer(*it, map, diffuse,
vb + m_curNumScorchVertices, ib + m_curNumScorchIndices) == SCORCH_BUFFER_FULL)
{
return;
}
}
}

Bool W3DScorch::writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map, UnsignedInt diffuse,
VertexFormatXYZDUV1* curVb, UnsignedShort* curIb)
W3DScorch::WriteScorchResult W3DScorch::writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map,
UnsignedInt diffuse, VertexFormatXYZDUV1* curVb,
UnsignedShort* curIb)
{
Real radius = scorch.radius;
Vector3 loc = scorch.location;
Expand All @@ -196,14 +197,28 @@ Bool W3DScorch::writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map,
{
maxY = map.getYExtent() - map.getBorderSizeInline();
}

const Int vertexCountX = maxX - minX;
const Int vertexCountY = maxY - minY;
if (vertexCountX <= 0 || vertexCountY <= 0)

@xezon xezon Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

@stephanmeesters stephanmeesters Aug 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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 xezon Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

{
return SCORCH_SKIPPED;
}

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

Int startVertex = m_curNumScorchVertices;
Int i, j;
for (j = minY; j < maxY; j++)
{
for (i = minX; i < maxX; i++)
{
if (m_curNumScorchVertices >= MAX_SCORCH_VERTEX)
return false;
curVb->diffuse = diffuse;
Real theZ = amtToFloat + getMapHeight(map, i, j);
// The scorchmarks are spaced out by 1.5 in the texture.
Expand All @@ -225,8 +240,6 @@ Bool W3DScorch::writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map,
{
for (i = 0; i < maxX - minX - 1; i++)
{
if (m_curNumScorchIndices + 6 > MAX_SCORCH_INDEX)
return false;
Int xNdx = i + minX + map.getBorderSizeInline();
Int yNdx = j + minY + map.getBorderSizeInline();
Bool flipForBlend = map.getFlipState(xNdx, yNdx);
Expand Down Expand Up @@ -257,5 +270,5 @@ Bool W3DScorch::writeScorchToBuffer(const TScorch& scorch, WorldHeightMap& map,
}
}

return true;
return SCORCH_WRITTEN;
}
Loading