quads chunk 3: importer quad detection (option A) - #329
Conversation
Adds the n-gon-aware re-import path discussed on #326. The existing AssimpToOgreImporter pipeline is untouched; this chunk only: 1. Caches the source file path on the imported Ogre::Mesh via UserObjectBindings("qtme.source_path"). MeshImporterExporter attaches it after every Assimp-backed import. 2. Adds EditableMesh::loadFromAssimpFile(path) which spins up an independent Assimp::Importer with aiProcess_Triangulate deliberately OFF, so source quads survive into aiMesh::mFaces and get recorded as EditableFace entries. Vertex attributes (positions/normals/UVs/colors/bone weights) are read out of the same scene; skeleton, animation, material, and tangent processing are skipped — Edit Mode operates on geometry only. Notes & deferred work: - Not yet wired into EditModeController::enterEditMode. Doing so safely needs a "user has modified this mesh" flag so re-importing doesn't discard prior edits — that's a follow-up chunk. - Re-import cost (~10–100ms typical, more on huge FBX) is acceptable as a one-time Tab-into-Edit-Mode cost. Tests (+6, standalone): - empty path / missing file rejected - OBJ quad → 1 EditableFace (4 verts), 2 fan triangles - triangle-only OBJ leaves `faces` empty (chunk-1 invariant) - mixed tri+quad OBJ produces both face types - non-empty mesh is replaced 197 standalone tests green (was 191; +6). Towards #326. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8865acb6a
ℹ️ 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".
| aiProcess_JoinIdenticalVertices | | ||
| aiProcess_GenSmoothNormals | | ||
| aiProcess_ValidateDataStructure | | ||
| aiProcess_LimitBoneWeights | | ||
| aiProcess_GlobalScale; |
There was a problem hiding this comment.
loadFromAssimpFile() uses a reduced flag set that omits the coordinate-system conversions used by the normal import path (notably left-handed conversion, and it also never applies the Z-up→Y-up bake done in MeshProcessor), so the editable vertex positions can be in a different basis than the live Ogre mesh. For non-.x assets and Z-up files, calling this path and then committing edits can write mirrored/rotated geometry back to the entity.
Useful? React with 👍 / 👎.
| const aiVertexWeight& vw = bone->mWeights[w]; | ||
| if (vw.mVertexId >= sub.vertices.size()) continue; | ||
| EditableBoneAssignment eba; | ||
| eba.boneIndex = static_cast<unsigned short>(b); |
There was a problem hiding this comment.
Bone assignments are recorded with boneIndex = b, which is only the mesh-local aiMesh::mBones array index, but downstream writeback (resizeEntityBuffers) treats EditableBoneAssignment::boneIndex as an Ogre skeleton handle. On skinned meshes where Assimp bone order differs from Ogre bone handles, this remaps vertex weights to the wrong bones and breaks deformation after topology updates.
Useful? React with 👍 / 👎.
|
1 participant
Summary
Chunk 3 of the quad migration (#326). Builds on chunks 1 (#327) and 2 (#328).
Adds the n-gon-aware re-import path agreed on the issue (option A): a new
EditableMesh::loadFromAssimpFile(path)reads the original source file directly with Assimp's triangulation flag DISABLED, so source quads survive intoEditableSubMesh::faces. The existingAssimpToOgreImporterpipeline is untouched.Changes
MeshImporterExporter::importercaches the source file path on the imported Ogre::Mesh viaUserObjectBindings("qtme.source_path"). Available everywhere the mesh is.EditableMesh::loadFromAssimpFile(path)spins up an independentAssimp::Importer, dropsaiProcess_Triangulate, walksaiMesh::mFacesand stores polygons (3/4/N) directly intoEditableFace. Triangulation mirror is built afterwards viatriangulateFaces()to keep the chunk-1 invariant. Skeleton/animation/material/tangent passes deliberately skipped — Edit Mode only needs geometry.Deliberately NOT done in this chunk
EditModeController::enterEditModeis NOT yet wired to use this path. Doing so safely requires modification-tracking (so re-importing doesn't blow away prior edits). That's a follow-up chunk; this one keeps the risk surface bounded by adding the new code path without changing any existing flows.Net user-visible behavior: zero change. Triangle-only assets behave identically; quad assets carry their polygon structure into the editor when (and only when) someone explicitly calls
loadFromAssimpFile.Tests (+6 standalone)
EditableFace(4 verts) + 2 fan triangles.facesempty (chunk-1 invariant).EditableMeshis replaced on subsequent loads.OBJ files written via
QFiletoQDir::tempPath()and removed after each test — no bundled binary assets.Test plan
Targeting
feat/quads-2-gpu-uploadStacked on chunk 2 (#328). When chunks 1+2 land on
feat/quads, this PR's base will auto-update.Towards #326.
🤖 Generated with Claude Code