Fix animated trace joint surface mapping - #153
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9a90fdf8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( meshIndex >= MD5_BackSideSurfaceIdOffset ) { | ||
| meshIndex -= MD5_BackSideSurfaceIdOffset; |
There was a problem hiding this comment.
For an otherwise valid MD5 containing at least 1,001 one-sided meshes, the front surface of mesh 1000 has ID 1000, so this unconditional subtraction resolves its trace to mesh 0 and reports the wrong joint; the loader explicitly permits up to 65,536 meshes. The equivalent first-match alias exists in rvRenderModelMD5R::NearestJoint at Model_md5r.cpp:6122-6123, where ID 1000 matches mesh 0's hypothetical backside before mesh 1000's front, even when mesh 0 does not generate backsides. The backside namespace must not overlap valid front IDs, or resolution must verify that a backside was actually generated before preferring it.
Useful? React with 👍 / 👎.
1 participant
Fixes #151.
Animated model traces were passing a transient dynamic-snapshot array position into the source model's
NearestJointlookup. MD5 and MD5R snapshots can omit or reorder authored meshes, and a two-sided material appends an extra reversed surface. A later generated backface can therefore have an array position greater than the source mesh count, which reached the fatal guard reported in the issue.Trace results now carry the snapshot surface's stable ID into joint lookup. Classic MD5 resolves a generated backface ID to its authored mesh index, while MD5R matches both front and back IDs against the mesh identifier. This also removes the source model's dependence on mutable per-snapshot surface positions.
Validation:
tools/build/meson_setup.ps1 compile -C builddirtools/build/meson_setup.ps1 install -C builddir --no-rebuild --skip-subprojectspython tools/tests/renderer_vulkan_md5r_compatibility.pypython tools/tests/renderer_gpu_skinning.pypython tools/tests/linux_arm64_source_portability.pypython tools/tests/docs_link_integrity.pygame/airdefense1benchmark using the complete map/proc/cm/AAS set attached to ERROR: idRestoreGame::ReadSyncId: marker mismatch while reading ReadTrace at offset 696660 (got 0x00000034, expected 0x5934514f) #123: passed with loaded AAS data, a valid engine screenshot, and no renderer error or fatal marker.The supplied map archive on the previous binary also reached gameplay by itself, so it does not contain the reporter's complete model/material override trigger. The unsafe generated-surface contract is deterministic in the source and is corrected for both animated model formats.