fix(N.5b T6): index-length validation + document VertsPerLandblock %6 invariant
Code review (Important #1): AddLandblock validated Vertices.Length but not Indices.Length. The indices loop indexes meshData.Indices[0..383] unconditionally — out-of-range input would throw IndexOutOfRangeException instead of the clearer ArgumentException the vertex check raises. Today LandblockMesh.Build always produces 384/384, so this is defensive forward-compat for future mesh sources. Code review (Important #2): The shader (terrain_modern.vert:gl_VertexID % 6) only correctly picks the cell-corner index because we bake `slot * VertsPerLandblock` into indices and 384 is a multiple of 6. That invariant is now documented in a comment near the constant — anyone changing it must audit the shader. Build green: 0 errors / 0 warnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0a77bd1fd7
commit
3418f65462
1 changed files with 12 additions and 1 deletions
|
|
@ -17,7 +17,14 @@ namespace AcDream.App.Rendering;
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public sealed unsafe class TerrainModernRenderer : IDisposable
|
public sealed unsafe class TerrainModernRenderer : IDisposable
|
||||||
{
|
{
|
||||||
private const int VertsPerLandblock = LandblockMesh.VerticesPerLandblock; // 384
|
// VertsPerLandblock MUST stay divisible by 6 — terrain_modern.vert uses
|
||||||
|
// `gl_VertexID % 6` to pick the cell-corner index (BL/BR/TR/TL), and
|
||||||
|
// because we bake `slot * VertsPerLandblock` into indices CPU-side and
|
||||||
|
// pass BaseVertex=0 to MultiDrawElementsIndirect, gl_VertexID becomes
|
||||||
|
// `slot * VertsPerLandblock + local_index`. The shader's modulo-6 only
|
||||||
|
// reduces to `local_index % 6` because 384 is a multiple of 6. Changing
|
||||||
|
// either constant without auditing the shader will silently mis-render.
|
||||||
|
private const int VertsPerLandblock = LandblockMesh.VerticesPerLandblock; // 384 (= 64 cells * 6 verts)
|
||||||
private const int IndicesPerLandblock = VertsPerLandblock;
|
private const int IndicesPerLandblock = VertsPerLandblock;
|
||||||
private const int VertexSize = 40; // sizeof(TerrainVertex)
|
private const int VertexSize = 40; // sizeof(TerrainVertex)
|
||||||
private const int IndexSize = sizeof(uint);
|
private const int IndexSize = sizeof(uint);
|
||||||
|
|
@ -89,6 +96,10 @@ public sealed unsafe class TerrainModernRenderer : IDisposable
|
||||||
throw new ArgumentException(
|
throw new ArgumentException(
|
||||||
$"Expected {VertsPerLandblock} vertices, got {meshData.Vertices.Length}",
|
$"Expected {VertsPerLandblock} vertices, got {meshData.Vertices.Length}",
|
||||||
nameof(meshData));
|
nameof(meshData));
|
||||||
|
if (meshData.Indices.Length != IndicesPerLandblock)
|
||||||
|
throw new ArgumentException(
|
||||||
|
$"Expected {IndicesPerLandblock} indices, got {meshData.Indices.Length}",
|
||||||
|
nameof(meshData));
|
||||||
|
|
||||||
if (_idToSlot.ContainsKey(landblockId))
|
if (_idToSlot.ContainsKey(landblockId))
|
||||||
RemoveLandblock(landblockId);
|
RemoveLandblock(landblockId);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue