diff --git a/src/EditModeController.cpp b/src/EditModeController.cpp index 6a41141a7..b13a2ec50 100644 --- a/src/EditModeController.cpp +++ b/src/EditModeController.cpp @@ -256,6 +256,55 @@ void EditModeController::toggleEditMode() enterEditMode(); } +// Try the n-gon-aware re-import path: when the entity's mesh has the +// `qtme.source_path` user-binding (cached by MeshImporterExporter at +// import time AND not yet wiped by a topology mutation), re-read the +// source asset through Assimp with aiProcess_Triangulate disabled so +// source quads survive into EditableSubMesh::faces. The cached +// `qtme.source_convert_lh` and `qtme.source_up_axis` keys make the +// editable mesh share basis with the rendered buffers; passing the +// live skeleton makes bone handles match MeshProcessor's convention. +// Returns true on success (caller uses the editable mesh as-is), +// false to signal that the legacy `loadFromEntity` path should run. +static bool tryLoadEditableMeshNGonPath( + Ogre::Entity* entity, EditableMesh* editableMesh) +{ + if (!entity || !editableMesh) return false; + const Ogre::MeshPtr meshPtr = entity->getMesh(); + if (!meshPtr) return false; + + const auto& bindings = meshPtr->getUserObjectBindings(); + const Ogre::Any& any = bindings.getUserAny("qtme.source_path"); + if (!any.has_value()) return false; + + std::string sourcePath; + try { + sourcePath = Ogre::any_cast(any); + } catch (const Ogre::Exception&) { + return false; // Any held the wrong type — fall back defensively. + } + if (sourcePath.empty()) return false; + + // Default: convert-LH=true matches AssimpToOgreImporter's behaviour + // for unknown origins; up-axis=Y-up is the safe default if the + // import didn't cache one. + bool convertLH = true; + const Ogre::Any& lhAny = bindings.getUserAny("qtme.source_convert_lh"); + if (lhAny.has_value()) { + try { convertLH = Ogre::any_cast(lhAny); } + catch (const Ogre::Exception&) {} + } + bool isZup = false; + const Ogre::Any& upAny = bindings.getUserAny("qtme.source_up_axis"); + if (upAny.has_value()) { + try { isZup = (Ogre::any_cast(upAny) == 2); } + catch (const Ogre::Exception&) {} + } + const Ogre::Skeleton* skel = + meshPtr->hasSkeleton() ? meshPtr->getSkeleton().get() : nullptr; + return editableMesh->loadFromAssimpFile(sourcePath, convertLH, isZup, skel); +} + bool EditModeController::enterEditMode() { if (m_editModeActive) @@ -270,67 +319,20 @@ bool EditModeController::enterEditMode() QList entities = sel->getResolvedEntities(); m_editEntity = entities.first(); - // Decompose mesh into editable data. Two paths: - // - // - n-gon path: when MeshImporterExporter has cached the source - // file path (qtme.source_path) on the Ogre::Mesh AND no edit - // has been committed since import (commitToEntity / resize- - // EntityBuffers wipe the cache on mutation), re-import the - // asset through Assimp with aiProcess_Triangulate disabled so - // source quads survive into EditableSubMesh::faces. - // - // - legacy path: read the live Ogre buffers via loadFromEntity. - // This is what every prior chunk used, and what every code - // path that doesn't have a source path (procedural primitives, - // .scene.glb sub-entities, post-edit re-entries) falls back to. - // - // The n-gon path enables Catmull-Clark subdivision, loop cut, and - // any future quad-aware op to act on real source quads instead of - // the diagonal triangulation Assimp emits by default. - // (Quad migration #326, chunk 4.) + // Decompose mesh into editable data. Prefer the n-gon path (re- + // import via Assimp with aiProcess_Triangulate off) when the mesh + // still carries qtme.source_path. Fall back to the legacy + // loadFromEntity path for procedural primitives, .scene.glb sub- + // entities, and post-edit re-entries (where commitToEntity / + // resizeEntityBuffers wipe the cache on mutation). The n-gon path + // is what enables Catmull-Clark subdivide / loop cut / future + // quad-aware ops to act on real source quads. (Quad migration + // #326, chunk 4.) m_editableMesh = std::make_unique(); - - bool loaded = false; - { - const Ogre::MeshPtr meshPtr = m_editEntity->getMesh(); - if (meshPtr) { - const auto& bindings = meshPtr->getUserObjectBindings(); - const Ogre::Any& any = bindings.getUserAny("qtme.source_path"); - if (any.has_value()) { - try { - const std::string sourcePath = - Ogre::any_cast(any); - // The original importer cached its - // convert-to-left-handed choice alongside the path. - // Apply the SAME flag here so the editable mesh - // stays in the same coordinate system as the - // rendered Ogre buffers — without this, on every - // non-.x asset the vertex / edge / face overlays - // would draw mirrored (X flipped) relative to the - // on-screen geometry. Defaults to true to match - // AssimpToOgreImporter's behaviour for unknown - // origins. - bool convertLH = true; - const Ogre::Any& lhAny = - bindings.getUserAny("qtme.source_convert_lh"); - if (lhAny.has_value()) { - try { - convertLH = Ogre::any_cast(lhAny); - } catch (const Ogre::Exception&) {} - } - if (!sourcePath.empty() - && m_editableMesh->loadFromAssimpFile(sourcePath, convertLH)) { - loaded = true; - SentryReporter::addBreadcrumb("edit_mode", - "Edit Mode entered via n-gon import path"); - } - } catch (const Ogre::Exception&) { - // The Any contained something other than a string — - // shouldn't happen but fall through to the legacy - // path defensively. - } - } - } + bool loaded = tryLoadEditableMeshNGonPath(m_editEntity, m_editableMesh.get()); + if (loaded) { + SentryReporter::addBreadcrumb("edit_mode", + "Edit Mode entered via n-gon import path"); } if (!loaded && !m_editableMesh->loadFromEntity(m_editEntity)) { SentryReporter::addBreadcrumb("edit_mode", "Failed to load mesh data for Edit Mode"); @@ -1703,24 +1705,9 @@ bool EditModeController::extrudeSelection() if (!m_editableMesh->resizeEntityBuffers(m_editEntity)) return false; - // Force Entity to rebuild its SubEntity list from the updated Mesh. - // Without this, Ogre's skeletal skinning pipeline may use stale - // animation blend buffers that still reference the old vertex layout. - m_editEntity->_deinitialise(); - m_editEntity->_initialise(true); - - // Invalidate RTSS shaders for the entity's materials so they regenerate - // against the new vertex declaration (with possibly new tangent / blend - // indices elements). Without this, bump maps / skinning may render wrong. - auto* shaderGen = Ogre::RTShader::ShaderGenerator::getSingletonPtr(); - if (shaderGen) { - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - const std::string& matName = m_editEntity->getSubEntity(i)->getMaterialName(); - if (!matName.empty()) - shaderGen->invalidateMaterial( - Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, matName); - } - } + // Refresh Entity caches + RTSS state (incl. tangent rebuild for + // bump-mapped materials). See `rewriteEntityAfterTopologyChange`. + rewriteEntityAfterTopologyChange(m_editEntity); // Select the new (offset) vertices by position — offset ensures uniqueness m_selectedVertices.clear(); @@ -1844,34 +1831,10 @@ bool EditModeController::applyBevelTopology( if (!m_editableMesh->resizeEntityBuffers(m_editEntity)) return false; - // _deinitialise/_initialise rebuilds SubEntities and resets their - // material to the SubMesh default. In edit mode the SubEntity holds - // the wireframe variant and MaterialEditor writes go to the SubEntity - // (not the SubMesh), so both must be preserved across the rebuild. - std::vector preMats; - preMats.reserve(m_editEntity->getNumSubEntities()); - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - preMats.push_back(m_editEntity->getSubEntity(i)->getMaterialName()); - } - - m_editEntity->_deinitialise(); - m_editEntity->_initialise(true); - - for (unsigned int i = 0; - i < m_editEntity->getNumSubEntities() && i < preMats.size(); ++i) { - if (!preMats[i].empty()) - m_editEntity->getSubEntity(i)->setMaterialName(preMats[i]); - } - - auto* shaderGen = Ogre::RTShader::ShaderGenerator::getSingletonPtr(); - if (shaderGen) { - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - const std::string& matName = m_editEntity->getSubEntity(i)->getMaterialName(); - if (!matName.empty()) - shaderGen->invalidateMaterial( - Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, matName); - } - } + // Refresh Entity caches + RTSS state (incl. tangent rebuild for + // bump-mapped materials), preserving per-subentity material + // overrides (wireframe variant, MaterialEditor writes). + rewriteEntityAfterTopologyChange(m_editEntity); m_selectedVertices.clear(); m_selectedEdges.clear(); @@ -1969,29 +1932,7 @@ bool EditModeController::applyBevelVertexTopology( if (!m_editableMesh->resizeEntityBuffers(m_editEntity)) return false; - std::vector preMats; - preMats.reserve(m_editEntity->getNumSubEntities()); - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) - preMats.push_back(m_editEntity->getSubEntity(i)->getMaterialName()); - - m_editEntity->_deinitialise(); - m_editEntity->_initialise(true); - - for (unsigned int i = 0; - i < m_editEntity->getNumSubEntities() && i < preMats.size(); ++i) { - if (!preMats[i].empty()) - m_editEntity->getSubEntity(i)->setMaterialName(preMats[i]); - } - - auto* shaderGen = Ogre::RTShader::ShaderGenerator::getSingletonPtr(); - if (shaderGen) { - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - const std::string& matName = m_editEntity->getSubEntity(i)->getMaterialName(); - if (!matName.empty()) - shaderGen->invalidateMaterial( - Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, matName); - } - } + rewriteEntityAfterTopologyChange(m_editEntity); m_selectedVertices.clear(); m_selectedEdges.clear(); @@ -2401,35 +2342,10 @@ void EditModeController::cancelBevel() m_selectedEdges = std::move(m_bevelSession.origSelectedEdges); m_selectedFaces = std::move(m_bevelSession.origSelectedFaces); - // Re-sync the entity with the restored mesh. Snapshot SubEntity - // materials before _deinitialise/_initialise (Ogre resets them to - // the SubMesh default) so wireframe / material-editor overrides - // survive the Esc path, matching the commit path. + // Re-sync the entity with the restored mesh — preserves wireframe / + // material-editor SubEntity overrides through the Esc path. m_editableMesh->resizeEntityBuffers(m_editEntity); - if (m_editEntity) { - std::vector preMats; - preMats.reserve(m_editEntity->getNumSubEntities()); - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) - preMats.push_back(m_editEntity->getSubEntity(i)->getMaterialName()); - - m_editEntity->_deinitialise(); - m_editEntity->_initialise(true); - - for (unsigned int i = 0; - i < m_editEntity->getNumSubEntities() && i < preMats.size(); ++i) { - if (!preMats[i].empty()) - m_editEntity->getSubEntity(i)->setMaterialName(preMats[i]); - } - } - auto* shaderGen = Ogre::RTShader::ShaderGenerator::getSingletonPtr(); - if (shaderGen && m_editEntity) { - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - const std::string& matName = m_editEntity->getSubEntity(i)->getMaterialName(); - if (!matName.empty()) - shaderGen->invalidateMaterial( - Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, matName); - } - } + rewriteEntityAfterTopologyChange(m_editEntity); m_bevelSession = {}; if (m_bevelGizmo) m_bevelGizmo->setVisible(false); @@ -2559,30 +2475,73 @@ bool EditModeController::commitKnife() // consecutive endpoints, so the visible preview becomes a real chain // of mesh edges. OnFace/OnVertex clicks aren't yet in scope — the // commit skips them and proceeds on the OnEdge subset. + // + // splitEdge — the primitive cutPath uses — is a triangle-only MVP + // (n-gon support requires ear-clip-aware rewiring). On a quad- + // imported mesh, every face is an n-gon and splitEdge bails + // immediately, so knife silently fails. Workaround until a proper + // n-gon splitEdge lands: build the HE from a triangle-mode COPY + // (clear `.faces` so buildFromEditableMesh falls back to the + // fan-triangulated `.triangles` mirror). The user gets a working + // knife at the cost of materialising the fan diagonals on every + // submesh the cut touches. Tracked as a follow-up. + EditableMesh triOnly; + triOnly.subMeshes() = m_editableMesh->subMeshes(); + // Remember which submeshes were originally n-gon-canonical + // (`!faces.empty()`) so we can restore them post-write-back. Without + // this, a knife on submesh 0 would convert every untouched submesh + // to fan triangles globally (since toEditableMesh writes back from + // the all-triangulated HE). + std::vector wasNGonSub; + wasNGonSub.reserve(triOnly.subMeshes().size()); + for (auto& sub : triOnly.subMeshes()) { + wasNGonSub.push_back(!sub.faces.empty()); + sub.faces.clear(); + } HalfEdgeMesh hm; - if (!hm.buildFromEditableMesh(*m_editableMesh)) { + if (!hm.buildFromEditableMesh(triOnly)) { cancelKnife(); return false; } - // Refuse the commit if any confirmed point isn't snapped to an edge. - // OnFace and OnVertex captures exist for the preview, but the MVP - // commit pipeline only knows how to cut along edges; silently - // dropping a face click would produce a mesh that doesn't match the - // line the user just drew, which is worse than failing the commit. + // Build the CutPoint list, translating OnVertex clicks into edge + // clicks: pick any incident edge to the vertex and use t=0 or t=1 + // depending on which endpoint the vertex sits at. `cutPath` only + // accepts edge inputs (its `splitEdge` primitive is edge-keyed), + // and `splitEdge` clamps t away from 0/1 by 1e-4 to avoid sliver + // faces — so the resulting vertex sits a hair off the user's click + // but topology stays clean. OnFace clicks are still rejected: the + // commit can't represent them with the current edge-walk algorithm. + std::vector cpts; + cpts.reserve(m_knifeSession.points.size()); for (const auto& p : m_knifeSession.points) { - if (p.kind != KnifePoint::OnEdge) { + if (p.kind == KnifePoint::OnEdge) { + cpts.push_back({p.edgeIndex, p.edgeT}); + continue; + } + if (p.kind != KnifePoint::OnVertex) { SentryReporter::addBreadcrumb("edit_mode", - "Knife: commit rejected (non-edge point — only edge cuts supported)"); + "Knife: commit rejected (OnFace point — only edge / vertex supported)"); cancelKnife(); return false; } - } - - std::vector cpts; - cpts.reserve(m_knifeSession.points.size()); - for (const auto& p : m_knifeSession.points) { - cpts.push_back({p.edgeIndex, p.edgeT}); + // OnVertex → find any incident edge in the triangle-mode HE + // and pick the t that puts the new split-vertex closest to the + // clicked vertex. + int incidentEdge = -1; + float incidentT = 0.0f; + for (size_t e = 0; e < hm.edgeCount(); ++e) { + const auto [ev0, ev1] = hm.edgeVertices(static_cast(e)); + if (ev0 == p.vertexIndex) { incidentEdge = static_cast(e); incidentT = 0.0f; break; } + if (ev1 == p.vertexIndex) { incidentEdge = static_cast(e); incidentT = 1.0f; break; } + } + if (incidentEdge < 0) { + SentryReporter::addBreadcrumb("edit_mode", + "Knife: commit rejected (OnVertex point — no incident edge)"); + cancelKnife(); + return false; + } + cpts.push_back({incidentEdge, incidentT}); } if (cpts.size() < 2) { SentryReporter::addBreadcrumb("edit_mode", @@ -2611,32 +2570,37 @@ bool EditModeController::commitKnife() cancelKnife(); return false; } - m_editableMesh->subMeshes() = std::move(updated.subMeshes()); - m_editableMesh->resizeEntityBuffers(m_editEntity); - std::vector preMats; - preMats.reserve(m_editEntity->getNumSubEntities()); - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - preMats.push_back(m_editEntity->getSubEntity(i)->getMaterialName()); + // Determine which submeshes the cut actually touched. A cut creates + // new vertices either at edge clicks or at interior crossings; each + // new vertex lives on faces in the submesh(es) it pierced. + // toEditableMesh writes EVERY submesh as triangle-only (since the + // HE was triangulated up-front), so without restoring untouched + // n-gon submeshes back from the original snapshot, a single knife + // op would silently convert unrelated submeshes to fan triangles. + // (Codex P1 review on this PR.) + std::set touchedSubs; + for (int v : cutVerts) { + for (int f : hm.facesAroundVertex(v)) { + touchedSubs.insert(hm.face(f).subMeshIndex); + } } - - m_editEntity->_deinitialise(); - m_editEntity->_initialise(true); - - for (unsigned int i = 0; - i < m_editEntity->getNumSubEntities() && i < preMats.size(); ++i) { - if (!preMats[i].empty()) - m_editEntity->getSubEntity(i)->setMaterialName(preMats[i]); + auto& outSubs = updated.subMeshes(); + for (size_t s = 0; + s < outSubs.size() && s < originalSubMeshes.size() && s < wasNGonSub.size(); + ++s) { + if (touchedSubs.count(static_cast(s))) continue; + if (!wasNGonSub[s]) continue; // already triangle-only — nothing to restore + // Untouched + originally n-gon → restore the original submesh + // verbatim. resizeEntityBuffers consumes both `triangles` and + // bone assignments, so we want the full pre-cut state. + outSubs[s] = originalSubMeshes[s]; } - if (auto* shaderGen = Ogre::RTShader::ShaderGenerator::getSingletonPtr()) { - for (unsigned int i = 0; i < m_editEntity->getNumSubEntities(); ++i) { - const std::string& matName = m_editEntity->getSubEntity(i)->getMaterialName(); - if (!matName.empty()) - shaderGen->invalidateMaterial( - Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, matName); - } - } + m_editableMesh->subMeshes() = std::move(outSubs); + m_editableMesh->resizeEntityBuffers(m_editEntity); + + rewriteEntityAfterTopologyChange(m_editEntity); // Clear edge/face selection — pre-cut IDs refer to retired topology // slots and the walk added new vertices that aren't in any existing @@ -2673,17 +2637,111 @@ bool EditModeController::commitKnife() // the HE primitive (centroid / first / last / per-cluster centroid for // by-distance). `applyMergeOp` factors that boilerplate. // --------------------------------------------------------------------------- +// Shared post-mesh-mutation hook: rewrite the entity's Ogre buffers, +// save / restore per-subentity material overrides, and tell RTSS to +// re-link shaders against the new vertex layout. Used by every Edit- +// Mode topology op AND by EditMeshTopologyCommand::applyMeshState so +// undo/redo preserves bump map / per-pixel lighting state. +// +// Bump-map / per-pixel lighting handling: after a topology op the new +// vertices written by EditableMesh::buildSubMeshBuffers have zero-valued +// tangents (EditableVertex defaults), and the declaration may even drop +// the VES_TANGENT slot entirely if `vertices[0].hasTangent == false` +// (which is the case on the n-gon import path that omits +// aiProcess_CalcTangentSpace). Either way RTSS's SRS_NORMALMAP TBN math +// collapses, dropping bump mapping and (depending on the shader path) +// basic per-pixel lighting. Fix: force `Mesh::buildTangentVectors` +// BEFORE `_deinitialise/_initialise` so the SubEntity vertex-decl cache +// reads the corrected layout, then re-run `applyNormalMapsToEntity` so +// RTSS re-attaches SRS_NORMALMAP against the fresh tangents. namespace { -// Shared post-mesh-mutation hook used by knife + merge: rewrite the entity's -// Ogre buffers, save / restore per-subentity material overrides, and tell -// RTSS to re-link shaders against the new vertex layout. Captures locally -// to avoid a public helper just for this. -inline void rewriteEntityAfterTopologyChange(Ogre::Entity* ent) { +// Returns true if any sub-entity of `ent` has a normal-map TUS — the +// signal we use to decide whether RTSS will need tangents on the next +// render. Materials that fail to load are skipped so a single broken +// resource doesn't abort the topology op. +bool entityWantsTangents(Ogre::Entity* ent) { + for (unsigned int i = 0; i < ent->getNumSubEntities(); ++i) { + auto mat = ent->getSubEntity(i)->getMaterial(); + if (!mat) continue; + if (!mat->isLoaded()) { + try { mat->load(); } + catch (const Ogre::Exception&) { continue; } + } + if (mat->getNumTechniques() == 0) continue; + auto* pass = mat->getTechnique(0)->getPass(0); + if (!pass) continue; + for (unsigned short t = 0; t < pass->getNumTextureUnitStates(); ++t) { + const auto& tusName = pass->getTextureUnitState(t)->getName(); + if (tusName == "normal_map" || tusName == "NormalMap") return true; + } + } + return false; +} + +// Force-rebuild tangent vectors on `mesh`. Logs (rather than throws) on +// failure since the caller runs from inside an entity-refresh hook. +void rebuildMeshTangents(const Ogre::MeshPtr& mesh) { + if (!mesh) return; + try { + // storeParityInW=true → VET_FLOAT4, matching what + // RTShaderHelper::applyNormalMap and the import path use. + mesh->buildTangentVectors(/*sourceTexCoordSet=*/0, + /*splitMirrored=*/false, + /*splitRotated=*/false, + /*storeParityInW=*/true); + } catch (const Ogre::Exception& e) { + Ogre::LogManager::getSingleton().logMessage( + "rewriteEntityAfterTopologyChange: buildTangentVectors " + "failed for '" + mesh->getName() + "': " + e.getDescription()); + } +} + +// Drop the cached RTSS shader programs for every sub-entity material +// so the next render regenerates them against the post-topology-op +// vertex declaration. No-op when the ShaderGenerator is absent. +void invalidateEntityRtssMaterials(Ogre::Entity* ent) { + auto* sg = Ogre::RTShader::ShaderGenerator::getSingletonPtr(); + if (!sg) return; + for (unsigned int i = 0; i < ent->getNumSubEntities(); ++i) { + const std::string& m = ent->getSubEntity(i)->getMaterialName(); + if (!m.empty()) + sg->invalidateMaterial( + Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, m); + } +} +} // namespace + +void EditModeController::rewriteEntityAfterTopologyChange(Ogre::Entity* ent) { + if (!ent) return; + + // Snapshot per-subentity material overrides — _deinitialise/_initialise + // resets them to the SubMesh default, but the wireframe overlay and + // MaterialEditor write to the SubEntity, not the SubMesh, so both + // must survive the rebuild. std::vector preMats; preMats.reserve(ent->getNumSubEntities()); for (unsigned int i = 0; i < ent->getNumSubEntities(); ++i) preMats.push_back(ent->getSubEntity(i)->getMaterialName()); + // Rebuild tangents BEFORE `_deinitialise/_initialise` so the + // SubEntity vertex-decl cache captured during `_initialise` already + // sees the VES_TANGENT element. Calling `buildTangentVectors` AFTER + // `_initialise` is too late: the SubEntity has already linked + // against the old (no-tangent) declaration and the next render + // compiles RTSS shaders against that stale layout, so the + // SRS_NORMALMAP code path collapses to a no-op even though the + // tangents physically exist in the buffer. + // + // We rebuild whenever any subentity is bump-mapped: new vertices + // written by `EditableMesh::buildSubMeshBuffers` start with zero- + // valued tangents (EditableVertex defaults), and on the n-gon + // import path — which omits `aiProcess_CalcTangentSpace` because + // that flag forces triangulation — the declaration drops VES_TANGENT + // entirely. `Mesh::buildTangentVectors` re-adds the element if + // missing and recomputes values from positions/normals/UVs. + if (entityWantsTangents(ent)) + rebuildMeshTangents(ent->getMesh()); + // Re-init the entity so its SubEntity caches (vertex / index // counts, skeleton anim buffers) sync to the resized mesh. // Without this the next render uses stale draw-call params and @@ -2697,34 +2755,16 @@ inline void rewriteEntityAfterTopologyChange(Ogre::Entity* ent) { ent->getSubEntity(i)->setMaterialName(preMats[i]); } - // Invalidate the cached RTSS technique so the next render - // regenerates it against the new vertex declaration. Lazy regen - // via SchemeResolverListener::handleSchemeNotFound runs on the - // next render. (Same pattern every other topology op uses.) - // - // KNOWN ISSUE post-chunk-4: bump-mapped meshes loaded through - // the n-gon import path lose their bump map (and sometimes basic - // per-pixel lighting) after a topology op. The cause is that - // `EditableMesh::loadFromAssimpFile` deliberately skips - // aiProcess_CalcTangentSpace (it forces triangulation) and the - // post-edit GPU upload path doesn't always reliably trigger an - // Ogre `buildTangentVectors` rebuild. Various RTSS - // re-attach permutations (sync / deferred / via - // applyNormalMapsToEntity / invalidate-only) all reproduce the - // failure on the same model. Tracked for a follow-up fix-PR - // with proper shader-pipeline instrumentation rather than - // continued blind permutation. Triangle-only assets and - // procedural primitives are unaffected. - if (auto* sg = Ogre::RTShader::ShaderGenerator::getSingletonPtr()) { - for (unsigned int i = 0; i < ent->getNumSubEntities(); ++i) { - const std::string& m = ent->getSubEntity(i)->getMaterialName(); - if (!m.empty()) - sg->invalidateMaterial( - Ogre::RTShader::ShaderGenerator::DEFAULT_SCHEME_NAME, m); - } - } + // Re-attach the RTSS SRS_NORMALMAP sub-render-state per material + // that has a normal-map TUS, then drop any cached shader programs + // so the next render regenerates against the new vertex layout. + // `applyNormalMapsToEntity` uses `removeShaderBasedTechnique + + // createShaderBasedTechnique` to start from a clean slate (some + // tangent-related state doesn't survive a buffer-format change), + // so it must run AFTER the tangent rebuild + _initialise. + MeshImporterExporter::applyNormalMapsToEntity(ent); + invalidateEntityRtssMaterials(ent); } -} // namespace // Find global vertex indices of the survivor positions after toEditableMesh // re-packed the per-submesh arrays. Each survivor is the unique vertex at @@ -3001,7 +3041,7 @@ int applyTopologyMutationNoSurvivor( editableMesh->recalculateNormalsFlat(); editableMesh->resizeEntityBuffers(editEntity); - rewriteEntityAfterTopologyChange(editEntity); + EditModeController::rewriteEntityAfterTopologyChange(editEntity); selVerts.clear(); selEdges.clear(); @@ -3481,7 +3521,7 @@ int EditModeController::fillSelection() validateMesh(); SentryReporter::addBreadcrumb("edit_mode", - QString("Fill (verts=%1, tris=%2)") + QString("Fill (verts=%1, faces=%2)") .arg(targetVerts.size()).arg(created)); updateSelectionOverlay(); @@ -3511,10 +3551,70 @@ bool EditModeController::knifeHitTest(const QPoint& screenPos, OgreWidget* widge const int vw = widget->width(); const int vh = widget->height(); + // Build the front-facing vertex / edge sets once (mirrors the + // chunk-4b behaviour of regular hitTestEdge / face selection): on + // a dense mesh, snapping to back-face geometry that's hidden behind + // the visible surface is impossible to control. Front-facing means + // the polygon's Newell normal points toward the camera. A vertex is + // front-facing if at least one incident polygon faces the camera; + // an edge is front-facing if at least one of its two adjacent + // polygons does. Edge keys use (min,max) global vertex indices so + // they line up with the triangle-mode HE used below. + Ogre::SceneNode* node = m_editEntity->getParentSceneNode(); + const Ogre::Vector3 camPos = camera->getDerivedPosition(); + auto isPolygonFrontFacing = [&](const EditableSubMesh& sub, + const std::vector& corners) -> bool { + if (corners.size() < 3 || !node) return false; + Ogre::Vector3 nrm = Ogre::Vector3::ZERO; + for (size_t i = 0; i < corners.size(); ++i) { + if (corners[i] >= sub.vertices.size()) return false; + const auto& a = sub.vertices[corners[i]].position; + const auto& b = sub.vertices[corners[(i + 1) % corners.size()]].position; + nrm.x += (a.y - b.y) * (a.z + b.z); + nrm.y += (a.z - b.z) * (a.x + b.x); + nrm.z += (a.x - b.x) * (a.y + b.y); + } + const Ogre::Vector3 worldNrm = node->_getDerivedOrientation() * nrm; + const Ogre::Vector3 anyCornerWorld = + node->convertLocalToWorldPosition(sub.vertices[corners[0]].position); + return worldNrm.dotProduct(camPos - anyCornerWorld) > 0.0f; + }; + + std::set frontVerts; + std::set> frontEdges; + for (size_t si = 0; si < m_editableMesh->subMeshes().size(); ++si) { + const auto& sub = m_editableMesh->subMeshes()[si]; + const int vertOffset = localToGlobal(static_cast(si), 0); + auto recordFrontPoly = [&](const std::vector& corners) { + for (size_t i = 0; i < corners.size(); ++i) { + const int g0 = vertOffset + static_cast(corners[i]); + const int g1 = vertOffset + static_cast(corners[(i + 1) % corners.size()]); + frontVerts.insert(g0); + frontEdges.emplace(std::min(g0, g1), std::max(g0, g1)); + } + }; + if (!sub.faces.empty()) { + for (const auto& face : sub.faces) { + if (face.indices.size() < 3) continue; + if (!isPolygonFrontFacing(sub, face.indices)) continue; + recordFrontPoly(face.indices); + } + } else { + for (const auto& tri : sub.triangles) { + std::vector corners = { + tri.indices[0], tri.indices[1], tri.indices[2] }; + if (!isPolygonFrontFacing(sub, corners)) continue; + recordFrontPoly(corners); + } + } + } + // Priority 1: vertex snap. Uses the same radius as regular edit-mode - // vertex picking so the user's eye can predict the snap. + // vertex picking so the user's eye can predict the snap. Skip + // back-face vertices (not in `frontVerts`) so dense meshes don't + // pull clicks to vertices the user can't see. const int snapVert = hitTestVertex(screenPos, camera, vw, vh, 10.0f); - if (snapVert >= 0) { + if (snapVert >= 0 && frontVerts.count(snapVert)) { auto [subIdx, localIdx] = globalToLocal(snapVert); if (subIdx < m_editableMesh->subMeshes().size() && localIdx < m_editableMesh->subMeshes()[subIdx].vertices.size()) { @@ -3538,7 +3638,6 @@ bool EditModeController::knifeHitTest(const QPoint& screenPos, OgreWidget* widge const Ogre::Real nx = static_cast(screenPos.x()) / vw; const Ogre::Real ny = static_cast(screenPos.y()) / vh; const Ogre::Ray ray = camera->getCameraToViewportRay(nx, ny); - Ogre::SceneNode* node = m_editEntity->getParentSceneNode(); Ogre::Vector3 rO = ray.getOrigin(); Ogre::Vector3 rD = ray.getDirection(); @@ -3548,9 +3647,18 @@ bool EditModeController::knifeHitTest(const QPoint& screenPos, OgreWidget* widge rD = worldToLocal.linear() * rD; } + // Build the HE from a triangle-mode COPY of the editable mesh, + // mirroring what the knife commit path does. Otherwise the HE + // edge indices we record on the click here (n-gon HE: one edge + // per polygon side) wouldn't match the indices `commitKnife` + // resolves against (triangle HE: one edge per fan side), and + // every cut would land on the wrong edge — manifesting as a + // knife that "does nothing" on quad-imported assets. + EditableMesh triOnly; + triOnly.subMeshes() = m_editableMesh->subMeshes(); + for (auto& sub : triOnly.subMeshes()) sub.faces.clear(); HalfEdgeMesh tmp; - if (tmp.buildFromEditableMesh(*m_editableMesh)) { - const Ogre::Vector3 camPosWorld = camera->getDerivedPosition(); + if (tmp.buildFromEditableMesh(triOnly)) { constexpr float kPixelRadius = 10.0f; float bestDepth = std::numeric_limits::infinity(); int bestEdgeIdx = -1; @@ -3560,6 +3668,18 @@ bool EditModeController::knifeHitTest(const QPoint& screenPos, OgreWidget* widge for (size_t e = 0; e < tmp.edgeCount(); ++e) { auto [gv0, gv1] = tmp.edgeVertices(static_cast(e)); if (gv0 < 0 || gv1 < 0) continue; + // Skip back-face edges. `frontEdges` was computed from + // the n-gon `m_editableMesh`, keyed by global vertex + // pair; the triangle-mode `tmp` indices use the same + // global numbering, so the lookup is direct. Note that + // `frontEdges` only contains polygon-perimeter edges: + // a fan-triangulation diagonal between two front-facing + // verts is rejected here, which is correct — the user + // shouldn't be able to click on a fake interior edge. + { + const std::pair key{std::min(gv0, gv1), std::max(gv0, gv1)}; + if (!frontEdges.count(key)) continue; + } auto [sub0, loc0] = globalToLocal(gv0); auto [sub1, loc1] = globalToLocal(gv1); if (sub0 >= m_editableMesh->subMeshes().size()) continue; @@ -3598,7 +3718,7 @@ bool EditModeController::knifeHitTest(const QPoint& screenPos, OgreWidget* widge } const Ogre::Vector3 local = p0 + d * t; const Ogre::Vector3 world = node->convertLocalToWorldPosition(local); - const float depth = (world - camPosWorld).dotProduct(camera->getDerivedDirection()); + const float depth = (world - camPos).dotProduct(camera->getDerivedDirection()); if (depth <= 0.0f) continue; // behind camera if (depth < bestDepth) { bestDepth = depth; diff --git a/src/EditModeController.h b/src/EditModeController.h index de551da0c..7dec3d4f3 100644 --- a/src/EditModeController.h +++ b/src/EditModeController.h @@ -885,6 +885,19 @@ private slots: void applyWireframeMaterials(); void removeWireframeMaterials(); +public: + /// Refresh an entity after a topology mutation: rebuild tangents + /// (when any material is bump-mapped), `_deinitialise/_initialise` + /// the entity, restore per-subentity material overrides, re-attach + /// RTSS SRS_NORMALMAP, and invalidate cached shader programs. Used + /// by every Edit-Mode topology op (subdivide, extrude, bevel, …) + /// AND by `EditMeshTopologyCommand::applyMeshState` so undo/redo + /// preserves bump map / per-pixel lighting state. Static so + /// command code can invoke it without a controller instance. + static void rewriteEntityAfterTopologyChange(Ogre::Entity* ent); + +private: + // Wireframe mode bool m_wireframeEnabled = false; std::map m_savedMaterials; ///< SubEntity index → original material name diff --git a/src/EditModeController_test.cpp b/src/EditModeController_test.cpp index 650c8cd5a..9f4a81ce6 100644 --- a/src/EditModeController_test.cpp +++ b/src/EditModeController_test.cpp @@ -27,6 +27,26 @@ The MIT License #include #include +// =========================================================================== +// rewriteEntityAfterTopologyChange — null + no-bump-map shape (chunk: quads +// follow-up). Full bump-map / RTSS exercise needs a GL context, which the +// test infra doesn't provide on macOS; the integration coverage lives in +// hand smoke tests on the bump-mapped Mixamo asset. Here we lock down the +// public-API contract: static, callable from outside the class (notably +// from EditMeshTopologyCommand::applyMeshState in the undo/redo path), +// null-tolerant, and a no-op when no entity material requests tangents. +// =========================================================================== + +TEST(EditModeControllerStandalone, RewriteEntityAfterTopologyChangeNullIsNoOp) { + // Regression for chunk 4b → quads follow-up: the static helper is + // reachable from command code in TransformCommands.cpp via a class- + // qualified call, and accepts a null entity without crashing. If + // someone refactors it back to a free function in an anonymous + // namespace, undo/redo will silently lose its lighting hook again. + EditModeController::rewriteEntityAfterTopologyChange(nullptr); + SUCCEED(); +} + // =========================================================================== // Pure geometry tests (no Ogre needed) // =========================================================================== diff --git a/src/EditableMesh.cpp b/src/EditableMesh.cpp index 611c692dd..5a482ab40 100644 --- a/src/EditableMesh.cpp +++ b/src/EditableMesh.cpp @@ -147,7 +147,9 @@ bool EditableMesh::loadFromMesh(const Ogre::MeshPtr& meshPtr) } bool EditableMesh::loadFromAssimpFile(const std::string& path, - bool convertToLeftHanded) + bool convertToLeftHanded, + bool isZup, + const Ogre::Skeleton* skeletonForBoneHandles) { if (path.empty()) return false; @@ -189,6 +191,18 @@ bool EditableMesh::loadFromAssimpFile(const std::string& path, m_subMeshes.clear(); m_subMeshes.reserve(scene->mNumMeshes); + // Mirror MeshProcessor's Z-up bake: when the source asset declares + // Z-up (FBX UpAxis = 2), the GUI importer rotates every position / + // normal / tangent +90° around X so the Ogre scene-graph stays Y-up + // without needing a node rotation. We must apply the SAME rotation + // here so the editable representation lives in the same basis as + // the rendered Ogre buffers; otherwise the vertex/edge/face overlays + // appear rotated 90° (and on commit, the mismatched positions get + // written back, silently rotating the geometry). + const Ogre::Quaternion zUpBake = isZup + ? Ogre::Quaternion(Ogre::Degree(90), Ogre::Vector3::UNIT_X) + : Ogre::Quaternion::IDENTITY; + for (unsigned m = 0; m < scene->mNumMeshes; ++m) { const aiMesh* aim = scene->mMeshes[m]; if (!aim || aim->mNumVertices == 0) continue; @@ -210,10 +224,10 @@ bool EditableMesh::loadFromAssimpFile(const std::string& path, for (unsigned i = 0; i < aim->mNumVertices; ++i) { EditableVertex& ev = sub.vertices[i]; const aiVector3D& p = aim->mVertices[i]; - ev.position = Ogre::Vector3(p.x, p.y, p.z); + ev.position = zUpBake * Ogre::Vector3(p.x, p.y, p.z); if (hasNormals) { const aiVector3D& n = aim->mNormals[i]; - ev.normal = Ogre::Vector3(n.x, n.y, n.z); + ev.normal = zUpBake * Ogre::Vector3(n.x, n.y, n.z); ev.hasNormal = true; } if (hasUVs) { @@ -227,33 +241,48 @@ bool EditableMesh::loadFromAssimpFile(const std::string& path, ev.hasColor = true; } if (hasTangents) { - const aiVector3D& t = aim->mTangents[i]; // RTSS expects FLOAT4 tangents with handedness in w. - // Compute parity from the input bitangent: if cross - // (normal × tangent) aligns with bitangent, parity = - // +1, else -1. Same convention MeshProcessor / - // applyNormalMapsToEntity use. - const aiVector3D& bt = aim->mBitangents[i]; - Ogre::Vector3 normalV = ev.hasNormal ? ev.normal : Ogre::Vector3::UNIT_Z; - Ogre::Vector3 tangentV(t.x, t.y, t.z); - Ogre::Vector3 expectedBT = normalV.crossProduct(tangentV); - float parity = expectedBT.dotProduct( - Ogre::Vector3(bt.x, bt.y, bt.z)) >= 0.0f ? 1.0f : -1.0f; - ev.tangent = Ogre::Vector4(t.x, t.y, t.z, parity); + // Apply the same Z-up bake to T/B/N before computing + // parity so the convention matches MeshProcessor. + Ogre::Vector3 T = zUpBake * Ogre::Vector3( + aim->mTangents[i].x, aim->mTangents[i].y, aim->mTangents[i].z); + Ogre::Vector3 B = zUpBake * Ogre::Vector3( + aim->mBitangents[i].x, aim->mBitangents[i].y, aim->mBitangents[i].z); + Ogre::Vector3 N = ev.hasNormal ? ev.normal : Ogre::Vector3::UNIT_Z; + float parity = N.crossProduct(T).dotProduct(B) >= 0.0f ? 1.0f : -1.0f; + ev.tangent = Ogre::Vector4(T.x, T.y, T.z, parity); ev.hasTangent = true; } } // Bone weights, if any. + // + // CRITICAL: assignments must use the same bone-handle convention + // the rendered Ogre mesh uses. MeshProcessor (the GUI import + // path) resolves `aiBone->mName` against the loaded skeleton and + // stores `Ogre::Bone::getHandle()`. If we instead stored aiBone + // mesh-local indices here, then on the next topology op (which + // re-emits VertexBoneAssignments through resizeEntityBuffers) + // those indices would be interpreted as handles — vertices would + // bind to whichever bone happens to live at that handle slot, + // not the bone the source file actually weighted them to. + // Skip bones that don't resolve so we never emit wild handles. if (aim->mNumBones > 0) { for (unsigned b = 0; b < aim->mNumBones; ++b) { const aiBone* bone = aim->mBones[b]; if (!bone) continue; + unsigned short handle = static_cast(b); + if (skeletonForBoneHandles) { + const std::string boneName = bone->mName.C_Str(); + if (!skeletonForBoneHandles->hasBone(boneName)) continue; + handle = static_cast( + skeletonForBoneHandles->getBone(boneName)->getHandle()); + } for (unsigned w = 0; w < bone->mNumWeights; ++w) { const aiVertexWeight& vw = bone->mWeights[w]; if (vw.mVertexId >= sub.vertices.size()) continue; EditableBoneAssignment eba; - eba.boneIndex = static_cast(b); + eba.boneIndex = handle; eba.weight = vw.mWeight; sub.vertices[vw.mVertexId].boneAssignments.push_back(eba); } @@ -487,6 +516,7 @@ bool EditableMesh::commitToEntity(Ogre::Entity* entity) // discarded by an n-gon re-import. (Quad migration #326, chunk 4.) mesh->getUserObjectBindings().eraseUserAny("qtme.source_path"); mesh->getUserObjectBindings().eraseUserAny("qtme.source_convert_lh"); + mesh->getUserObjectBindings().eraseUserAny("qtme.source_up_axis"); return true; } @@ -682,6 +712,7 @@ bool EditableMesh::resizeEntityBuffers(Ogre::Entity* entity) // Topology has changed — same rationale as commitToEntity above. mesh->getUserObjectBindings().eraseUserAny("qtme.source_path"); mesh->getUserObjectBindings().eraseUserAny("qtme.source_convert_lh"); + mesh->getUserObjectBindings().eraseUserAny("qtme.source_up_axis"); return true; } diff --git a/src/EditableMesh.h b/src/EditableMesh.h index 579a443be..b88c39334 100644 --- a/src/EditableMesh.h +++ b/src/EditableMesh.h @@ -304,8 +304,31 @@ class EditableMesh * @return true on success; false if the file is missing, can't be * parsed, or contains no mesh data. */ + /** + * @param isZup If true, applies the same +90°-around-X rotation that + * `MeshProcessor` bakes into the rendered Ogre buffers + * when the source asset declares a Z-up coordinate + * system (FBX `UpAxis = 2`). Without this, the editable + * vertices live in the source's pre-bake basis while the + * rendered buffers are post-bake, and selection overlays + * appear rotated 90° relative to the rendered geometry. + * The original importer caches its choice at + * `getUserAny("qtme.source_up_axis")` (an int — 1 = Y-up, + * 2 = Z-up). + * @param skeletonForBoneHandles If non-null, `aiBone->mName` is + * resolved against this skeleton via `getBone(name)` and + * the resulting `Ogre::Bone::getHandle()` is stored as + * `EditableBoneAssignment::boneIndex`. This matches the + * handle convention `MeshProcessor` writes into the live + * Ogre mesh; without it, this path emits mesh-local + * aiBone indices that re-bind vertices to the wrong + * bones after a topology op. Pass nullptr only for + * unskinned meshes. + */ bool loadFromAssimpFile(const std::string& path, - bool convertToLeftHanded = true); + bool convertToLeftHanded = true, + bool isZup = false, + const Ogre::Skeleton* skeletonForBoneHandles = nullptr); /** * @brief Merge vertices at (approximately) coincident positions within diff --git a/src/EditableMesh_test.cpp b/src/EditableMesh_test.cpp index 5c1c5b9ca..2ec09e6c0 100644 --- a/src/EditableMesh_test.cpp +++ b/src/EditableMesh_test.cpp @@ -1024,6 +1024,78 @@ TEST(EditableMeshStandalone, LoadFromAssimpFileMixedTriAndQuadKeepsBoth) { EXPECT_EQ(sub.triangles.size(), 3u); } +TEST(EditableMeshStandalone, LoadFromAssimpFileAppliesZUpBakeWhenRequested) { + // Regression for the quads follow-up: when the source asset was + // declared Z-up (FBX UpAxis = 2), MeshProcessor bakes a +90° X + // rotation into the rendered Ogre buffers. loadFromAssimpFile must + // apply the SAME rotation when isZup=true so the editable mesh + // lives in the same basis. Without this, vertex overlays would + // appear rotated 90° on Z-up assets. + // + // OBJ ignores UpAxis metadata, so passing isZup=true here triggers + // the rotation deterministically regardless of file format. + const QString path = writeObj("editmesh_zup", + "v 0 0 0\nv 1 0 0\nv 0 1 0\n", // y=1 vertex at index 2 + "f 1 2 3\n"); + ASSERT_FALSE(path.isEmpty()); + + EditableMesh ymesh; + ASSERT_TRUE(ymesh.loadFromAssimpFile( + path.toStdString(), /*convertLH=*/false, /*isZup=*/false)); + EditableMesh zmesh; + ASSERT_TRUE(zmesh.loadFromAssimpFile( + path.toStdString(), /*convertLH=*/false, /*isZup=*/true)); + QFile::remove(path); + + ASSERT_EQ(ymesh.subMeshes()[0].vertices.size(), 3u); + ASSERT_EQ(zmesh.subMeshes()[0].vertices.size(), 3u); + + // The vertex at OBJ index 3 is (0,1,0) — Y-up. After +90° X bake + // (R_x90 * (0,1,0) = (0,0,1)) it should land on the Z axis. + const auto& y = ymesh.subMeshes()[0].vertices[2].position; + const auto& z = zmesh.subMeshes()[0].vertices[2].position; + EXPECT_NEAR(y.x, 0.0f, 1e-5f); + EXPECT_NEAR(y.y, 1.0f, 1e-5f); + EXPECT_NEAR(y.z, 0.0f, 1e-5f); + EXPECT_NEAR(z.x, 0.0f, 1e-5f); + EXPECT_NEAR(z.y, 0.0f, 1e-5f); + EXPECT_NEAR(z.z, 1.0f, 1e-5f); +} + +TEST(EditableMeshStandalone, LoadFromAssimpFileSkipsBonesNotInSkeleton) { + // Regression for the quads follow-up bone-handle bug. Without a + // skeleton lookup, this path emitted aiBone mesh-local indices as + // if they were Ogre bone handles. With skeletonForBoneHandles=null + // the legacy behaviour (mesh-local indices) is preserved for + // unskinned meshes; with a non-null skeleton, only resolvable + // bones contribute assignments. This standalone test exercises + // the unskinned-mesh path (OBJ has no bones), confirming the new + // signature compiles and behaves identically when there are no + // bones — the skinned-mesh skeleton-lookup case is exercised in + // the EditModeController integration tests where a real skeleton + // is available. + const QString path = writeObj("editmesh_nobone", + "v 0 0 0\nv 1 0 0\nv 0 1 0\n", + "f 1 2 3\n"); + ASSERT_FALSE(path.isEmpty()); + + EditableMesh m1, m2; + ASSERT_TRUE(m1.loadFromAssimpFile(path.toStdString())); + ASSERT_TRUE(m2.loadFromAssimpFile( + path.toStdString(), /*convertLH=*/true, /*isZup=*/false, + /*skeletonForBoneHandles=*/nullptr)); + QFile::remove(path); + + // Both should produce identical output for an unskinned mesh. + ASSERT_EQ(m1.subMeshCount(), 1u); + ASSERT_EQ(m2.subMeshCount(), 1u); + EXPECT_EQ(m1.subMeshes()[0].vertices.size(), + m2.subMeshes()[0].vertices.size()); + for (const auto& v : m1.subMeshes()[0].vertices) { + EXPECT_TRUE(v.boneAssignments.empty()); + } +} + TEST(EditableMeshStandalone, LoadFromAssimpFileReplacesPreviousContents) { // Loading into a non-empty EditableMesh should replace the // existing submeshes — like buildFromEditableMesh does. diff --git a/src/HalfEdgeMesh.cpp b/src/HalfEdgeMesh.cpp index 40653c393..22e490c1f 100644 --- a/src/HalfEdgeMesh.cpp +++ b/src/HalfEdgeMesh.cpp @@ -5325,22 +5325,72 @@ int HalfEdgeMesh::fillSelection(const std::vector& vertexIndices) return 0; } - // 3. Fan-triangulate from vertexIndices[0]. For n=3 emits one triangle; - // for n=4 emits two; for general N emits N-2. - int triCount = 0; - for (int i = 1; i + 1 < n; ++i) { - appendTriangle(vertexIndices[0], vertexIndices[i], - vertexIndices[i + 1], subIdx); - ++triCount; + // 3. Pick a winding that faces the same way as the surrounding mesh. + // Without this, a fill on a hole's boundary loop produces a face + // whose normal points INTO the volume (the user's selection order + // is whatever std::set / the loop walker produced; nothing + // guarantees it matches the existing topology). + // Heuristic: compare the candidate face's Newell normal to the + // average normal of every existing face that shares at least one + // of the selected vertices. If the dot product is negative the + // winding is inverted; reverse it. + auto computeNewellNormal = + [this](const std::vector& verts) -> Ogre::Vector3 { + Ogre::Vector3 normal = Ogre::Vector3::ZERO; + const size_t N = verts.size(); + for (size_t i = 0; i < N; ++i) { + const auto& a = m_vertices[verts[i]].position; + const auto& b = m_vertices[verts[(i + 1) % N]].position; + normal.x += (a.y - b.y) * (a.z + b.z); + normal.y += (a.z - b.z) * (a.x + b.x); + normal.z += (a.x - b.x) * (a.y + b.y); + } + return normal; + }; + + Ogre::Vector3 candidateNormal = computeNewellNormal(vertexIndices); + Ogre::Vector3 referenceNormal = Ogre::Vector3::ZERO; + for (int v : vertexIndices) { + for (int f : facesAroundVertex(v)) { + const auto fv = faceVertices(f); + if (fv.size() < 3) continue; + referenceNormal += computeNewellNormal(fv); + } + } + std::vector winding = vertexIndices; + if (referenceNormal.squaredLength() > 1e-12f + && candidateNormal.squaredLength() > 1e-12f + && candidateNormal.dotProduct(referenceNormal) < 0.0f) { + std::reverse(winding.begin(), winding.end()); + } + + // 4. Build the new face. For n=3 emit a single triangle (matches + // the existing fan output); for n>=4 emit a single n-gon HEFace + // so the result round-trips back through toEditableMesh as one + // EditableFace with N indices, not N-2 fan triangles. Without + // this branch, filling a 4-vertex selection would create two + // triangles whose shared diagonal then surfaces as a fan-edge + // in Edit Mode (and Standard Subdivide treats them as + // triangles, not as a quad). Returns the number of polygons + // created (always 1 here — kept as `int` to preserve the + // function signature; callers use the return only as a + // success/fail flag). + int created; + if (n == 3) { + appendTriangle(winding[0], winding[1], winding[2], subIdx); + created = 1; + } else { + const int fIdx = appendFace(winding, subIdx); + if (fIdx < 0) return 0; + created = 1; } - if (triCount == 0) return 0; rebuildEdgesAndTwins(); compactBoundaryHalfEdges(); buildBoundaryHalfEdges(); fixVertexHalfEdges(); - return triCount; + return created; } bool HalfEdgeMesh::validate() const diff --git a/src/HalfEdgeMesh_test.cpp b/src/HalfEdgeMesh_test.cpp index 9bf64eac9..3599a4e84 100644 --- a/src/HalfEdgeMesh_test.cpp +++ b/src/HalfEdgeMesh_test.cpp @@ -4293,19 +4293,35 @@ TEST(HalfEdgeMeshStandalone, FillSelectionFourVerticesEmitsTwoTriangles) { ASSERT_TRUE(he.buildFromEditableMesh(mesh)); ASSERT_EQ(activeFaceCount(he), 1); - // Fan-triangulate the (3, 4, 5, 6) quad from vertex 3. Produces - // (3, 4, 5) and (3, 5, 6) — a watertight 4-vertex fill. - EXPECT_EQ(he.fillSelection({3, 4, 5, 6}), 2); - EXPECT_EQ(activeFaceCount(he), 3); + // Fill (3, 4, 5, 6) as a single quad face — NOT a fan-triangulation. + // Pre-quads-followup this returned 2 (triangle count); now it + // returns 1 (face count) and the result round-trips back through + // toEditableMesh as one 4-index EditableFace. + EXPECT_EQ(he.fillSelection({3, 4, 5, 6}), 1); + EXPECT_EQ(activeFaceCount(he), 2); EXPECT_TRUE(he.validate()); EditableMesh back; ASSERT_TRUE(he.toEditableMesh(back)); EXPECT_TRUE(isManifold(back)); + + // The new face must round-trip as a quad EditableFace, not 2 tris. + ASSERT_EQ(back.subMeshes().size(), 1u); + const auto& subBack = back.subMeshes()[0]; + ASSERT_FALSE(subBack.faces.empty()) + << "n>=4 fill must populate EditableSubMesh::faces"; + bool sawQuad = false; + for (const auto& f : subBack.faces) { + if (f.indices.size() == 4) { sawQuad = true; break; } + } + EXPECT_TRUE(sawQuad) + << "fillSelection of 4 verts must produce a single quad face"; } -TEST(HalfEdgeMeshStandalone, FillSelectionFiveVerticesFanTriangulatesIntoThree) { - // Pentagon fan-triangulated from vertexIndices[0]: 5 vertices → 3 tris. +TEST(HalfEdgeMeshStandalone, FillSelectionFiveVerticesProducesSinglePentagon) { + // Pentagon fill: 5 vertices → ONE 5-vertex EditableFace, not 3 fan + // triangles. (Quads-followup: a single n-gon HEFace round-trips as + // a single n-gon EditableFace through toEditableMesh.) EditableMesh mesh; EditableSubMesh sub; sub.materialName = "FillMat"; @@ -4325,8 +4341,19 @@ TEST(HalfEdgeMeshStandalone, FillSelectionFiveVerticesFanTriangulatesIntoThree) ASSERT_TRUE(he.buildFromEditableMesh(mesh)); // Vertices 0..4 form a convex pentagon. Fill its loop in order. - EXPECT_EQ(he.fillSelection({0, 1, 2, 3, 4}), 3); + EXPECT_EQ(he.fillSelection({0, 1, 2, 3, 4}), 1); EXPECT_TRUE(he.validate()); + + // Confirm round-trip preserves the 5-gon. + EditableMesh back; + ASSERT_TRUE(he.toEditableMesh(back)); + ASSERT_EQ(back.subMeshes().size(), 1u); + bool sawPentagon = false; + for (const auto& f : back.subMeshes()[0].faces) { + if (f.indices.size() == 5) { sawPentagon = true; break; } + } + EXPECT_TRUE(sawPentagon) + << "fillSelection of 5 verts must produce a single pentagon face"; } // =========================================================================== @@ -4537,8 +4564,8 @@ TEST(HalfEdgeMeshStandalone, FillSelectionAcceptsOrphanedVertices) { HalfEdgeMesh he; ASSERT_TRUE(he.buildFromEditableMesh(mesh)); - // Fill with all 4 orphans → 2 new triangles (fan from vert 0). - EXPECT_EQ(he.fillSelection({0, 1, 3, 2}), 2); + // Fill with all 4 orphans → ONE new quad face (quads-followup). + EXPECT_EQ(he.fillSelection({0, 1, 3, 2}), 1); EXPECT_TRUE(he.validate()); } diff --git a/src/MeshImporterExporter.cpp b/src/MeshImporterExporter.cpp index 8e398df67..00a858195 100755 --- a/src/MeshImporterExporter.cpp +++ b/src/MeshImporterExporter.cpp @@ -904,7 +904,19 @@ void MeshImporterExporter::applyNormalMapsToEntity(const Ogre::Entity* en) auto mat = subEnt->getMaterial(); if (!mat) continue; // Ensure the material is fully loaded so TUS names are populated. - if (!mat->isLoaded()) mat->load(); + // `load()` can throw on broken/unresolvable resources; this hook + // runs on every topology mutation AND undo/redo, so a single + // bad material shouldn't abort the edit op. Skip the offending + // sub-entity and let the rest of the entity refresh. + if (!mat->isLoaded()) { + try { + mat->load(); + } catch (const Ogre::Exception& e) { + log.logMessage("applyNormalMapsToEntity: skipping mat '" + + mat->getName() + "' — load failed: " + e.getDescription()); + continue; + } + } if (mat->getNumTechniques() == 0) continue; auto* pass = mat->getTechnique(0)->getPass(0); if (!pass) continue; @@ -1023,6 +1035,18 @@ void MeshImporterExporter::importer(const QStringList &_uriList, unsigned int ad // every non-.x asset. (Chunk 4.) mesh->getUserObjectBindings().setUserAny( "qtme.source_convert_lh", Ogre::Any(convertLH)); + // Cache the source up-axis (1 = Y-up, 2 = Z-up) so + // EditModeController can apply MeshProcessor's + // +90°-around-X bake when re-importing the asset + // through the n-gon-aware path. Without this the + // editable representation lives in pre-bake space + // while the rendered buffers are post-bake — the + // overlays appear rotated 90° on FBX/glTF Z-up + // assets, and a commit would write the rotated + // positions back. (Quad migration follow-up.) + mesh->getUserObjectBindings().setUserAny( + "qtme.source_up_axis", + Ogre::Any(importer.getSceneUpAxis())); } if (!mesh) { // Animation-only file: skeleton/animations were loaded, but there is no mesh. diff --git a/src/commands/TransformCommands.cpp b/src/commands/TransformCommands.cpp index 458cc1495..e3830c36b 100644 --- a/src/commands/TransformCommands.cpp +++ b/src/commands/TransformCommands.cpp @@ -664,9 +664,10 @@ void EditMeshTopologyCommand::applyMeshState( // and skeletal skinning are preserved through undo/redo. ctrl->currentMesh()->resizeEntityBuffers(ctrl->editEntity()); - // Refresh Entity's SubEntity caches for the new vertex layout - ctrl->editEntity()->_deinitialise(); - ctrl->editEntity()->_initialise(true); + // Refresh Entity caches + RTSS state — same hook every topology op + // uses, so undo/redo preserves bump map / per-pixel lighting on + // bump-mapped assets through the n-gon import path. + EditModeController::rewriteEntityAfterTopologyChange(ctrl->editEntity()); // Restore full selection state (vertices, edges, and faces) ctrl->deselectAll();