PS1/RSD: smooth vertex colours and MAT lit/unlit support

[fernandotonon]

Summary

Fixes PlayStation RSD/PLY imports where Psy-Q G / per-corner colours looked flat in QtMeshEditor while Blender showed smooth blending, and preserves the Blender/Psy-Q MAT lit vs unlit flag through RSD import/export.

Root cause

1. Lighting vs vertex colours: Lit FFP passes modulated vertex diffuse by N·L, which could make corner colour gradients read almost flat on simple shapes.
2. RSD + MAT routing: importPsyqPlyWithFaceMaterials previously only ran when a MAT sidecar referenced at least one valid texture. NTEX=0 assets fell back to face-colour import and dropped extra Gouraud corner colours.
3. MAT flag handling: The packed integer after the face index in Blender-exported MAT rows was not being parsed or written, so the MAT no-light / unlit bit was lost on round-trip.

What changed

• Untextured Psy-Q PLY materials still use the face-material path whenever parsed MAT rows exist, so all Gouraud corner colours survive import.
• MAT parsing/writing now preserves the Blender exporter flag integer and maps its least significant bit to MatEntry::unlit.
• RSD import carries unlit into FaceMaterial, splits textured/untextured Psy-Q submeshes by both texture slot and lighting mode, and names unlit material buckets with _nl.
• RSD texture rebinding now recognizes _texN_nl materials and configures Ogre passes from both vertex-colour presence and the MAT unlit flag.
• RSD export derives the MAT no-light bit back from imported Psy-Q material naming / pass state so lit-unlit intent survives round-trip.

Files

• src/MeshImporterExporter.cpp
• src/PS1/PS1MAT.cpp
• src/PS1/PS1MAT.h
• src/PS1/PS1MAT_test.cpp
• src/PS1/PS1PLY.cpp
• src/PS1/PS1PLY.h

Testing

• QT_QPA_PLATFORM=offscreen ./bin/UnitTests --gtest_filter='PS1*'
  • Existing local UnitTests binary was stale and matched 0 tests.
• QT_QPA_PLATFORM=offscreen ./bin/UnitTests --gtest_list_tests
  • Confirmed the existing binary did not include the PS1MAT suite yet.
• cmake --build . --target UnitTests -j2
  • Rebuild started locally to refresh the test binary.

Made with Cursor

Summary by CodeRabbit

• New Features

  • Preserve an exported/imported "unlit" material flag so unlit submeshes round-trip.
  • Material pass setup now respects presence of vertex colours and the unlit flag for correct lighting and vertex-colour behavior.
  • Per-face material grouping/import now accounts for unlit state.

• Bug Fixes

  • Improved import fallback when per-face material import produces no mesh.

• Tests

  • Updated and added MAT parsing/serialization tests covering the unlit flag.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a per-face "unlit" MAT flag and tests; centralizes Ogre pass setup via PS1PLY::configurePsyqRsdMaterialPass; propagates unlit state through PLY import/export, textured submesh rebinds, and buckets textured faces by texture+unlit.

Changes

PS1 PLY unlit flag, pass helper, import/export wiring

Layer / File(s)	Summary
MAT flag parsing, encoding, and tests src/PS1/PS1MAT.cpp, src/PS1/PS1MAT.h, src/PS1/PS1MAT_test.cpp	Add MatEntry::unlit; parse optional packed <flag> (LSB = unlit); encode Blender-compatible flag bits on write; update tests/fixtures and expectations accordingly.
Ogre pass configuration helper src/PS1/PS1PLY.cpp, src/PS1/PS1PLY.h	Add PS1PLY::configurePsyqRsdMaterialPass(Ogre::Pass*, bool hasVertexColour, bool matUnlit) to set lighting, ambient/diffuse/emissive/specular, and TVC tracking based on vertex colours and unlit.
Mesh builders adopt helper and preserve unlit src/PS1/PS1PLY.cpp	Replace inline pass setup in buildMeshFromTriSoup and buildMeshFromTexturedSoups with the helper; add TexturedSubmeshSoup::unlit and append _nl to cloned material names when unlit.
PLY import: face-material bucketing by texture+unlit src/PS1/PS1PLY.cpp	Refactor importPsyqPlyWithFaceMaterials to bucket faces by texture index and unlit, add importPsyqPlyWithFaceMaterialsImpl(...), and set FaceMaterial.unlit usage in bucketing/assignment.
MeshImporterExporter rebind & export unlit propagation src/MeshImporterExporter.cpp	Switch PLY import routing to use useFaceMaterialsImport (presence of MAT entries); when rebinding _texN/_texN_nl materials detect _nl and whether the target submesh has VES_DIFFUSE, call configurePsyqRsdMaterialPass(pass, hasVC, slotUnlit); record submeshUnlit during export and set me.unlit for synthesized per-face MAT entries.
Public header updates src/PS1/PS1PLY.h, src/PS1/PS1MAT.h	Forward-declare Ogre::Pass, declare the new helper, document packed MAT flag semantics, and add unlit fields to public structs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fernandotonon/QtMeshEditor#496: Modifies the same _texN textured material binding path in MeshImporterExporter.cpp to conditionally preserve vertex-color tracking based on submesh content.
• fernandotonon/QtMeshEditor#454: Earlier RSD and Psy-Q PLY pipeline refactor that established import/export patterns for PS1 MAT/PLY handling referenced here.

