Skip to content

feat(ps1): model-space TMD/HMD RAM scanner (#674)#674

Merged
fernandotonon merged 3 commits into
masterfrom
feat/ps1-tmd-hmd-scanner-674
May 26, 2026
Merged

feat(ps1): model-space TMD/HMD RAM scanner (#674)#674
fernandotonon merged 3 commits into
masterfrom
feat/ps1-tmd-hmd-scanner-674

Conversation

@fernandotonon
Copy link
Copy Markdown
Owner

@fernandotonon fernandotonon commented May 25, 2026

Summary

Adds two format-aware runtime scanners that find Sony SDK mesh structures in PS1 system RAM and emit them as model-space ReconstructedMesh parts, bypassing the inverse-projection screen-space path. On TMD-using games this is what turns the "blob of triangles" output into recognizable geometry without waiting on #676 (forked mednafen with in-core GTE hook).

  • PsxTmdRamScanner — sweeps RAM (4-byte stride) for the 0x00000041 TMD magic. Validates flags / numObj / offsets / counts, walks vertex pools + primitive packets (mono/Gouraud tri & quad, textured + no-light variants), and emits via the new EmuHooks::onModelMesh hook. Material names for textured packets use the standard PS1Rip_tpage_XXXX_clut_YYYY_stN_dmN template so the existing capture texture-decode pipeline (PS1: CLUT-aware texture decoding (4bpp/8bpp/15bpp → RGBA) #421) binds them with no extra wiring. Supports both flag=0 (on-disk relative offsets) and flag=1 (KSEG0 absolute pointers).
  • PsxHmdRamScanner — v1 stub for the 0x00000050 HMD magic. Opt-in via QTMESH_PS1_HMD_SCANNER=1; surfaces candidate counts for diagnostics. Full primitive walk lands once a known-good HMD asset is available to validate against (HMD's hierarchical layout is a documented false-positive source).
  • CaptureBuffer::addModelMesh dedupes by FNV-1a 64-bit content hash so the same asset encountered every frame only stores once.
  • MeshReconstructor::buildParts appends snapshot.modelMeshes to the parts list; the existing MeshTopologyHash dedupe in reconstructDeduped collapses byte-identical TMDs into one unique mesh + N instance transforms.
  • MeshReconstructionStats::modelMeshVertices flows into gteInversePercent as trusted vertices, so the quality indicator flips from 0% → ~100% on TMD-using games.
  • UI / Sentry: status bar appends tmd N / hmd N; new Gp0CaptureSource::RamModelMesh (label ram_model_mesh) wins the primary-source label whenever any TMD/HMD surfaced; new ps1.rip.capture.modelmesh Sentry breadcrumb; ps1.rip.matrix.stats adds model_meshes=N.

Builds on #662 (live GP0 FIFO bridge); rebase against master once #662 is merged.

Coverage

Format Where used v1
TMD (0x41) Sony SDK statics — Tekken 1/2/3, Ridge Racer 1/RR, Net Yaroze homebrew, Wipeout 1/2097, R-Type Delta, Klonoa, many pre-FF7 Square emits meshes
HMD (0x50) Sony SDK hierarchical/skinned stub (candidate count only)
Custom engines (Crash, Spyro, FFVII field, MGS) proprietary packed layouts out of scope — needs #676

Test plan

  • PsxTmdRamScannerTest.FindsSingleTmdAtOffset — hand-crafted 1-object 1-tri TMD at byte 0x10000 of 2 MiB zeroed RAM → 1 mesh, 3 verts, finite world-space positions.
  • PsxTmdRamScannerTest.RejectsGarbageHeader — wrong magic / oversized numObj / bad flags / OOB pointers → 0 meshes.
  • PsxTmdRamScannerTest.DedupesIdenticalCopies — two byte-identical TMDs at different offsets → 1 mesh (content-hash dedupe).
  • PsxTmdRamScannerTest.RejectsZeroVertexObject — degenerate object header → 0 meshes.
  • PsxTmdRamScannerTest.HandlesPureRandomEntropyWithoutFalsePositives — 64 KiB of seeded random bytes → 0 false positives.
  • PsxTmdRamScannerTest.EmitsCorrectMaterialNameForTexturedPrim0x24 textured-tri packet → PS1Rip_tpage_1234_clut_abcd_... material binding.
  • PsxHmdRamScannerTest.CountsPlausibleHmdCandidates — magic + plausible map length → counted; OOB map length → rejected.
  • PsxHmdRamScannerTest.EmitsNothingByDefault — env-gated stub returns 0 meshes by default.
  • All 67 existing PS1-related tests across 17 suites still pass (CaptureBuffer / Gp0HookDispatch / Gp0CapturePaths / MeshReconstructor / MeshTopologyHash / PsxOrderingTable / PsxGteRamScanner / PsxGpuRamScanner / PsxGteCop2 / PsxGteIsoDedupe / GteCapture / GpuCommandParser / PS1Rip*).
  • Manual smoke on a TMD-using game (Net Yaroze SDK demo or Ridge Racer 1 disc): confirm tmd N shows non-zero in the status bar and source flips to ram_model_mesh.
  • Manual smoke on Crash/Spyro: confirm tmd 0 (as expected — custom engines fall through to the screen-space path).

Out of scope (queued follow-ups)

  • #675 — heuristic + math fix for the screen-space inverse path.
  • #676 — forked mednafen_psx_libretro with in-core RTPS/RTPT callback for ground-truth model-space recovery on any game.
  • #677 — disc/ISO scanner for RSD/PLY/MAT/GRP/TIX off-line ripping.

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Added support for capturing model-space meshes directly from PS1 system RAM using format-aware scanners for Sony SDK mesh structures.
  • Improvements

    • Enhanced session window display and diagnostics with additional model mesh statistics and candidate counts.
    • Expanded mesh reconstruction to include captured model meshes as additional mesh parts.
  • Documentation

    • Updated PS1 runtime geometry extraction design documentation with model-space RAM scanning strategy and new scanner capabilities.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Warning

Review limit reached

@fernandotonon, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 41 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b6c7dc4c-6539-4eee-832d-532746861cfe

📥 Commits

Reviewing files that changed from the base of the PR and between 291bfce and e98b758.

📒 Files selected for processing (5)
  • src/PS1/PS1_RIP_DESIGN.md
  • src/PS1/runtime/Gp0HookDispatch.cpp
  • src/PS1/runtime/MeshReconstructor.cpp
  • src/PS1/runtime/PsxModelRamScanCommon.h
  • src/PS1/runtime/PsxTmdRamScanner_test.cpp
📝 Walkthrough

Walkthrough

This PR adds model-space RAM scanner support to the PS1 geometry extraction pipeline. It introduces PSX TMD and HMD structure scanners that detect and reconstruct meshes directly from emulator RAM, extends the capture pipeline to track these model-space outputs, updates statistics and telemetry to report scanner activity, and integrates all components into the existing rendering/capture framework.

Changes

PS1 Model-Space Mesh Capture Integration

Layer / File(s) Summary
Core Data Model and Capture Infrastructure
src/PS1/runtime/CapturedModelMesh.h, src/PS1/runtime/ReconstructedMesh.h, src/PS1/runtime/MeshReconstructor.h, src/PS1/runtime/CaptureBuffer.h, src/PS1/runtime/CaptureBuffer.cpp, src/PS1/runtime/CaptureSnapshot.h, src/PS1/runtime/CaptureSnapshot.cpp, src/PS1/runtime/EmuHooks.h
New CapturedModelMesh POD struct bundles reconstructed mesh with metadata (source address, format label, content hash). Extracted ReconstructedVertex/SubMesh/Mesh types to shared ReconstructedMesh.h. Extended CaptureBuffer with addModelMesh() method (validates, deduplicates by hash, appends), modelMeshes() accessor, and internal dedup storage. Updated CaptureSnapshot to carry modelMeshes collection. Added EmuHooks::onModelMesh() virtual callback for scanner emission.
Capture Statistics and Source Attribution
src/PS1/runtime/Gp0CaptureStats.h, src/PS1/runtime/Gp0CaptureStats.cpp, src/PS1/runtime/MeshReconstructionStats.h, src/PS1/runtime/MeshReconstructionStats.cpp, src/PS1/runtime/Gp0HookDispatch.cpp
Added Gp0CaptureSource::RamModelMesh enum variant and three new frame counters: ramTmdMeshes, ramHmdMeshes, ramHmdCandidates. Updated gteInversePercent() to include modelMeshVertices in "trusted" numerator. Introduced promoteModelMeshSource() helper to override primary source to RamModelMesh when actual (not candidate-only) meshes are emitted.
TMD RAM Scanner Implementation and Tests
src/PS1/runtime/PsxModelRamScanCommon.h, src/PS1/runtime/PsxTmdRamScanner.h, src/PS1/runtime/PsxTmdRamScanner.cpp, src/PS1/runtime/PsxTmdRamScanner_test.cpp
Adds little-endian readers, bounds-checked accessors, and FNV-1a hash utility in PsxModelRamScanCommon.h. PsxTmdRamScanner searches RAM for TMD headers, validates object tables, reads vertex pools, walks primitive packets, reconstructs textured/flat-lit triangles with material grouping, computes content hash over byte span, emits via onModelMesh(). Comprehensive test suite: valid mesh discovery, malformed header rejection, deduplication, zero-vertex filtering, random data robustness, material naming for textured packets, and regression test for payload hashing coverage.
HMD RAM Scanner and Frame Capture Integration
src/PS1/runtime/PsxHmdRamScanner.h, src/PS1/runtime/PsxHmdRamScanner.cpp, src/PS1/runtime/PsxHmdRamScanner_test.cpp, src/PS1/runtime/Gp0HookDispatch.cpp
PsxHmdRamScanner performs magic-bytes-only candidate counting with plausibility filter (v1 stub). captureFromSystemRam() gates on QTMESH_PS1_HMD_SCANNER env var and EmuHooks capture state. Integrated into Gp0HookDispatch::captureFrameFromSystemRam() to run both TMD and HMD scanners, populate stats, and promote primary source. Tests verify candidate counting, env-var gating, and that bare candidates do not promote source without emitted meshes.
Mesh Reconstruction and Hook Integration
src/PS1/runtime/MeshReconstructor.cpp, src/PS1/runtime/RipperHooks.h, src/PS1/runtime/RipperHooks.cpp
MeshReconstructor::buildParts() appends non-empty model meshes from snapshot.modelMeshes as additional parts, aggregates vertex counts, and expands bounding box. RipperHooks::onModelMesh() forwards captured meshes to m_buffer when capture enabled. endGpuCapturePass() overrides primarySource to RamModelMesh when TMD/HMD mesh counts are non-zero.
Telemetry and UI Reporting
src/PS1/runtime/PS1RipManager.cpp, src/PS1/runtime/PS1RipWorker.cpp, src/PS1/runtime/PS1RipSessionWindow.cpp
Extended Sentry breadcrumbs: ps1.rip.matrix.stats adds model_meshes field, ps1.rip.capture.summary includes ramTmdMeshes/ramHmdMeshes/ramHmdCandidates counts, new conditional ps1.rip.capture.modelmesh breadcrumb reports when actual meshes emitted. onMeshBuilt() expands GP0 status string to display TMD/HMD/candidate diagnostics.
Build Configuration and Design Documentation
src/PS1/CMakeLists.txt, tests/CMakeLists.txt, src/PS1/PS1_RIP_DESIGN.md
Registered PsxHmdRamScanner.cpp, PsxTmdRamScanner.cpp and related headers in build. Updated design doc: revised per-core capability matrix with scan notes, added "Model-space RAM scanners" section describing TMD validation rules, HMD v1 stub, dedup/material handling, and primary-source promotion; rewrote troubleshooting guidance to explain screen-space blob causes and direct users to model-space scanner path with env-var toggles.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • fernandotonon/QtMeshEditor#412: The main PR directly implements the PSX TMD/HMD RAM scanner components and capture pipeline extensions defined as Phase 3 deliverables in this epic.

Possibly related PRs

  • fernandotonon/QtMeshEditor#601: Builds on Phase-4 by extending CaptureSnapshot/MeshReconstructor flow to carry and reconstruct new model-space RAM-scanner meshes as additional parts.
  • fernandotonon/QtMeshEditor#671: Both PRs modify the PS1 RIP pipeline breadcrumb/reporting in PS1RipManager/PS1RipWorker/PS1RipSessionWindow to thread new capture metadata through telemetry paths.
  • fernandotonon/QtMeshEditor#645: The main PR extends Gp0HookDispatch system-RAM capture and EmuHooks to integrate the new TMD/HMD RAM scanners, building directly on that PR's introduction of RAM-based GP0 capture infrastructure.

Poem

🐰 A scanner hops through RAM so deep,
Finding meshes in the PSX heap,
TMD blobs and HMD dreams,
Reconstructed triangular seams,
Model-space geometry to keep!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main feature addition: format-aware scanners for PS1 model-space mesh structures (TMD/HMD), clearly identifying both the primary change and the associated issue.
Description check ✅ Passed The description is comprehensive and well-structured, covering summary, technical details, test plan checklist, and out-of-scope follow-ups. It follows the repository template with detailed explanations of each scanner's behavior, integration points, and validation strategy.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ps1-tmd-hmd-scanner-674

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ce3daff8c

ℹ️ 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".

Comment thread src/PS1/runtime/PsxTmdRamScanner.cpp Outdated
Comment on lines +429 to +433
// We don't know prim-payload sizes ahead of time. Conservatively reserve 8 bytes
// per prim (smallest packet) — used purely for dedupe range, not for parsing.
const auto r = resolveOffset(h.primOff, flags, tmdStart, ramSize, size_t(h.nPrim) * 8u);
if (r.ok)
maxEnd = std::max(maxEnd, r.base + size_t(h.nPrim) * 8u);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Hash full primitive payload when computing TMD span

computeTmdSpan assumes 8 bytes per primitive, but many supported packets are larger (for example ilen=5/6/8), so span can end before the real primitive stream ends. Because this same span is used for contentHash and for scan skipping, two different TMD blobs with identical prefixes can be deduped as one, and the scanner can resume inside the same blob instead of advancing past it.

Useful? React with 👍 / 👎.

Comment on lines +39 to +40
if (stats.ramTmdMeshes > 0 || stats.ramHmdMeshes > 0)
stats.primarySource = Gp0CaptureSource::RamModelMesh;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not mark HMD candidates as model-mesh capture source

The HMD scanner currently returns only candidate counts and explicitly emits no meshes, but endGpuCapturePass treats any nonzero ramHmdMeshes as proof of model-space mesh capture and forces primarySource to ram_model_mesh. With QTMESH_PS1_HMD_SCANNER=1, this can mislabel captures (including false positives) and hide the actual GP0 source attribution.

Useful? React with 👍 / 👎.

fernandotonon added a commit that referenced this pull request May 25, 2026
Adds two format-aware runtime scanners that find Sony SDK mesh structures in
PS1 system RAM and emit them as model-space `ReconstructedMesh` parts,
bypassing the inverse-projection screen-space path that produces "blob of
triangles" output on retail captures.

`PsxTmdRamScanner` sweeps RAM at 4-byte stride for the `0x00000041` TMD magic.
For each candidate it validates flags, numObj, object-header offset ranges
and primitive counts, then walks the vertex/normal pools and primitive packets
(mono/Gouraud triangles & quads, textured + no-light variants) and emits a
fully-formed `CapturedModelMesh` via the new `EmuHooks::onModelMesh` hook.
Material names for textured packets reuse `MeshReconstructor::textureMaterialName`
so the existing capture texture-decode pipeline (#421) binds them with no
extra wiring. `PsxHmdRamScanner` is a v1 stub (opt-in via
`QTMESH_PS1_HMD_SCANNER=1`) that surfaces magic-bytes candidates for diagnostics
while we collect a known-good HMD asset to validate the primitive walk against.

`CaptureBuffer::addModelMesh` dedupes by FNV-1a 64-bit hash of the TMD byte
span so the same asset encountered every frame only stores once.
`MeshReconstructor::buildParts` appends `snapshot.modelMeshes` to the parts
list — the dedupe pass in `reconstructDeduped` collapses byte-identical TMDs
to one unique mesh + N instances, same as for screen-space matrix-grouped parts.
`MeshReconstructionStats::modelMeshVertices` count flows into `gteInversePercent`
as trusted vertices, so the quality indicator flips from 0% to ~100% on
TMD-using games.

UI / Sentry: `Gp0CaptureStats::ramTmdMeshes`/`ramHmdMeshes` track per-frame
counts; `Gp0CaptureSource::RamModelMesh` (label `ram_model_mesh`) is the new
primary source when any TMD/HMD surfaced; status bar appends `tmd N / hmd N`;
new `ps1.rip.capture.modelmesh` breadcrumb; `ps1.rip.matrix.stats` adds
`model_meshes=N`.

Disable per-format via `QTMESH_PS1_TMD_SCANNER=0`.

Tests (8 new, all green): finds single TMD at offset, rejects garbage headers,
dedupes identical copies, rejects zero-vertex objects, handles random entropy
without false positives, emits correct PS1Rip_tpage_*_clut_* material names
for textured prims; HMD scanner counts plausible candidates and emits nothing
by default. 67 PS1 tests across 17 suites pass with the new code.

Co-authored-by: Cursor <cursoragent@cursor.com>
fernandotonon added a commit that referenced this pull request May 25, 2026
…s mesh distinction

Addresses two code review findings on #674:

1) PsxTmdRamScanner::computeTmdSpan no longer assumes 8 bytes per primitive.
   The walker now records its actual end byte position via
   PrimContext::maxPrimStreamEnd, and computeTmdSpan uses that directly.
   Two TMDs with identical 8-byte-per-prim prefixes but different full
   payloads now produce distinct contentHash values, so they are no longer
   dedupe-merged into one, and the scanner advance correctly skips past
   the full blob instead of resuming inside it.

2) HMD scanner v1 returns candidate counts, not emitted meshes. Split
   `Gp0CaptureStats::ramHmdMeshes` (actual onModelMesh emissions, 0 until
   the v2 walker lands) from new `ramHmdCandidates` (plausible magic-byte
   hits). Only emitted meshes promote primarySource to RamModelMesh —
   bare candidate counts no longer hide GP0 source attribution behind a
   misleading `ram_model_mesh` label.

Status bar + Sentry breadcrumbs surface both fields with the new
`hmd_cand` suffix so testers can still confirm the magic-bytes path
works on HMD-using titles without misattributing the capture.

Adds PsxTmdRamScannerTest.ContentHashCoversFullPrimitivePayload and
PsxHmdRamScannerTest.HmdCandidatesDoNotPromoteModelMeshSource.

Co-authored-by: Cursor <cursoragent@cursor.com>
@fernandotonon fernandotonon force-pushed the feat/ps1-tmd-hmd-scanner-674 branch from 1ce3daf to 54d84b9 Compare May 25, 2026 23:30
fernandotonon and others added 2 commits May 25, 2026 21:43
Adds two format-aware runtime scanners that find Sony SDK mesh structures in
PS1 system RAM and emit them as model-space `ReconstructedMesh` parts,
bypassing the inverse-projection screen-space path that produces "blob of
triangles" output on retail captures.

`PsxTmdRamScanner` sweeps RAM at 4-byte stride for the `0x00000041` TMD magic.
For each candidate it validates flags, numObj, object-header offset ranges
and primitive counts, then walks the vertex/normal pools and primitive packets
(mono/Gouraud triangles & quads, textured + no-light variants) and emits a
fully-formed `CapturedModelMesh` via the new `EmuHooks::onModelMesh` hook.
Material names for textured packets reuse `MeshReconstructor::textureMaterialName`
so the existing capture texture-decode pipeline (#421) binds them with no
extra wiring. `PsxHmdRamScanner` is a v1 stub (opt-in via
`QTMESH_PS1_HMD_SCANNER=1`) that surfaces magic-bytes candidates for diagnostics
while we collect a known-good HMD asset to validate the primitive walk against.

`CaptureBuffer::addModelMesh` dedupes by FNV-1a 64-bit hash of the TMD byte
span so the same asset encountered every frame only stores once.
`MeshReconstructor::buildParts` appends `snapshot.modelMeshes` to the parts
list — the dedupe pass in `reconstructDeduped` collapses byte-identical TMDs
to one unique mesh + N instances, same as for screen-space matrix-grouped parts.
`MeshReconstructionStats::modelMeshVertices` count flows into `gteInversePercent`
as trusted vertices, so the quality indicator flips from 0% to ~100% on
TMD-using games.

UI / Sentry: `Gp0CaptureStats::ramTmdMeshes`/`ramHmdMeshes` track per-frame
counts; `Gp0CaptureSource::RamModelMesh` (label `ram_model_mesh`) is the new
primary source when any TMD/HMD surfaced; status bar appends `tmd N / hmd N`;
new `ps1.rip.capture.modelmesh` breadcrumb; `ps1.rip.matrix.stats` adds
`model_meshes=N`.

Disable per-format via `QTMESH_PS1_TMD_SCANNER=0`.

Tests (8 new, all green): finds single TMD at offset, rejects garbage headers,
dedupes identical copies, rejects zero-vertex objects, handles random entropy
without false positives, emits correct PS1Rip_tpage_*_clut_* material names
for textured prims; HMD scanner counts plausible candidates and emits nothing
by default. 67 PS1 tests across 17 suites pass with the new code.

Co-authored-by: Cursor <cursoragent@cursor.com>
…s mesh distinction

Addresses two code review findings on #674:

1) PsxTmdRamScanner::computeTmdSpan no longer assumes 8 bytes per primitive.
   The walker now records its actual end byte position via
   PrimContext::maxPrimStreamEnd, and computeTmdSpan uses that directly.
   Two TMDs with identical 8-byte-per-prim prefixes but different full
   payloads now produce distinct contentHash values, so they are no longer
   dedupe-merged into one, and the scanner advance correctly skips past
   the full blob instead of resuming inside it.

2) HMD scanner v1 returns candidate counts, not emitted meshes. Split
   `Gp0CaptureStats::ramHmdMeshes` (actual onModelMesh emissions, 0 until
   the v2 walker lands) from new `ramHmdCandidates` (plausible magic-byte
   hits). Only emitted meshes promote primarySource to RamModelMesh —
   bare candidate counts no longer hide GP0 source attribution behind a
   misleading `ram_model_mesh` label.

Status bar + Sentry breadcrumbs surface both fields with the new
`hmd_cand` suffix so testers can still confirm the magic-bytes path
works on HMD-using titles without misattributing the capture.

Adds PsxTmdRamScannerTest.ContentHashCoversFullPrimitivePayload and
PsxHmdRamScannerTest.HmdCandidatesDoNotPromoteModelMeshSource.

Co-authored-by: Cursor <cursoragent@cursor.com>
@fernandotonon fernandotonon force-pushed the feat/ps1-tmd-hmd-scanner-674 branch from 54d84b9 to 291bfce Compare May 26, 2026 01:45
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/PS1/runtime/MeshReconstructor.cpp (1)

227-241: ⚡ Quick win

Unify duplicated bounds-update logic to avoid drift.

This block duplicates min/max bounds code already used elsewhere; extracting a shared helper keeps future stats changes consistent.

♻️ Proposed refactor
 namespace {
+void expandBounds(MeshReconstructionStats &stats, const ReconstructedVertex &v)
+{
+    if (!stats.hasBounds()) {
+        stats.boundsMinX = stats.boundsMaxX = v.px;
+        stats.boundsMinY = stats.boundsMaxY = v.py;
+        stats.boundsMinZ = stats.boundsMaxZ = v.pz;
+        return;
+    }
+    stats.boundsMinX = std::min(stats.boundsMinX, v.px);
+    stats.boundsMaxX = std::max(stats.boundsMaxX, v.px);
+    stats.boundsMinY = std::min(stats.boundsMinY, v.py);
+    stats.boundsMaxY = std::max(stats.boundsMaxY, v.py);
+    stats.boundsMinZ = std::min(stats.boundsMinZ, v.pz);
+    stats.boundsMaxZ = std::max(stats.boundsMaxZ, v.pz);
+}
+
 void accumulateVertexStats(const ReconstructedVertex &v, MeshReconstructionStats &stats, bool usedGte)
 {
     ++stats.totalVertices;
@@
-    if (!stats.hasBounds()) {
-        stats.boundsMinX = stats.boundsMaxX = v.px;
-        stats.boundsMinY = stats.boundsMaxY = v.py;
-        stats.boundsMinZ = stats.boundsMaxZ = v.pz;
-        return;
-    }
-    stats.boundsMinX = std::min(stats.boundsMinX, v.px);
-    stats.boundsMaxX = std::max(stats.boundsMaxX, v.px);
-    stats.boundsMinY = std::min(stats.boundsMinY, v.py);
-    stats.boundsMaxY = std::max(stats.boundsMaxY, v.py);
-    stats.boundsMinZ = std::min(stats.boundsMinZ, v.pz);
-    stats.boundsMaxZ = std::max(stats.boundsMaxZ, v.pz);
+    expandBounds(stats, v);
 }
@@
-                    if (!statsOut->hasBounds()) {
-                        statsOut->boundsMinX = statsOut->boundsMaxX = v.px;
-                        statsOut->boundsMinY = statsOut->boundsMaxY = v.py;
-                        statsOut->boundsMinZ = statsOut->boundsMaxZ = v.pz;
-                    } else {
-                        statsOut->boundsMinX = std::min(statsOut->boundsMinX, v.px);
-                        statsOut->boundsMaxX = std::max(statsOut->boundsMaxX, v.px);
-                        statsOut->boundsMinY = std::min(statsOut->boundsMinY, v.py);
-                        statsOut->boundsMaxY = std::max(statsOut->boundsMaxY, v.py);
-                        statsOut->boundsMinZ = std::min(statsOut->boundsMinZ, v.pz);
-                        statsOut->boundsMaxZ = std::max(statsOut->boundsMaxZ, v.pz);
-                    }
+                    expandBounds(*statsOut, v);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/PS1/runtime/MeshReconstructor.cpp` around lines 227 - 241, This
duplicated min/max bounds logic should be centralized: extract a helper (e.g.,
MeshStats::updateBoundsWithVertex or MeshReconstructor::updateBoundsForVertex)
and replace the duplicated block in MeshReconstructor::… (the place with the if
(!statsOut->hasBounds()) check) with a call to that helper; the helper should
take the stats object and a vertex (v) and implement the hasBounds() check plus
min/max updates so all callers (including accumulateVertexStats) share the exact
same bounds-update behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/PS1/PS1_RIP_DESIGN.md`:
- Around line 78-85: The Markdown table for "Per-core capability" lacks
surrounding blank lines causing markdownlint MD058; open
src/PS1/PS1_RIP_DESIGN.md, locate the table that starts with "**Per-core
capability (`#662`, `#674`):**" and ends after the CI / no-disc smoke row, and add a
single blank line immediately before the table and a single blank line
immediately after the table so the table is separated from surrounding
paragraphs.

In `@src/PS1/runtime/Gp0HookDispatch.cpp`:
- Line 428: The primary source is promoted before model-mesh counters are
populated: move or repeat the call so
promoteModelMeshSource(pickPrimarySource(...)) runs after stats.ramTmdMeshes and
stats.ramHmdMeshes are computed; specifically, ensure stats.ramTmdMeshes and
stats.ramHmdMeshes are assigned (the code that sets them around the block that
currently assigns at lines ~527-530) before calling
pickPrimarySource/promoteModelMeshSource (functions: pickPrimarySource,
promoteModelMeshSource) so RamModelMesh can be considered when choosing
stats.primarySource; alternatively, call promoteModelMeshSource again after the
counters are set to update stats.primarySource.

In `@src/PS1/runtime/PsxModelRamScanCommon.h`:
- Around line 45-46: The FNV-1a 64-bit offset basis constant is incorrect:
replace the current kFnvOffset value with the canonical 64-bit offset basis
14695981039346656037ULL (0xcbf29ce484222325) in PsxModelRamScanCommon.h so that
constexpr uint64_t kFnvOffset and the existing constexpr uint64_t kFnvPrime use
the correct standard values.

---

Nitpick comments:
In `@src/PS1/runtime/MeshReconstructor.cpp`:
- Around line 227-241: This duplicated min/max bounds logic should be
centralized: extract a helper (e.g., MeshStats::updateBoundsWithVertex or
MeshReconstructor::updateBoundsForVertex) and replace the duplicated block in
MeshReconstructor::… (the place with the if (!statsOut->hasBounds()) check) with
a call to that helper; the helper should take the stats object and a vertex (v)
and implement the hasBounds() check plus min/max updates so all callers
(including accumulateVertexStats) share the exact same bounds-update behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3fa80718-3cf3-4fe4-8561-bcbcdfabf62e

📥 Commits

Reviewing files that changed from the base of the PR and between 63e6ec7 and 291bfce.

📒 Files selected for processing (29)
  • src/PS1/CMakeLists.txt
  • src/PS1/PS1_RIP_DESIGN.md
  • src/PS1/runtime/CaptureBuffer.cpp
  • src/PS1/runtime/CaptureBuffer.h
  • src/PS1/runtime/CaptureSnapshot.cpp
  • src/PS1/runtime/CaptureSnapshot.h
  • src/PS1/runtime/CapturedModelMesh.h
  • src/PS1/runtime/EmuHooks.h
  • src/PS1/runtime/Gp0CaptureStats.cpp
  • src/PS1/runtime/Gp0CaptureStats.h
  • src/PS1/runtime/Gp0HookDispatch.cpp
  • src/PS1/runtime/MeshReconstructionStats.cpp
  • src/PS1/runtime/MeshReconstructionStats.h
  • src/PS1/runtime/MeshReconstructor.cpp
  • src/PS1/runtime/MeshReconstructor.h
  • src/PS1/runtime/PS1RipManager.cpp
  • src/PS1/runtime/PS1RipSessionWindow.cpp
  • src/PS1/runtime/PS1RipWorker.cpp
  • src/PS1/runtime/PsxHmdRamScanner.cpp
  • src/PS1/runtime/PsxHmdRamScanner.h
  • src/PS1/runtime/PsxHmdRamScanner_test.cpp
  • src/PS1/runtime/PsxModelRamScanCommon.h
  • src/PS1/runtime/PsxTmdRamScanner.cpp
  • src/PS1/runtime/PsxTmdRamScanner.h
  • src/PS1/runtime/PsxTmdRamScanner_test.cpp
  • src/PS1/runtime/ReconstructedMesh.h
  • src/PS1/runtime/RipperHooks.cpp
  • src/PS1/runtime/RipperHooks.h
  • tests/CMakeLists.txt

Comment thread src/PS1/PS1_RIP_DESIGN.md
Comment thread src/PS1/runtime/Gp0HookDispatch.cpp Outdated
Comment thread src/PS1/runtime/PsxModelRamScanCommon.h Outdated
…, shared bounds helper

Addresses three more code review findings on #674:

1) PsxModelRamScanCommon::fnv1a64 used a malformed 64-bit offset basis
   (1469598103934665603ULL — one digit short of canonical). Replaced
   with the standard FNV-1a 64-bit constants
   (0xcbf29ce484222325 / 0x100000001b3). Pre-existing contentHash values
   were never persisted across runs (in-session dedupe only) so the
   change is safe. Added Fnv1a64MatchesCanonicalReference test.

2) Gp0HookDispatch::captureFromSystemRam was calling promoteModelMeshSource
   while stats.ramTmdMeshes / ramHmdMeshes were still 0 — the inner
   merged-RAM scan can't see model meshes because the outer wrapper
   populates those counters AFTER this returns. Removed the no-op
   promotion from captureFromSystemRam and added a single
   promoteModelMeshSource call inside captureFrameFromSystemRam, AFTER
   stats.ramTmdMeshes is assigned. RamModelMesh can now actually win.
   Added CaptureFramePromotesPrimarySourceWhenTmdEmitted test.

3) MeshReconstructor.cpp had two near-identical bounds-update blocks
   (accumulateVertexStats and the snapshot.modelMeshes loop). Extracted
   `expandBounds(stats, vertex)` so both share the same hasBounds()
   anchor + min/max logic — keeps future stats tweaks in lock-step.

Also added blank lines around the per-core capability table in
PS1_RIP_DESIGN.md so it stops tripping markdownlint MD058.

Co-authored-by: Cursor <cursoragent@cursor.com>
@sonarqubecloud
Copy link
Copy Markdown

@fernandotonon fernandotonon merged commit 7ed1732 into master May 26, 2026
20 checks passed
@fernandotonon fernandotonon deleted the feat/ps1-tmd-hmd-scanner-674 branch May 26, 2026 06:44
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.

1 participant