Skip to content

feat(PS1): Psy-Q PLY quad topology + qtmesh scan and import dialog#478

Merged
fernandotonon merged 13 commits into
masterfrom
feature/ps1-ply-quad-weld-roundtrip
May 10, 2026
Merged

feat(PS1): Psy-Q PLY quad topology + qtmesh scan and import dialog#478
fernandotonon merged 13 commits into
masterfrom
feature/ps1-ply-quad-weld-roundtrip

Conversation

@fernandotonon
Copy link
Copy Markdown
Owner

@fernandotonon fernandotonon commented May 10, 2026

Summary

This branch extends Psy-Q PLY import/export (quad/ngon metadata and split vertex/normal pools for welding) and fixes qtmesh scan and file pickers around PlayStation assets.

Scan (qtmesh scan)

  • Inspect PlayStation TMD, Psy-Q PLY (not Stanford PLY), and RSD sidecars by resolving PLY= and using the same Ogre importers as the editor (headless render target + materials).
  • Default include globs now always contain **/*.tmd and **/*.rsd** (Assimp does not register those extensions).
  • qtmesh.yml with explicit scan.include: after loading YAML/JSON, missing **/*.tmd, **/*.rsd, **/*.ply** patterns are merged so local configs (like this repo’s qtmesh.yml) no longer exclude PS1 files when scanning a folder such as ./TMD.
  • Repo qtmesh.yml lists TMD/RSD/PLY includes for clarity.

UI

  • Welcome startup dialog and File → Import use MeshImporterExporter::importFileDialogFilter() / importFileDialogFilterFromExtensionList(Manager::defaultImportExtensions()) so filters stay aligned before MainWindow exists.

Tests

  • ScanEngine_test: RSD→OBJ inspect, minimal TMD / Psy-Q PLY inspect (Ogre), YAML include merge, default globs.
  • ScanConfig_test, Manager_test, MeshImporterExporter_test: filter and extension helpers.

Notes

  • Scanning static TMD/RSD with the repo qtmesh.yml may still report rule errors (require_skeleton / require_animations) because those assets have no skeleton; files are still enumerated and inspected.

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Added support for PlayStation/Psy-Q asset formats (.tmd, .rsd, .ply) in file dialogs and asset scanning.
    • Enabled RSD file inspection with automatic geometry resolution.
  • Improvements

    • Enhanced quad-merging logic during PS1 PLY export for improved geometry handling.
    • File dialogs now include all newly supported formats in their filters.
  • Tests

    • Added comprehensive test coverage for new PlayStation format support and asset inspection workflows.

Review Change Stack

fernandotonon and others added 7 commits May 9, 2026 16:46
…export

- buildMeshFromTriSoup: indexed mesh with welded corners (quantized pos/normal/colour)
- exportPsyqPlyFromEntity: per-submesh quad merge (shared directed edge, normal agreement)
  with Psy-Q quad face lines; MAT face colours only if all exported faces have colour data
- Remove duplicate WeldKey/quantize definitions after moving welding keys earlier

Co-authored-by: Cursor <cursoragent@cursor.com>
- Record per-face vertex count (3 or 4) when parsing PLY; store blob on mesh
  under kPsyqPlyFaceLayoutUserKey after import
- Export uses stored layout on single-submesh meshes when triangle count still
  matches; emit type-1 quad lines without heuristic merge; fall back if winding
  no longer matches
- Document that Ogre still renders triangle lists; quads are not n-gons in GPU

Co-authored-by: Cursor <cursoragent@cursor.com>
- On import, rebuild polygon lists from Psy-Q face layout and welded
  triangle indices, then writeNgonFacesToMesh (same qtme.faces.<i>
  binding as FBX) so Edit Mode and export see true quads.
- Recover quad corner order as PS1/TMD split (v0,v1,v2)+(v1,v2,v3)
  after per-triangle winding flips, instead of sorting corners by angle.
- Export prefers readNgonFacesFromMesh for single-submesh meshes; fan
  n-gons to Psy-Q triangles; fall back to heuristic quad merge otherwise.
- readNgonFacesFromMesh: catch std::bad_cast on wrong Any payload.
- Drop the separate Psy-Q face-layout UserAny blob; qtme.faces is canonical.

Co-authored-by: Cursor <cursoragent@cursor.com>
- Weld positions and normals in separate tables (PosWeldKey / NrmWeldKey);
  header line uses nV and nN independently like original Psy-Q files.
- Face lines emit distinct vertex and normal index tuples (tri and quad).
- Heuristic quad merge carries parallel normal indices; merged quads pick
  per-corner normals from the source triangle that owns each welded position.
- Import-time corner weld still uses combined pos+normal+colour (unchanged).

Co-authored-by: Cursor <cursoragent@cursor.com>
- Add documentation/playstation-rsd-ply.md (RSD descriptor, PLY/MAT, qtme.faces vs quad merge, normal pool welding)
- README + DocsApp: PlayStation RSD, clarify .ply dispatch, link to doc
- PS1PLY_test: Ogre fixture tests for heuristic quad merge and quad import re-export
- WelcomeDialog: include .ply and .rsd in quick-open filter

Co-authored-by: Cursor <cursoragent@cursor.com>
- ScanEngine: inspect TMD, Psy-Q PLY, and RSD-referenced geometry via Ogre
  headless context; default Assimp include set adds tmd/rsd extensions.
- ScanConfig: merge **/*.tmd, **/*.rsd, **/*.ply into explicit scan.include
  from qtmesh.yml so local configs do not drop PlayStation assets.
- qtmesh.yml: document PlayStation include patterns for repo scans.
- WelcomeDialog + File→Import: use MeshImporterExporter import filter helper
  and Manager::defaultImportExtensions() so startup dialog matches Import.
- Tests: ScanEngine (RSD/TMD/Psyq PLY), ScanConfig include merge, Manager,
  MeshImporterExporter filter builder.

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

coderabbitai Bot commented May 10, 2026

Warning

Rate limit exceeded

@fernandotonon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 32 minutes and 53 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cf55530-c57f-4886-bf4a-4fb7567db686

📥 Commits

Reviewing files that changed from the base of the PR and between f52279e and a40225e.

📒 Files selected for processing (1)
  • src/ScanEngine.cpp
📝 Walkthrough

Walkthrough

PR expands PlayStation/Psy-Q mesh format support (.tmd, .rsd, .ply) by introducing new extension/filter accessor APIs, implementing headless Ogre-based asset inspection including RSD descriptor parsing, refining PS1 PLY quad-merge validation, and providing comprehensive test coverage and infrastructure.

Changes

PlayStation Format Support & Asset Scanning

Layer / File(s) Summary
Extension & Filter API Contracts
src/Manager.h, src/Manager.cpp, src/MeshImporterExporter.h, src/MeshImporterExporter.cpp
Manager::defaultImportExtensions() returns space-separated valid extensions; MeshImporterExporter::importFileDialogFilterFromExtensionList() generates file-dialog filters from extension lists.
UI File-Dialog Integration
src/WelcomeDialog.cpp, src/mainwindow.cpp
Import dialogs now use MeshImporterExporter::importFileDialogFilter() and defaultImportExtensions() instead of hardcoded extension lists.
Scan Configuration & Format Inclusion
src/ScanConfig.h, src/ScanConfig.cpp
Default constructor includes PlayStation .tmd/.rsd globs alongside Assimp/Ogre extensions; scan.include overrides automatically gain PlayStation globs when missing via appendEditorOnlyMeshScanGlobsIfMissing().
Asset Scanning & RSD Inspection
src/ScanEngine.h, src/ScanEngine.cpp
Adds headless Ogre-based inspection for .tmd/.ply; RSD descriptor parser resolves geometry file path; Ogre mesh inspection populates AssetInfo vertex/face/material counts; test-only testApplyOgreMeshInspectCounts() helper.

PS1 PLY Export Quad-Merge Validation

Layer / File(s) Summary
Welded Normal Validation in Quad Merge
src/PS1/PS1PLY.cpp
Introduces PsyqWeldedTri structure; refactored normalIndexForWeldedPos() signature to check welded normal agreement on interior edges before allowing quad merges.
PS1 PLY Test Mesh Builders
src/PS1/PS1PLY_test.cpp
New createInterleavedPosNormalMesh() builds interleaved position/normal Ogre meshes; createTwoTriQuadMesh() refactored to use shared builder; createSplitNormalTwoTriMesh() creates coplanar pairs with disagreeing normals.
Quad-Merge Validation Test
src/PS1/PS1PLY_test.cpp
Test ExportHeuristicSkipsQuadMergeWhenSharedEdgeNormalsDisagree asserts PS1 PLY export preserves 2 faces when shared-edge normals disagree.

Test Infrastructure & Fixtures

Layer / File(s) Summary
CMake Unit Test Configuration
src/CMakeLists.txt
UnitTests target now defines QTMESH_UNIT_TESTS preprocessor macro alongside BATCH_EXPORTER_TEST_SEAM.
Generalized Ogre Mesh Test Builders
src/TestHelpers.h
New createInMemoryMeshSharedVertsPlusLocalSubmesh() builds Ogre meshes with shared vertex pool on submesh 0 and non-shared local pool on submesh 1.
Fixture Consolidation
src/commands/TransformCommands_test.cpp
Removed local createInMemoryTwoSubMeshMesh; SubMeshTransformCommand test now uses shared createInMemoryMeshSharedVertsPlusLocalSubmesh() helper.

API & Integration Tests

Layer / File(s) Summary
Manager Extension Accessor Test
src/Manager_test.cpp
DefaultImportExtensions_IncludesPlayStationFormats asserts Manager::defaultImportExtensions() contains .tmd, .rsd, .ply.
Filter Helper Function Test
src/MeshImporterExporter_test.cpp
ImportFileDialogFilterFromExtensionList_BuildsRows validates filter format includes PlayStation rows and correct suffix for given extension list.
ScanConfig Inclusion & Default Tests
src/ScanEngine_test.cpp
Three tests verify explicit YAML scan.include gains PlayStation globs, avoids duplicates, and defaults include TMD/RSD patterns.
Asset Inspection Integration Tests
src/ScanEngine_test.cpp
Four tests validate RSD→OBJ resolution, minimal TMD binary loading, Ogre mesh inspection with shared/local submesh counting, and PSX PLY geometry parsing; test helpers for little-endian TMD blob generation.

CI Configuration

Layer / File(s) Summary
Asset Validation Scan Patterns
qtmesh.yml
Added *.tmd, *.rsd, *.ply globs to scan.include list for PlayStation/Psy-Q asset validation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • fernandotonon/QtMeshEditor#454: Directly related—both add PlayStation PS1 support (PS1 PLY/RSD/MAT), extend Manager valid extensions, modify MeshImporterExporter import-file-filter behavior, and introduce overlapping PS1 import/export modules.
  • fernandotonon/QtMeshEditor#289: Related through ScanConfig's scan.include/default include-glob behavior—both modify how default globs are generated and how explicit overrides are handled.
  • fernandotonon/QtMeshEditor#394: Related through PlayStation format support expansion—both modify Manager's extension list, MeshImporterExporter file-dialog filters, and WelcomeDialog import handling.

Poem

🐰 Extend, refine, and scan with care,
PlayStation formats now beyond compare,
Normals welded, quads merged true,
RSD descriptors—geometry through and through!
From TMD to PLY, the mesh files dance,
Test helpers march in perfectly matched stance. 🎨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.41% 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 clearly summarizes the main changes: extending Psy-Q PLY support (quad topology) and adding PS1 asset support to qtmesh scan and import dialogs.
Description check ✅ Passed The description covers the required template sections with comprehensive technical details about scan improvements, UI changes, and tests, matching the scope and features of this PR.
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 feature/ps1-ply-quad-weld-roundtrip

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.

Co-authored-by: Cursor <cursoragent@cursor.com>
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: 1ddc725c98

ℹ️ 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/PS1PLY.cpp Outdated
Comment thread src/ScanEngine.cpp Outdated
fernandotonon and others added 4 commits May 10, 2026 00:08
Co-authored-by: Cursor <cursoragent@cursor.com>
…skip quad merge on split normals

- fillAssetInfoFromOgreMesh adds vertexData for submeshes with useSharedVertices=false when sharedVertexData exists (CodeRabbit P2).
- Heuristic tri→quad merge requires matching welded normal indices on the shared interior edge (CodeRabbit P1).
- Tests: ScanEngine Ogre inspect count helper (QTMESH_UNIT_TESTS), PS1PLY export split-edge case; TestHelpers mesh factory.

Co-authored-by: Cursor <cursoragent@cursor.com>
- PS1PLY tests: single createInterleavedPosNormalMesh builder for two-tri fixtures.
- TransformCommands_test: reuse TestHelpers createInMemoryMeshSharedVertsPlusLocalSubmesh.

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

- PS1PLY: PsyqWeldedTri bundles corners; drops 14-arg function and redundant static.
- TestHelpers shared+local mesh: constexpr std::array buffers; NOSONAR for Ogre-owned VertexData.

Co-authored-by: Cursor <cursoragent@cursor.com>
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/WelcomeDialog.cpp (1)

71-75: ⚡ Quick win

Log the click before opening the dialog.

Right now the ui.action breadcrumb is only added after a file is picked, so cancelled opens disappear from telemetry. Add the breadcrumb before QFileDialog::getOpenFileName(...).

Suggested change
     connect(openFileBtn, &QPushButton::clicked, this, [this]() {
+        SentryReporter::addBreadcrumb("ui.action", "Welcome: Open File");
         const QString filter = MeshImporterExporter::importFileDialogFilterFromExtensionList(
             Manager::defaultImportExtensions());
         QString file = QFileDialog::getOpenFileName(
             this, tr("Open 3D File"), QString(), filter, nullptr,
             QFileDialog::DontUseNativeDialog | QFileDialog::HideNameFilterDetails);

As per coding guidelines, "Track user-facing actions and significant operations with SentryReporter::addBreadcrumb(category, message). Use \"ui.action\" for toolbar/menu clicks, \"ai.tool_call\" for MCP tool invocations, \"file.import\"/\"file.export\" for I/O operations."

🤖 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/WelcomeDialog.cpp` around lines 71 - 75, The code currently calls
QFileDialog::getOpenFileName(...) and only adds the ui.action breadcrumb after a
file is chosen, so cancelled opens aren't logged; before calling
QFileDialog::getOpenFileName in WelcomeDialog (the block that builds filter via
MeshImporterExporter::importFileDialogFilterFromExtensionList and uses
Manager::defaultImportExtensions()), call
SentryReporter::addBreadcrumb("ui.action", "Open 3D File") to log the
toolbar/menu click immediately so the action is recorded even if the dialog is
cancelled.
🤖 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/ScanConfig.cpp`:
- Around line 313-320: The lambda hasExtensionGlob currently treats any
substring match as a hit (using p.contains(token)), which incorrectly matches
things like "**/*.ply2"; update hasExtensionGlob to only accept true glob
patterns for the extension by performing an exact glob check against each
pattern instead of substring matching — e.g. check if a pattern equals "*.ext"
or ends with a path-separator + "*.ext" (or use a QRegularExpression anchored to
the end like "(?:.*/|\\*\\*/)?\\*\\.ext$") so that only exact "*.ext" globs
(possibly prefixed by directories or "**/") are considered present; modify the
implementation in hasExtensionGlob and iterate over patterns accordingly.

In `@src/ScanEngine.cpp`:
- Around line 642-666: The code restores info.filePath back to the .rsd after
calling ScanEngine::inspectAsset (using geomPath) which causes
require_textures_exist to resolve relative texture paths against the .rsd
instead of the referenced geometry; to fix, stop overwriting info.filePath with
the .rsd path—either leave info.filePath as the inspected asset path (use
inner.filePath or geomPath) or move the assignment so it happens only after
texture existence checks; update relativePath/format/fileSize likewise to
reflect the inspected asset (inner) so require_textures_exist resolves relative
textures correctly.
- Around line 157-205: The code currently sets info.materialCount =
mesh->getNumSubMeshes(), which overcounts when multiple submeshes share the same
material; modify fillAssetInfoFromOgreMesh to compute unique materials instead:
collect non-empty material names from each SubMesh (use sm->getMaterialName())
into a temporary set or deduplicate info.materialNames, then set
info.materialCount to the size of that unique collection (and ensure
info.materialNames only contains unique names if you want them de-duplicated).
Update/remove the hard-coded assignment of mesh->getNumSubMeshes() and compute
the value after iterating submeshes.

---

Nitpick comments:
In `@src/WelcomeDialog.cpp`:
- Around line 71-75: The code currently calls QFileDialog::getOpenFileName(...)
and only adds the ui.action breadcrumb after a file is chosen, so cancelled
opens aren't logged; before calling QFileDialog::getOpenFileName in
WelcomeDialog (the block that builds filter via
MeshImporterExporter::importFileDialogFilterFromExtensionList and uses
Manager::defaultImportExtensions()), call
SentryReporter::addBreadcrumb("ui.action", "Open 3D File") to log the
toolbar/menu click immediately so the action is recorded even if the dialog is
cancelled.
🪄 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: e4c00c6b-0f6f-4223-b1cc-7236b1902b55

📥 Commits

Reviewing files that changed from the base of the PR and between 2a9563d and f52279e.

📒 Files selected for processing (19)
  • qtmesh.yml
  • src/CMakeLists.txt
  • src/Manager.cpp
  • src/Manager.h
  • src/Manager_test.cpp
  • src/MeshImporterExporter.cpp
  • src/MeshImporterExporter.h
  • src/MeshImporterExporter_test.cpp
  • src/PS1/PS1PLY.cpp
  • src/PS1/PS1PLY_test.cpp
  • src/ScanConfig.cpp
  • src/ScanConfig.h
  • src/ScanEngine.cpp
  • src/ScanEngine.h
  • src/ScanEngine_test.cpp
  • src/TestHelpers.h
  • src/WelcomeDialog.cpp
  • src/commands/TransformCommands_test.cpp
  • src/mainwindow.cpp

Comment thread src/ScanConfig.cpp
Comment thread src/ScanEngine.cpp
Comment thread src/ScanEngine.cpp
… scan mesh id

- Replace std::function import callback with a template (Sonar cpp:S5213).
- nextScanInspectMeshId() replaces mutable file-scope atomic.
- RSD/TMD/PLY inspect lambdas capture paths explicitly; const SubMesh pointers in counts.

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

@fernandotonon fernandotonon merged commit b5ba99a into master May 10, 2026
19 checks passed
@fernandotonon fernandotonon deleted the feature/ps1-ply-quad-weld-roundtrip branch May 10, 2026 06:49
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.

Research: PS1 TIM / TMD / RSD feasibility for QtMesh (validation vs import/export)

1 participant