feat(PS1/RSD): textured RSD import/export round-trip - #496
Merged
Conversation
Adds full texture support to the Sony Psy-Q RSD importer and exporter so
assets produced by the PlayStation-RSD Blender exporter (e.g. Wood.jpg
referenced by Example Project.rsd) round-trip with materials and UVs
preserved.
PS1MAT
- Promote MatEntry from a single QColor to a full material description:
shadingChar (F/S/G), typeChar (C/G/T/D/H), textured flag, textureIndex,
per-corner UVs (PS1 pixel coordinates), and 1-4 per-vertex colours.
- Rewrite parseMatFile and writeMatFile to handle every Psy-Q material
type with its correct payload (single colour C, smooth-shaded G, plain
texture T, textured-flat-colour D, textured-smooth-colour H), tolerating
the slight count variants tris/quads use in the wild.
PS1PLY
- Add importPsyqPlyWithFaceMaterials(): one submesh per textureIndex
bucket plus one for untextured faces, each with per-corner positions,
normals, UVs and vertex colours. Submesh materials are named
PLY/<mesh>_texN / PLY/<mesh>_solid so the RSD importer can rebind
Ogre textures.
- Extend exportPsyqPlyFromEntity() with an optional outFaceTextures sink
that captures per-face textured flag, source submesh index, corner count
and UVs. Textured submeshes skip the heuristic quad-merge so UVs stay
intact, while untextured paths keep existing behaviour.
MeshImporterExporter
- Load non-TIM RSD textures (JPG/PNG/BMP/...) via Ogre codecs with a
QImage fallback and stash dimensions so UVs can be normalised on import.
- When the parsed MAT exposes textured entries, build PS1PLY::FaceMaterial
per face and route imports through the new textured PLY path; after the
entity is created, bind the right Ogre texture to each _texN submesh.
- On RSD export, synthesise MAT entries (T for textured faces, C for
untextured), copy referenced texture images next to the .rsd, and emit
matching NTEX / TEX[] descriptor entries.
Tests
- New PS1MAT_test.cpp covering C/G/T/H parse paths plus a mixed
write-and-reread round trip.
- PS1RSD: new ParseBlenderExporterTextureLayout case for the NTEX +
external JPG layout produced by the Blender exporter.
- PS1PLY: new Ogre-backed tests
(ImportWithFaceMaterials_SplitsTexturedAndSolidIntoSubmeshes,
TexturedPlyRoundTrip_ExportRecoversPerFaceUvAndTextureFlag) exercising
the textured import + export sinks end-to-end.
- test_main.cpp: force QT_QPA_PLATFORM=xcb on Linux so Ogre's
externalWindowHandle path gets a valid X11 XID under Xvfb / Wayland
sessions — without this, UnitTests was failing locally with
"tryInitOgre() failed".
Manual verification with the user-supplied
'PlayStation-RSD-exporter-for-Blender-3.2.1/Example Project.rsd': qtmesh
info reports 2 submeshes (solid + tex0) with Wood.jpg in textures; qtmesh
convert round-trips the file and emits RoundTrip.rsd referencing
RoundTrip.{ply,mat} and a copied Wood.png.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR preserves per-face UVs and texture bindings across MAT/PLY/RSD: MAT entries gain typed fields (UVs, textureIndex, vertex colors), PLY gains textured import/export using per-face descriptors, RSD preloads and binds textures during import and emits synthesized MAT + PNG sidecars on export. ChangesTextured Material Support for PS1 PLY/MAT/RSD Import/Export
Sequence DiagramssequenceDiagram
participant User
participant MeshImporter as MeshImporterExporter
participant TexturePreloader as TexturePreloader
participant SlotTable as SlotTable(resource+dims)
participant PLYImporter as importPsyqPlyWithFaceMaterials
participant MaterialPatcher as MaterialPatcher
User->>MeshImporter: open .rsd (descriptor + .mat/.ply)
MeshImporter->>TexturePreloader: read rsd.textures list
TexturePreloader->>SlotTable: load textures (Ogre codec -> QImage fallback) and record dims
MeshImporter->>PLYImporter: provide per-face FaceMaterial[] (when MAT entries reference slots)
SlotTable-->>PLYImporter: supply texture dimensions for UV normalization
PLYImporter-->>MeshImporter: produce Ogre mesh (textured/untextured submeshes)
MeshImporter->>MaterialPatcher: bind preloaded textures to *_texN materials and recompile
MeshImporter->>User: return imported Ogre::Entity
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/MeshImporterExporter.cpp (1)
1538-1551: ⚡ Quick winAvoid re-decoding the texture file just to capture its dimensions.
loadExternalTextureForRsdalready decoded the image and uploaded it to Ogre'sTextureManager. Re-opening the same file viaQImage qprobe(texPath)re-runs the codec for the second time per texture (and a third time, since the helper itself can also pay a QImage decode internally on the fallback path). Query the just-loadedTexturePtrinstead — it has the dimensions on hand:♻️ Suggested change
- } else { - // Non-TIM raster format (PNG/JPG/BMP/TGA…) - QString extErr; - if (loadExternalTextureForRsd(texPath, ogreName, &extErr)) { - const QImage qprobe(texPath); - rsdTexSlots[ti].resourceName = resName; - rsdTexSlots[ti].width = qprobe.isNull() ? 0 : qprobe.width(); - rsdTexSlots[ti].height = qprobe.isNull() ? 0 : qprobe.height(); - } else { + } else { + // Non-TIM raster format (PNG/JPG/BMP/TGA…) + QString extErr; + if (loadExternalTextureForRsd(texPath, ogreName, &extErr)) { + rsdTexSlots[ti].resourceName = resName; + if (auto tex = Ogre::TextureManager::getSingleton().getByName( + ogreName, Ogre::ResourceGroupManager::DEFAULT_RESOURCE_GROUP_NAME)) { + rsdTexSlots[ti].width = static_cast<int>(tex->getWidth()); + rsdTexSlots[ti].height = static_cast<int>(tex->getHeight()); + } + } else {Alternatively, give
loadExternalTextureForRsdan(int* outW, int* outH)out-param that returns the dimensions from theOgre::Imageit already populated, so the dimensions don't depend on a secondTextureManager::getByName()round-trip either.🤖 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/MeshImporterExporter.cpp` around lines 1538 - 1551, The code re-opens the image file with QImage after calling loadExternalTextureForRsd, causing a redundant decode; instead retrieve the just-loaded Ogre::Texture (via Ogre::TextureManager::getByName or by returning it from loadExternalTextureForRsd) and read its width/height to populate rsdTexSlots[ti].width and .height (or modify loadExternalTextureForRsd to accept outW/outH or return image/texture and use those values), and remove the QImage qprobe(texPath) re-decode and stringly-constructed size logic.
🤖 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/MeshImporterExporter.cpp`:
- Around line 2225-2236: The QImage is being constructed assuming img data is
PF_BYTE_RGBA which can cause OOB reads or channel/alpha misordering; before
building QImage in MeshImporterExporter.cpp (around tex->convertToImage(img,
true) and the QImage(...) call) check img.getFormat() and if it is not
Ogre::PixelFormat::PF_BYTE_RGBA perform a bulk conversion to PF_BYTE_RGBA (using
Ogre::PixelUtil::bulkPixelConversion into a temporary Ogre::Image or buffer) so
the buffer layout is guaranteed, then use that converted data to create the
QImage and save the PNG; reference tex->convertToImage, Ogre::Image img,
img.getFormat(), Ogre::PixelUtil::bulkPixelConversion and ensure the constructed
QImage uses the converted PF_BYTE_RGBA buffer.
In `@src/PS1/PS1MAT.cpp`:
- Around line 234-236: Add Sentry breadcrumbs around the MAT file I/O: when
attempting to open the MAT file (the QFile f(matPath) / if (!f.open(...)) block)
call SentryReporter::addBreadcrumb("file.import", QStringLiteral("open MAT %1
failed").arg(matPath)) (include the error text or outError when available), and
also add a success breadcrumb on successful open like
SentryReporter::addBreadcrumb("file.import", QStringLiteral("open MAT %1
succeeded").arg(matPath)); likewise instrument the MAT write/export code path
referenced around the f.write / close block (the export block at lines ~289-291)
with SentryReporter::addBreadcrumb("file.export", ...) for both success and
failure cases, using the exact symbols QFile f(matPath), f.open, outError and
the export write functions to locate where to insert the calls.
In `@src/PS1/PS1PLY.cpp`:
- Around line 1459-1463: The file I/O blocks that open and read/write files
(e.g., the QFile f(filePath) read path and the other similar block around lines
2046-2054) are missing Sentry breadcrumbs; add
SentryReporter::addBreadcrumb(...) calls immediately before (or after a
successful) open/read and before a successful write/close using category
"file.import" for reads and "file.export" for writes, and include a descriptive
message containing the operation and the filePath (for example:
SentryReporter::addBreadcrumb("file.import", QString("Importing file:
%1").arg(filePath));). Ensure these breadcrumbs are placed in the same functions
that call QFile::open (and the function that calls the export write) so
readNonEmptyLines usage and the corresponding export routine are both
instrumented.
- Around line 1493-1494: The code coerces negative textureIndex into slot 0
causing wrong bindings; change the logic around fm.textured and textureIndex
used to compute submeshKey (the variable and expression using fm.textured,
fm.textureIndex, submeshKey and the call to getSoup) so that when fm.textured is
true but fm.textureIndex is < 0 you treat it as untextured (use -1) or fail
validation instead of mapping to 0; update any callers/consumers of
getSoup/TexturedSubmeshSoup accordingly and consider adding a debug/logging
check to surface invalid textureIndex values.
- Around line 2028-2039: The ngon export path is failing to set
PsyqExportFace::hasUv and UV fields, so when you later build ExportFaceTexture
from allExportFaces the textured flag and UVs are lost; fix by ensuring the ngon
branch populates PsyqExportFace::hasUv and its u/v_uv (or otherwise preserves
per-face UV state from the original source faces) when constructing
allExportFaces (used by outFaceTextures), so that in the ExportFaceTexture
creation (where you read ef.hasUv, ef.u, ef.v_uv) the correct textured/UV values
are present; adjust the code that handles useNgonExport to copy UV presence and
coordinates into each PsyqExportFace.
---
Nitpick comments:
In `@src/MeshImporterExporter.cpp`:
- Around line 1538-1551: The code re-opens the image file with QImage after
calling loadExternalTextureForRsd, causing a redundant decode; instead retrieve
the just-loaded Ogre::Texture (via Ogre::TextureManager::getByName or by
returning it from loadExternalTextureForRsd) and read its width/height to
populate rsdTexSlots[ti].width and .height (or modify loadExternalTextureForRsd
to accept outW/outH or return image/texture and use those values), and remove
the QImage qprobe(texPath) re-decode and stringly-constructed size logic.
🪄 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: b9425540-1472-41d3-aa61-805db44b3d92
📒 Files selected for processing (9)
src/MeshImporterExporter.cppsrc/PS1/PS1MAT.cppsrc/PS1/PS1MAT.hsrc/PS1/PS1MAT_test.cppsrc/PS1/PS1PLY.cppsrc/PS1/PS1PLY.hsrc/PS1/PS1PLY_test.cppsrc/PS1/PS1RSD_test.cppsrc/test_main.cpp
Correctness: - PS1PLY: reject negative textureIndex in importPsyqPlyWithFaceMaterials (route invalid index to untextured bucket instead of slot 0). - PS1PLY: preserve per-corner UVs in the Ngon export path so polygons and fanned triangles populate PsyqExportFace::hasUv + u/v_uv. - MeshImporterExporter: convert non-RGBA Ogre image data via PixelUtil::bulkPixelConversion before constructing the QImage used for texture PNG sidecars, and pass an explicit row stride. Nitpick: - MeshImporterExporter: read texture width/height from the loaded Ogre::TexturePtr instead of redecoding the source file with QImage. Instrumentation: - Add Sentry breadcrumbs (file.import / file.export) around PS1 MAT and PS1 PLY open/parse/write success and failure paths. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/MeshImporterExporter.cpp`:
- Around line 2151-2154: OutTex instances created in the texture export path
(see OutTex ot; ot.resourceName = QString::fromStdString(texName); ot.outFile =
ot.resourceName; and the Ogre::TextureManager::getSingleton().getByName(texName)
usage) set ot.outFile to the raw resource name which lets exists() short‑circuit
and leaves copied sidecars stale; change ot.outFile to a normalized basename
(strip path/dirs) and write the exported copy to that basename, explicitly
overwrite any existing sidecar or generate a unique renamed file before writing
so the export always refreshes the sidecar; apply the same change to the other
OutTex creation block with identical logic.
- Around line 1740-1747: The code in importPsyqPlyWithFaceMaterials()
unconditionally disables vertex colour tracking via
pass->setVertexColourTracking(Ogre::TVC_NONE), which wipes out the per-corner
colours you just imported; change this so vertex colours are preserved/modulate
the texture (e.g., use Ogre::TVC_DIFFUSE or Ogre::TVC_AMBIENT depending on your
intended modulation) instead of TVC_NONE, or conditionally set TVC_NONE only
when the mesh truly has no vertex colours—update the call to
pass->setVertexColourTracking(...) near the texture unit setup (and keep
mat->compile()) so textured faces that rely on vertex colour modulation render
correctly.
- Around line 1522-1535: resName is built from only the source filename so
different assets referencing the same basename (e.g., "Wood.jpg") collide in
Ogre's global texture registry; change the texture naming so resources are
scoped per-asset (e.g., include the asset ID, RSD path, or a stable hash of
texPath/parent asset) instead of just QFileInfo::completeBaseName(), then use
that scoped name for ogreName and all subsequent TextureManager calls
(resourceExists, remove, loadImage); ensure you no longer remove a
globally-named texture that might belong to another asset by deriving the unique
name in the resName construction and using that everywhere (symbols to update:
resName, texPath, ogreName,
Ogre::TextureManager::getSingleton().resourceExists/remove/loadImage).
- Around line 1491-1556: The per-texture RSD load loop (the for loop over ti
that uses resolve, PS1TIM::loadTimToOgreImage, and loadExternalTextureForRsd and
fills RsdTextureSlot) needs Sentry breadcrumbs for each significant action; add
SentryReporter::addBreadcrumb("file.import", "...") calls for: starting to
resolve/load a texture (include texPath or texRel), successful TIM load (after
loadTimToOgreImage and after creating tex via Ogre::TextureManager::loadImage),
successful external load (after loadExternalTextureForRsd and after fetching tex
via getByName), and failures (TIM load failed and external load failed, include
timErr/extErr), using the loop variable ti and resName in messages to uniquely
identify entries.
🪄 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: 7e983cb4-3e9d-4f1f-9150-fbd1c7ac10d3
📒 Files selected for processing (3)
src/MeshImporterExporter.cppsrc/PS1/PS1MAT.cppsrc/PS1/PS1PLY.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- src/PS1/PS1MAT.cpp
- src/PS1/PS1PLY.cpp
Correctness: - Scope preloaded RSD texture resources by absolute path. Adds scopedRsdResourceName() (hash-suffixed: `Wood__abcd1234.jpg`) so two RSDs that both ship a `Wood.jpg` don't collide in Ogre's global TextureManager registry. rsdResourceNameToBasename() recovers the clean basename on export. - Preserve per-corner vertex colours on textured submeshes after applying the RSD texture: only force TVC_NONE when the submesh has no VES_DIFFUSE stream; otherwise leave TVC_DIFFUSE so Psy-Q H/D/G materials still tint the texture. - Always refresh texture sidecars on export. Drops the QFileInfo::exists short-circuit (which silently left the .rsd referencing stale content from a previous export) and normalises ot.outFile via rsdResourceNameToBasename() so scoped resource names land as plain filenames next to the .rsd. Instrumentation: - Add per-texture Sentry breadcrumbs (file.import) for missing, TIM-loaded, externally-loaded, and failed cases inside the RSD texture preload loop. Verified with the Blender exporter Example Project.rsd: import + round- trip now generates Wood.png next to RoundTrip.rsd and re-import returns 2 submeshes / 110 triangles as expected. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
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/MeshImporterExporter.cpp`:
- Around line 1703-1735: The MatEntry values for texture usage are copied into
PS1PLY::FaceMaterial (fm) without validating that the textureIndex actually
points into rsdTexSlots, which lets a single invalid textured face cause
importPsyqPlyWithFaceMaterials(...) to fail; before assigning fm.textured and
fm.textureIndex (and before filling uv coords) validate me.textureIndex against
rsdTexSlots and if invalid set fm.textured = false and fm.textureIndex = -1 (and
avoid assigning u/v), then populate faceMats[fi] and call
importPsyqPlyWithFaceMaterials; preserve the existing fallback to
importPsyqPly/importPsyqPlyWithFaceColors if the textured importer rejects the
file.
- Around line 2327-2331: The code unconditionally assigns ot.outFile to the PNG
name even if QImage::save failed; change the logic in the texture export loop
around the qi.copy().save(png, "PNG") call to check its boolean return value and
only update ot.outFile = QFileInfo(png).fileName() when save(...) returns true
(otherwise leave ot.outFile unchanged and emit an error/warning via the existing
logging mechanism). Locate this in MeshImporterExporter.cpp where
qi.copy().save(png, "PNG") and ot.outFile are handled and perform the
conditional assignment and logging there.
🪄 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: 102ed635-1ab0-457e-beb5-cf59263bf05c
📒 Files selected for processing (1)
src/MeshImporterExporter.cpp
Correctness: - Sanitise per-face MAT texture references before calling the textured PLY importer: a single entry that claims to be textured but points at a missing slot now downgrades to solid (textureIndex=-1) for that face only, instead of forcing importPsyqPlyWithFaceMaterials() to reject the whole batch and drop textures on every face. - Verify QImage::save() succeeded before mutating ot.outFile. QImage signals write failure via the return value (no exception), so the previous unconditional update could leave the .rsd referencing a sidecar that was never written. Failures now log an Ogre error and emit a Sentry breadcrumb; ot.outFile keeps its previous basename so callers can fall back to the originally referenced texture. Co-authored-by: Cursor <cursoragent@cursor.com>
Blender's RSD exporter encodes baked AO / Gouraud shading as per-corner
vertex colours inside Psy-Q `G` (smooth) and `H` (textured smooth) MAT
entries -- e.g. a wall quad with corners `152 197 84 120` carries a soft
shadow gradient. Our previous exporter averaged the corners into a single
`avgRgbCorners()` colour per face and emitted only `C` (flat) or `T`
(textured-no-colour) entries, so re-imported meshes lost the gradient and
the walls rendered as flat grey instead of the smoothly-shaded original.
The fix plumbs per-corner colours through the PLY → MAT pipeline:
PS1/PS1PLY:
- `ExportFaceTexture` and `PsyqExportFace` gain `hasCornerColors` +
`cornerColors[4]` (matches the PLY corner order, zero-padded for tris).
- The NGON export path adds `setPsyqCornerColors()` next to the existing
`setPsyqUv()` helper -- populates each tri/quad/fanned-tri face.
- The non-NGON path collects per-tri-corner colours in `smCornerCols` and
threads them through `mergeSubmeshTrisToQuads`, which now resolves each
output quad corner from whichever source triangle touched the welded
position (`cornerColorForWeldedPos`). Single (non-merged) tris carry
their three corner colours straight through.
MeshImporterExporter:
- The MAT synthesis loop now picks the proper Psy-Q type from the corner-
colour variance:
textured + smooth corners -> `H` (per-corner tint)
textured + uniform corner -> `D` (flat colour)
textured + no colour -> `T`
untextured + smooth corners -> `G` (smooth Gouraud)
untextured + uniform -> `C`
- `shadingChar` stays `'F'` to match the Blender exporter convention -- PLY
stores per-face normals, so smoothness is encoded via typeChar only.
Verified end-to-end with `Example Project.rsd`: the round-trip MAT now
preserves all 12 H quads as 24 H tris and keeps 56 of the 50 G quads as
smooth G tris (the rest correctly degrade to C only when a single tri's
three corners happen to be uniform after the quad split).
Adds `PS1PLYOgreTest.ExportSurfacesPerCornerVertexColours` covering the
gradient-survival contract via the non-NGON heuristic-merge path.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
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/MeshImporterExporter.cpp`:
- Around line 2229-2241: The code strips asset-scoping via
rsdResourceNameToBasename(ot.resourceName) which can collapse distinct resources
to the same ot.outFile and cause rsdOutTextures slots to point to the same
sidecar; to fix, after computing ot.outFile but before pushing into
rsdOutTextures and resourceToSlot, detect collisions against existing
rsdOutTextures' outFile values and make the filename unique (e.g., append a
short discriminator derived from texName or a hash of ot.resourceName or the
slot index) so each ot.outFile remains unique; implement this uniqueness logic
where ot.outFile is set (around rsdResourceNameToBasename usage) and then
proceed to set width/height, compute slot, push_back to rsdOutTextures and
emplace resourceToSlot as before.
- Around line 2387-2399: The current try/catch around qi.copy().save(...)
swallows sidecar write failures (you set ot.outFile on success, log and
breadcrumb on failure) but still proceeds to emit the .rsd, leaving TEX[]
pointing to missing files; change the flow so a sidecar write failure aborts the
.rsd write: either rethrow the exception from the catch (or set and propagate an
export-failed flag) instead of ignoring it, and ensure ot.outFile is only set on
success; apply the same change to the other occurrence around the code handling
the second sidecar (the block referenced at lines ~2421-2424) so a failed
qi.copy().save(...) prevents emitting the .rsd and still records the
SentryReporter breadcrumb.
🪄 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: f4bb9236-cd4d-44ff-9595-d0a6535275ef
📒 Files selected for processing (4)
src/MeshImporterExporter.cppsrc/PS1/PS1PLY.cppsrc/PS1/PS1PLY.hsrc/PS1/PS1PLY_test.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/PS1/PS1PLY.cpp
Two CodeRabbit findings on the smooth-shading commit, both around the texture-sidecar write loop: 1. Output filename collision (`ot.outFile`). `rsdResourceNameToBasename()` strips the asset-scoping hash, so two distinct resources that share a source filename (e.g. two different `Wood.jpg` imports living in `Ogre::TextureManager` under unique `Wood__<hash>.jpg` names) collapse to the same `outFile`. The second sidecar overwrites the first and a later TEX[] slot ends up pointing at the wrong image. Now we check the candidate basename against already-claimed entries in `rsdOutTextures` and fall back to the scoped resource filename when a collision is detected — keeps the common case clean (no hash suffix) while staying correct. 2. `.rsd` was written even when the texture sidecar PNG failed to save. `QImage::save()` returns false (it does not throw); previously we logged + breadcrumbed but kept going, leaving TEX[] entries pointing at files that were never written. The export now sets a `sidecarWriteFailed` flag and returns -1 before emitting the `.rsd`. The catch block is upgraded the same way (and now logs/breadcrumbs instead of swallowing the exception). Verified the Example Project.rsd round-trip still succeeds end-to-end and all 26 PS1 tests still pass. Co-authored-by: Cursor <cursoragent@cursor.com>
…n round-trip Blender's RSD exporter writes G entries with 4 RGB triples regardless of face shape, padding triangles with a trailing `0 0 0`. Our import path treated those padded entries as size-mismatched (4 colours but 3-corner PLY tri) and fell back to a single flat colour, so original Gouraud tris showed up as faceted walls in the round-trip render. In parallel, Ogre triangulates every imported quad into two triangles and the post-export heuristic tri→quad merger failed to reconstruct adjacent coplanar quads (e.g. room walls), leaving visible diagonal seams in smooth-shaded surfaces. Fixes: - `importPsyqPlyWithFaceMaterials`: relax the corner-colour check from `==` to `>= pf.corners` so padded MAT entries still populate per-corner shading on tris (extra trailing triples are ignored). - Track parent-face provenance per emitted triangle in the textured submesh soup so `buildMeshFromTexturedSoups` can recover the original Psy-Q tri/quad polygons after welding and persist them as `qtme.faces.<i>` n-gon bindings (the same mechanism FBX/OBJ already use). - `exportPsyqPlyFromEntity`: lift the single-submesh restriction on the n-gon export path and run it per submesh when every submesh exposes a valid binding. The heuristic merger remains as a fallback for meshes imported through other paths. - `PS1MAT::writeMatFile`: pad tri G entries to 4 corners with a trailing `0 0 0` to match the Blender exporter / PS1 emulator convention. Verified: `Example Project.rsd` (24 tris + 43 quads, 5 C + 50 G + 12 H) round-trips to a multiset-identical MAT/PLY pair (zero entries differ). Co-authored-by: Cursor <cursoragent@cursor.com>
PS1 hardware and emulators only consume the native TIM texture format, so the previous PNG sidecar produced an .rsd that loaded fine in QtMeshEditor but failed on any real PS1 toolchain. Switch the RSD export path to always invoke `PS1TIM::saveOgreImageToTim16`, regardless of the source format (JPG/PNG/BMP/TGA/etc.) — the 16bpp BGR555 layout matches what the importer already decodes via `PS1TIM::loadTimToOgreImage`, so the round-trip stays self-consistent. The legacy PNG-writing branch is kept as a defensive fallback for the unlikely case the TIM encoder fails (e.g. zero-byte image, exception in pixel conversion), so the descriptor never references a missing file. Verified: `Example Project.rsd` (originally referencing `Wood.jpg`) exports as `TEX[0]=Wood.tim` with a valid 16bpp TIM payload, and the re-import → re-export loop preserves the same `Wood.tim` reference. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract three helpers to keep the new ngon-binding / TIM-sidecar paths
under SonarCloud's nesting (S134) and cognitive-complexity (S3776)
ceilings without changing behaviour:
- `recoverPolygonsFromProvenance` (PS1PLY.cpp): hoists the per-tri
parent-face recovery out of `buildMeshFromTexturedSoups`'s body so
the build loop only calls a single helper and pushes the resulting
`EditableSubMesh` into the binding list. Also switches the inner
unordered_map lookup from `find`/`emplace` to `try_emplace` (S6030)
and replaces the manual completeness loop with `std::none_of`.
- `ngonFacesValid` (PS1PLY.cpp): collapses the multi-submesh n-gon
pre-validation loop's nested `break`s into a single `std::all_of`
call, removing the S134/S924 violations the loop introduced.
- `writeSidecar` / `writePngFallback` (MeshImporterExporter.cpp): pull
the TIM-then-PNG sidecar logic out of the export loop so the latter
is just `try { ok = writeSidecar(ot); } catch (Ogre::Exception&) {}`
with a single break on failure. Also catches `Ogre::Exception`
specifically instead of `std::exception` (S1181) since that's the
only exception path through `TextureManager::getByName` /
`convertToImage` / `PixelUtil::bulkPixelConversion`.
The texture-slot collision check now uses `std::any_of` (S5566)
instead of a hand-rolled range-for loop.
Co-authored-by: Cursor <cursoragent@cursor.com>
- Make `sd` a `const SubData&` in the per-submesh ngon export loop (cpp:S5350) — the body never mutates the struct, only locks/unlocks buffers via its shared-ptr members. - Inline the texture-slot collision lambda directly into the `std::any_of` call (cpp:S6004) so the predicate's scope is bounded to the if statement that uses it. Co-authored-by: Cursor <cursoragent@cursor.com>
|
This was referenced May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds full texture support to the Sony Psy-Q RSD importer and exporter so assets produced by the PlayStation-RSD Blender exporter (e.g.
Wood.jpgreferenced byExample Project.rsd) round-trip with materials and UVs preserved.Highlights
MatEntrynow captures the full Psy-Q material description (shading char, type char, texture index, per-corner UVs, per-vertex colours). Parser/writer cover all material types:C(flat solid),G(smooth),T(textured),D(textured + flat colour),H(textured + smooth).importPsyqPlyWithFaceMaterials()builds one submesh per texture bucket (plus one for untextured faces) with per-corner UVs and vertex colours; submeshes are namedPLY/<mesh>_texN/PLY/<mesh>_solidso the RSD importer can rebind Ogre textures by parsing the suffix.exportPsyqPlyFromEntity()gained anoutFaceTexturessink that captures per-face textured flag + source submesh + corner UVs, and textured submeshes skip heuristic quad-merge to keep UVs intact._texNsubmesh after entity creation. Export synthesisesT/CMAT entries, copies referenced images next to the.rsd, and writesNTEX+TEX[]descriptor entries.QT_QPA_PLATFORM=xcbon Linux so Ogre'sexternalWindowHandlepath gets a valid X11 XID under Xvfb / Wayland sessions; without this,UnitTestswas failing locally withtryInitOgre() failed.Manual verification
Using the user-supplied
PlayStation-RSD-exporter-for-Blender-3.2.1/Example Project.rsd:```
$ qtmesh info "Example Project.rsd" --json
{
"materials": ["PLY/Example Project_rsd_ply_solid", "PLY/Example Project_rsd_ply_tex0"],
"submeshes": 2,
"textures": ["Wood.jpg"],
"triangles": 110,
"vertices": 160,
...
}
$ qtmesh convert "Example Project.rsd" -o "RoundTrip.rsd"
Writes RoundTrip.{rsd,ply,mat} + Wood.png next to it.
```
Tests added
PS1MAT.ParseFlatColorEntries/ParseSmoothColorQuadEntries/ParseTexturedQuadEntry_HType/ParseTexturedTriEntry_TType/WriteAndRoundTrip_MixedEntries/RejectsMissingHeader— full parser/writer coverage for the new MAT types.PS1RSD.ParseBlenderExporterTextureLayout— RSD parser acceptsNTEX+ external image references (Wood.jpg).PS1PLY.ImportWithFaceMaterials_SplitsTexturedAndSolidIntoSubmeshes— verifies multi-submesh layout with UV element only on textured submeshes.PS1PLY.TexturedPlyRoundTrip_ExportRecoversPerFaceUvAndTextureFlag— end-to-end: import textured+solid mesh, export, confirm per-face UV envelope and submesh routing survive.Full focused-suite run (
PS1*:MeshImporterExporter*:ScanEngine*): 164 / 164 passing underxvfb-run.Test plan
Q_OS_LINUX).qtmesh info+qtmesh convertonExample Project.rsdround-trip cleanly (manual smoke).Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Tests