Skip to content

fix(snapshot): restore refuses a mesh with fewer than 3 vertices - #423

Merged
siracozmen01 merged 2 commits into
mainfrom
fix/restore-mesh-min-vertices
Nov 8, 2026
Merged

siracozmen01 merged 2 commits into
mainfrom
fix/restore-mesh-min-vertices

Conversation

@siracozmen01

@siracozmen01 siracozmen01 commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

m3CreateMeshShape refuses a mesh with fewer than 3 vertices with m3_errorInvalid, in MeshContentCheck in src/shape.c, and replay goes through the same check. m3World_Restore checks a mesh's counts in MeshTailEnd in src/snapshot.c. That check refuses only a negative count, a count past its limit, and a vertex count that is zero while the triangle count is not, or the reverse. So a snapshot could hold a mesh of 1 or 2 vertices, which no create or replay builds, and the restored world stepped with it. A probe on main cut a 3-vertex mesh to 2 and to 1 vertex in its snapshot, and both restored with m3_success (#422).

Change

MeshTailEnd also refuses a live mesh with fewer than M3_MESH_MIN_VERTICES vertices, so restore returns m3_errorFormat before any byte lands and leaves the world as it was. A live mesh already has at least 1 triangle, since its triangle count is zero only when its vertex count is.

M3_MESH_MIN_VERTICES, 3, is a new internal name in src/world_internal.h. The public create, the content check that every create shares, the journal replay and restore all use it, so the four checks cannot drift apart.

No step, result or golden hash moves: the check only refuses snapshots the library cannot write. The height field and hull tails already run their create checks.

Verification

  • TestRestoreRefusesMeshUnderThreeVertices in test/test_restore_links.c builds a mesh of 3 vertices whose one triangle uses only the first two. It cuts the mesh to 2 vertices, and to 1 vertex with the triangle rewritten to name vertex 0 at every corner, so both cuts pass the index check and only the vertex count is short. Each is refused with the world's hash unchanged and no misuse counted, and the clean snapshot still restores.
  • With the new condition removed, which is main's restore, both cuts restore and the test fails twice, once per cut. With it, a gcc-14 Debug build with -DMAUL3D_WERROR=ON passes the test.
  • tools/check.sh pr passes on this head, b2b7b11: 78 and 76 tests, clang-tidy clean, and fuzz with no finding.

Closes #422

@siracozmen01 siracozmen01 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review at b23f4a6: changes needed (1). I read the code and did not run it.

1. The 1-vertex cut is refused on main too, for another reason, so it does not test the new check. The mesh's one triangle is {0, 1, 0}. Cut to 1 vertex, the triangle still names vertex 1. MeshContentFits (src/snapshot.c:210-217) already refuses an index at or past the vertex count, so m3World_Restore returns m3_errorFormat on main for keep == 1 as well.

  • Only the 2-vertex cut fails without the new condition. The body's "With the new check removed, the test fails on the refusal" holds for that cut alone.
  • Make the triangle use vertex 0 only ({0, 0, 0}, which create takes as it takes {0, 1, 0}). Then both cuts stay valid apart from the vertex count, and each is refused only by vc > 0 && vc < 3. Please confirm both fail on main.

What you asked about.

  • The same bound as create. MeshContentCheck in src/shape.c:148 refuses vertexCount < 3 || triangleCount < 1. The new (vc > 0 && vc < 3) with the existing (vc == 0) != (tc == 0) refuses the same live meshes, since a live mesh's triangle count is then at least 1. The 3 is a literal in both places, not a shared constant, so the two agree now but nothing ties them. An internal constant used by both would make drift impossible. That is not needed for this fix to be correct.
  • Only live meshes. A freed slot writes 0 and 0. vc > 0 && vc < 3 is false for it, and the pairing check still holds, so a world with destroyed meshes restores as before.
  • Nothing the library writes is refused, and no hash moves. Every live mesh comes from a create or a replay through MeshContentCheck, so its vertex count is 3 or more. The change is a refusal in MeshTailEnd before any write, and no write path changes. TestRestoreAcceptsWhatTheEngineWrites still covers the written forms.
  • fix without ! fits at 1.x: restore now refuses bytes the library never writes, and no API changes.
  • The body has Problem, Change and Verification, and no column-0 name( line.

@siracozmen01 siracozmen01 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Re-review at b2b7b11: ready, once CI goes green.

  • My point is fixed. For the 1-vertex cut the test now zeroes the three indices at cut + head, where MeshContentFits reads them, right after the kept vertices. Both cuts pass the index check, and only the vertex count is short. The session reports that both fail with main's restore.

  • The constant changes no behavior. M3_MESH_MIN_VERTICES (3, in world_internal.h) replaces the literal at the four sites, each with the same comparison and the same refusal:

    • MeshInputCheck in shape_create.c (m3_errorInvalid)
    • MeshContentCheck in shape.c (m3_errorInvalid)
    • m3ReplayCreateMeshShape in journal_replay_shapes.c (false)
    • MeshTailEnd in snapshot.c (-1)

    Both files that did not have it now include world_internal.h. The comparisons are against the same int32_t values.

  • No minimum-vertex literal is left in shape_create.c, shape.c, journal_replay_shapes.c, snapshot.c, shape_data.c, shape_state.c, mesh_bvh.c or journal.c. The k < 3 loops that remain walk a triangle's corners.

No findings.

@siracozmen01
siracozmen01 merged commit c25527f into main Nov 8, 2026
29 checks passed
@siracozmen01
siracozmen01 deleted the fix/restore-mesh-min-vertices branch November 8, 2026 13:31
@rawframe-release rawframe-release Bot mentioned this pull request Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restore takes a mesh with fewer than 3 vertices, which create refuses

1 participant