Poem

🐰 I found a tiny flag, a single-bit so bright,
It tells the shader "sleep" and turns the lighting light.
I tucked a pass into a helper neat and small,
Sorted faces by their texture and their pall,
Then hopped to tests that pass — a carrot for us all!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% 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 and concisely summarizes the main changes: smooth vertex colors and MAT lit/unlit support for PS1/RSD, matching the primary objectives described in the PR description.
Description check	✅ Passed	The PR description is comprehensive, providing a clear summary of the problem, root causes, changes made, affected files, and testing approach. All required template sections are present and well-populated.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rsd-vertex-color-smooth-shading

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 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/PS1MAT.cpp`:
- Around line 345-346: The code currently collapses both 'G' and 'S'
smooth-shading tokens into 'S', breaking round-trip fidelity; change the shading
calculation to preserve e.shadingChar when it is 'G' or 'S' (e.g. set shading =
e.shadingChar for valid tokens, otherwise 'F') so the output written by the ts
<< ... line emits the original 'G' or 'S' token instead of always 'S'.

In `@src/PS1/PS1PLY.cpp`:
- Around line 544-545: The call in PS1PLY.cpp hard-codes matUnlit=true when
invoking PS1PLY::configurePsyqRsdMaterialPass(p0, haveColors, true), which
invents an unlit/MAT bit for MAT-less PLY imports; change the third argument so
we do not synthesize unlit: compute the unlit flag from the actual pass lighting
state (e.g. use !p0->getLightingEnabled() or otherwise preserve the source
lighting) or simply pass false for MAT-less/face-colour imports, rather than
hard-coding true, so configurePsyqRsdMaterialPass only marks unlit when the
original material/pass indicates it.

🪄 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: e105d4db-a7d8-46ad-8bed-37de79b30c0c

📥 Commits

Reviewing files that changed from the base of the PR and between 354d520 and b753bdc.

📒 Files selected for processing (6)

• src/MeshImporterExporter.cpp
• src/PS1/PS1MAT.cpp
• src/PS1/PS1MAT.h
• src/PS1/PS1MAT_test.cpp
• src/PS1/PS1PLY.cpp
• src/PS1/PS1PLY.h

[coderabbitai]

🧹 Nitpick comments (1)

src/PS1/PS1MAT_test.cpp (1)

112-132: ⚡ Quick win

Add explicit unlit assertions for textured parse fixtures.

Line 112 and Line 143 now encode flag=0, but these tests don’t assert e.unlit == false. Adding that check in both tests will lock the textured parse path to the new flag semantics.

Proposed test additions

@@ TEST(PS1MAT, ParseTexturedQuadEntry_HType)
     ASSERT_EQ(e.vertColors.size(), 4);
     EXPECT_EQ(e.vertColors[0], QColor(72, 72, 72));
     EXPECT_EQ(e.vertColors[3], QColor(80, 80, 80));
+    EXPECT_FALSE(e.unlit);
@@ TEST(PS1MAT, ParseTexturedTriEntry_TType)
     EXPECT_EQ(e.uvs[2].u, 50); EXPECT_EQ(e.uvs[2].v, 60);
     EXPECT_TRUE(e.vertColors.isEmpty());
+    EXPECT_FALSE(e.unlit);

Also applies to: 143-160

🤖 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/PS1MAT_test.cpp` around lines 112 - 132, The textured-parse tests
that call PS1MAT::parseMatFile and inspect the first MatEntry (e) must also
assert the new flag semantics by verifying the unlit flag is false; update both
relevant test cases (the one that checks e.textured == true and the other
similar textured fixture) to include EXPECT_FALSE(e.unlit) or EXPECT_EQ(e.unlit,
false) after obtaining const auto& e so the textured parse path is locked to the
new flag 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.

Nitpick comments:
In `@src/PS1/PS1MAT_test.cpp`:
- Around line 112-132: The textured-parse tests that call PS1MAT::parseMatFile
and inspect the first MatEntry (e) must also assert the new flag semantics by
verifying the unlit flag is false; update both relevant test cases (the one that
checks e.textured == true and the other similar textured fixture) to include
EXPECT_FALSE(e.unlit) or EXPECT_EQ(e.unlit, false) after obtaining const auto& e
so the textured parse path is locked to the new flag behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a4406dbd-9714-42da-b7f9-557ca1a2b706

📥 Commits

Reviewing files that changed from the base of the PR and between b753bdc and 00f926d.

📒 Files selected for processing (3)

• src/PS1/PS1MAT.cpp
• src/PS1/PS1MAT_test.cpp
• src/PS1/PS1PLY.cpp

🚧 Files skipped from review as they are similar to previous changes (2)

• src/PS1/PS1MAT.cpp
• src/PS1/PS1PLY.cpp

[sonarqubecloud]

Quality Gate passed

Issues
15 New issues
0 Accepted issues

Measures
0 Security Hotspots
39.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud