quads chunk 4: wire enterEditMode to n-gon import - #332
Conversation
* fix(scan): harden scan --fix and improve reporting Fix scan --fix crashes in Ogre/Assimp paths, prevent FBX simplify from bloating files, and enrich scan output with [fixed]/[skipped] tags plus saved-bytes and keys-removed summaries. Default viewport FSAA to 0 when unset and add a regression test. Made-with: Cursor * fix(tests): accept compacted FBX animation curves FBX animation curve channels may be emitted with a single key when the channel is constant. Relax the AnimationCurves test accordingly. Also replace a `strlen`-based suffix length with `std::string_view` sizing for Sonar. Made-with: Cursor
Hooks Edit Mode into the loadFromAssimpFile path added in chunk 3, so
fresh-imported assets enter Edit Mode with their source quads intact
in EditableSubMesh::faces.
Modification tracking:
- commitToEntity (same-count edits) and resizeEntityBuffers (topology
edits) wipe the qtme.source_path cached on Ogre::Mesh. After any
user edit, the live GPU buffers diverge from the source file, so
re-importing would discard the edit.
- This makes "is this mesh modified?" a single-bit invariant: if
qtme.source_path is set, the mesh matches the source; if not, the
mesh has diverged (or was never imported, e.g. procedural primitives).
enterEditMode flow:
1. If qtme.source_path is set → loadFromAssimpFile (n-gon path).
2. Else (or on re-import failure) → loadFromEntity (legacy path).
Procedural primitives, .scene.glb sub-entities, post-edit re-entries,
and any path that doesn't carry a source path all fall back cleanly.
Tests (+5; Ogre-bound, run on Linux CI):
- commitToEntity wipes the cached path.
- resizeEntityBuffers wipes the cached path.
- enterEditMode uses the n-gon path when source_path is set
(faces non-empty for a quad OBJ).
- enterEditMode falls back to legacy when source_path absent
(faces empty — chunk-1 invariant preserved).
- After commit-and-exit, re-entering uses the legacy path.
Towards #326. Smoke-testable end-to-end: import a quad asset, Tab into
Edit Mode, the inspector should report n-gon submeshes with face count
matching the source file. Catmull-Clark and loop cut land in chunks 5
and 7 on top of this.
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: 93e2b33c2f
ℹ️ 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".
| if (!sourcePath.empty() | ||
| && m_editableMesh->loadFromAssimpFile(sourcePath)) { | ||
| loaded = true; |
There was a problem hiding this comment.
This call now routes imported meshes through loadFromAssimpFile whenever qtme.source_path exists, but that loader reads raw Assimp geometry without the transform pipeline used by normal import (notably aiProcess_ConvertToLeftHanded and the Z-up bake done in MeshProcessor). For formats like FBX (and especially Z-up assets), entering Edit Mode can therefore load vertices in a different basis than the live Ogre mesh; on exit with commit, commitToEntity writes those mismatched positions back and can mirror/rotate geometry even when the user made no intentional shape change.
Useful? React with 👍 / 👎.
| && m_editableMesh->loadFromAssimpFile(sourcePath)) { | ||
| loaded = true; |
There was a problem hiding this comment.
Using loadFromAssimpFile here also changes bone-index semantics for skinned meshes: that path stores assignments with mesh-local indices, while the normal importer writes Ogre bone handles. After any topology edit that triggers resizeEntityBuffers, those local indices are re-emitted as if they were handles, so skinning can bind vertices to the wrong bones when aiBone order does not match Ogre handle order. The new unconditional n-gon routing makes this reachable for any imported rigged asset with qtme.source_path.
Useful? React with 👍 / 👎.
The original AssimpToOgreImporter applies aiProcess_ConvertToLeftHanded
to every non-.x asset, which flips X (and inverts UV V) so Ogre's
left-handed coordinate system gets correct geometry. My chunk-4
loadFromAssimpFile re-imported the same source file without that flag,
so EditableMesh ended up in the original (right-handed) coordinate
space while the rendered Ogre buffers were in the flipped (left-handed)
space. The vertex / edge / face overlays in Edit Mode therefore drew
mirrored relative to the on-screen mesh. (Reported manually on a Tom &
Jerry asset — overlay points appeared reflected through the Y-Z plane.)
Fix:
- MeshImporterExporter::importer caches the convert-to-left-handed
decision alongside the source path on Ogre::Mesh
(qtme.source_convert_lh).
- EditableMesh::loadFromAssimpFile gains a `convertToLeftHanded`
parameter (default true to match the importer's typical behaviour).
Applies aiProcess_ConvertToLeftHanded when set.
- EditModeController::enterEditMode reads the cached flag and passes
it through, so the editable mesh ends up in the same coordinate
system as the rendered Ogre buffers.
- commitToEntity / resizeEntityBuffers now wipe both
qtme.source_path AND qtme.source_convert_lh on user edits — the
pair is conceptually one cache entry.
Existing 6 LoadFromAssimpFile tests still green (default param keeps
their behaviour identical).
Towards #326.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
1 participant
Summary
Chunk 4 of the quad migration (#326). First chunk where the n-gon path is reachable through the normal Edit Mode flow — and the first chunk worth smoke testing manually.
When a user imports a mesh, Tabs into Edit Mode, and the source asset has quads, the editor now sees those quads as
EditableFacen-gons (not Assimp's diagonal triangulation) — preparing the ground for Catmull-Clark subdivide (chunk 5) and loop cut (chunk 7).Modification tracking
The whole chunk hinges on a single-bit invariant: if
qtme.source_pathis set on the Ogre mesh, the mesh matches the source; if not, the mesh has been edited (or was never imported).EditableMesh::commitToEntity(same-count edits) andresizeEntityBuffers(topology edits) wipe the path on every commit.EditModeController::enterEditModechecks the path: present →loadFromAssimpFile(n-gon); absent →loadFromEntity(legacy).This means user edits never get clobbered by re-imports, and procedural /
.scene.glb/ post-edit entries fall back cleanly.Tests (+5; Ogre-bound — Linux CI only)
commitToEntitywipesqtme.source_path.resizeEntityBufferswipesqtme.source_path.enterEditModeuses the n-gon path when source_path is set (faces non-empty for a quad OBJ).enterEditModefalls back to legacy when source_path absent (faces empty).The 197 standalone tests still pass.
Test plan
EditModeController::currentMesh()->subMeshes()[s].facesshould be non-empty in the inspector for source-quad submeshes.Towards #326.
🤖 Generated with Claude Code