fix(mocap): retarget live body drive via landmark directions - #939
fix(mocap): retarget live body drive via landmark directions#939fernandotonon wants to merge 8 commits into
Conversation
Drive Mixamo skeletons from MediaPipe landmark segment directions (same aim math as applyMotionClip) instead of quaternion deltas and mirror-L/R swaps. Adds PoseIK debug overlay, per-frame skinning refresh, and tests. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughChangesMocap capture and retargeting
ONNX runtime configuration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant VideoFrameSource
participant MocapController
participant PoseIKSolver
participant BodyRetargeter
participant MocapPoseDebugOverlay
VideoFrameSource->>MocapController: deliver decoded QVideoFrame
MocapController->>PoseIKSolver: solve canonical body landmarks
PoseIKSolver-->>MocapController: return quaternions and landmark directions
MocapController->>BodyRetargeter: evaluate frame with neutral reference
BodyRetargeter-->>MocapController: return retargeted skeleton locals
MocapController->>MocapPoseDebugOverlay: render live landmarks and PoseIK segments
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 8d707b632c
ℹ️ 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".
| // stream recordBody expects (identity for roles unresolved that frame), and | ||
| // push a SEPARATE undo command so face + body each undo cleanly. | ||
| if (!d->bodyBones.empty() && d->bodyTake.size() >= 2) { | ||
| if (d->bodyRetargeter && d->bodyTake.size() >= 2) { |
There was a problem hiding this comment.
When body recording is enabled, this branch still serializes only bf.quats, even though the live preview now uses bf.world and bf.visibility as its primary landmark-direction inputs. MocapRecorder::recordBody() consequently falls back to applyMotionClip() with an empty clipRestDir, so motions that were corrected by the new direction-based live path are baked through the old quaternion path and can play back differently from what the user recorded. Pass the landmark stream/neutral anchors into the bake or bake the live retargeter's evaluated locals.
Useful? React with 👍 / 👎.
| if (c < 0 || c >= Jc) | ||
| continue; |
There was a problem hiding this comment.
On rigs containing an unmapped helper or twist bone between canonical bones, this continue leaves W[i] without that bone's actual world transform. A later mapped descendant reads that entry as Wp and computes Wp.Inverse() * Wt, producing an invalid local orientation (and Ogre quaternion defaults may be uninitialized). Populate W for every hierarchy node using its standing/bind local transform even when the node itself is not emitted.
Useful? React with 👍 / 👎.
| BodyManualBoneSnapshot snap; | ||
| snap.boneName = bone->getName(); | ||
| snap.bindLocal = bone->getOrientation(); | ||
| snap.wasManuallyControlled = bone->isManuallyControlled(); | ||
| d->bodyManualRestore.push_back(std::move(snap)); |
There was a problem hiding this comment.
When both Head and Body are enabled, the head is marked manually controlled before this body snapshot, so snap.wasManuallyControlled is always true even if it was false before preview. During restoration the dedicated head snapshot first restores false, but the subsequent body snapshot loop overwrites it back to true, leaving the head outside normal animation control after every body preview. Snapshot all body bones before enabling the head, exclude the head from this list, or reverse the restoration precedence.
Useful? React with 👍 / 👎.
| if (!d->skinningListener) | ||
| d->skinningListener = std::make_unique<Impl::SkinningFrameListener>(); | ||
| d->skinningListener->impl = d.get(); | ||
| Ogre::Root::getSingleton().addFrameListener(d->skinningListener.get()); |
There was a problem hiding this comment.
The listener is registered before camera->open() and predictor loading, but either failure path only calls restoreEntityState() and returns while the controller remains Idle; stopPreview() therefore cannot reach the matching removeFrameListener(). A failed preview leaves Ogre holding this listener, and later retries can register it again or controller destruction can leave a dangling callback. Register it only after all fallible setup succeeds or remove it on every early-return cleanup path.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Mocap/MocapRecorder.cpp (1)
339-345: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe validation harness no longer mirrors the live path.
The comment states that this branch runs "the EXACT math the LIVE preview runs". After this PR the live preview drives
BodyRetargeterwith MediaPipe landmark directions, but this call passes only canonical quaternions (rt.setNeutralReference(q)andrt.evaluateFrame(q, 0xFFFFFFFFu)), so the retargeter takes the quaternion-delta branch. A rendered clip fromQTMESH_MOCAP_USE_RETARGETER=1therefore validates a different code path.If the recorder has the source landmarks per frame, pass them to both calls. If it does not, correct the comment so it does not promise live-path parity.
Also applies to: 363-364
🤖 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/Mocap/MocapRecorder.cpp` around lines 339 - 345, Update the validation harness around BodyRetargeter rt, including its setNeutralReference and evaluateFrame calls, to pass the per-frame MediaPipe landmark directions used by the live preview when those source landmarks are available, preserving the live retargeter branch for both initialization and evaluation. If MocapRecorder lacks those landmarks, revise the nearby “EXACT math the LIVE preview runs” comment to state that the harness uses the quaternion-delta path instead of claiming live-path parity.
🧹 Nitpick comments (16)
src/Mocap/MocapLiveTypes.h (1)
12-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDefault-initialize
quatsfor consistency.
worldandvisibilityuse{}, butquatsdoes not. ABodyLiveFrame body;declaration (seesrc/Mocap/MocapController.cppline 202) therefore leavesquatsindeterminate untilvalidis set. The current consumers guard onvalid, so this is not an active defect. Add the initializer to keep the type safe under future changes.♻️ Proposed change
- std::array<std::array<float, 4>, PoseIK::kCanonicalRoles> quats; + std::array<std::array<float, 4>, PoseIK::kCanonicalRoles> quats{};🤖 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/Mocap/MocapLiveTypes.h` around lines 12 - 18, Update the quats member initializer in BodyLiveFrame to value-initialize it with {}, matching world and visibility so default-constructed frames have initialized quaternion data.src/Mocap/MocapPoseDebugOverlay.cpp (2)
301-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
m_lastScaleis written but never read.Remove the field, or use it to skip a rebuild when the scale is unchanged.
🤖 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/Mocap/MocapPoseDebugOverlay.cpp` at line 301, Address the unused m_lastScale assignment in the MocapPoseDebugOverlay update path: either remove the m_lastScale field and its write, or use it to detect an unchanged scale and skip the rebuild while preserving rebuilds when the scale changes.
232-233: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueScale the overlay offset with the entity size.
setPosition(0.8f, 0.f, 0.f)is a fixed local offset, butupdate()scales the stick figure to the entity height. For a tall or a small entity the overlay overlaps the mesh. Derive the offset from the same height value thatupdate()receives.🤖 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/Mocap/MocapPoseDebugOverlay.cpp` around lines 232 - 233, Update the overlay root positioning in MocapPoseDebugOverlay initialization to derive the x offset from the entity height used by update(), replacing the fixed 0.8f value while preserving the zero y and z offsets. Reuse the existing height-related symbol or pass the height into the setup path so the overlay scales consistently with the stick figure.src/Mocap/MocapController.cpp (2)
145-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider one bone snapshot type.
BodyDriveBoneandBodyManualBoneSnapshothold the same three fields plus arole.BodyDriveBonewithrole = -1already covers the snapshot case. One type reduces the parallel bookkeeping inbeginPreviewWithLiveSourceandrestoreEntityState.🤖 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/Mocap/MocapController.cpp` around lines 145 - 156, Replace BodyManualBoneSnapshot with BodyDriveBone, using role = -1 for snapshot entries. Update beginPreviewWithLiveSource and restoreEntityState to use the unified type and remove the redundant snapshot struct while preserving existing boneName, bindLocal, and wasManuallyControlled handling.
279-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the raw role bit literals with the
PoseIKenumerators.
kTorsoResolvedMaskuses(1u << 0) | (1u << 1) | (1u << 2)andskipHeaduses(1u << 5). ThePoseIKrole enumerators already name these indices. Literals break silently if the role order changes.♻️ Proposed change
- static constexpr uint32_t kTorsoResolvedMask = - (1u << 0) | (1u << 1) | (1u << 2); // hip, abdomen, chest + static constexpr uint32_t kTorsoResolvedMask = + (1u << PoseIK::Hip) | (1u << PoseIK::Abdomen) | (1u << PoseIK::Chest);- const uint32_t skipHead = - (d->headEnabled && !d->headBone.isEmpty()) ? (1u << 5) : 0u; + const uint32_t skipHead = (d->headEnabled && !d->headBone.isEmpty()) + ? (1u << PoseIK::Head) + : 0u;Also applies to: 1064-1065
🤖 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/Mocap/MocapController.cpp` around lines 279 - 280, Replace the raw bit shifts in kTorsoResolvedMask and skipHead with the corresponding PoseIK role enumerators for hip, abdomen, chest, and head, while preserving the existing mask behavior. Use the enumerator-based shifts so role index changes remain synchronized with these masks.src/Mocap/MocapBodyDriveDebug.cpp (2)
147-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
PoseIK::kCanonicalRolesfor the bound.
roleNamecompares against the literal22. UsePoseIK::kCanonicalRoles, and add a static assertion that the name table matches that count.♻️ Proposed change
- if (role >= 0 && role < 22) + static_assert(std::size(names) == PoseIK::kCanonicalRoles, + "role name table out of sync"); + if (role >= 0 && role < PoseIK::kCanonicalRoles) return names[role];🤖 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/Mocap/MocapBodyDriveDebug.cpp` around lines 147 - 158, Update roleName to use PoseIK::kCanonicalRoles instead of the literal 22 when validating role indices, and add a static assertion confirming the names table contains exactly that number of entries. Preserve the existing "?" fallback for out-of-range roles.
234-253: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueResolve the role-to-bone map once per frame.
The bone lookup scans every skeleton bone and calls
MotionInbetween::canonicalIndexForBonefor each of the ten debug roles. That is10 * numBonesstring conversions per logged frame. Build one role-to-bone table before the loop.🤖 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/Mocap/MocapBodyDriveDebug.cpp` around lines 234 - 253, Build the role-to-bone table once before the kLimbRoles loop by scanning skel’s bones a single time, converting each bone name with MotionInbetween::canonicalIndexForBone, and storing the matching bone and child relationship by role. Update the loop to retrieve entries from this table while preserving the resolvedMask filtering and existing child selection behavior.src/Mocap/MocapPoseDebugOverlay.h (1)
17-33: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDocument or enforce the detach requirement.
The class owns an
Ogre::SceneNodeand twoOgre::ManualObjectinstances but has no destructor. If the owner is destroyed while the overlay is attached, those scene objects stay in the scene manager. Add a destructor that callsdetach(), or state the "caller must calldetach()before destruction" contract in a comment.🤖 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/Mocap/MocapPoseDebugOverlay.h` around lines 17 - 33, Add a destructor to MocapPoseDebugOverlay that calls detach(), ensuring the owned scene node and ManualObject instances are removed when an attached overlay is destroyed. Keep the existing detach cleanup behavior and avoid requiring callers to remember an additional lifecycle step.src/Mocap/PoseIKSolver_test.cpp (2)
211-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the comment with the assertion.
The comment mentions yaw, but the assertion measures the Hip role rotation. Reword the comment to state that the torso must stay stable during a nod.
🤖 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/Mocap/PoseIKSolver_test.cpp` around lines 211 - 212, Update the comment above the EXPECT_LT assertion in the nodding test to state that the torso remains stable during a nod, matching the asserted PoseIK::Hip rotation behavior; leave the assertion unchanged.
236-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a fresh
Solverfor the corrected pass.
PoseIK::Solverkeeps per-frame history (m_prevSecondary,m_prevPrimary,m_prevQuats,m_prevHipLine).fOkis the third frame on the same solver, so the twist reference and quaternion hold-over from the mirrored frame feed into it. The assertions then depend on frame order rather than on the swap itself. Solve the corrected landmarks with a second solver that starts from the same neutral baseline.♻️ Proposed change
Landmarks fixed = tPose(); + PoseIK::Solver fixedSolver; + const auto fixedBase = fixedSolver.solveFrame(fixed.data()); set(fixed, 15, 0.18f, -0.75f, 0.f); set(fixed, 13, 0.18f, -0.55f, 0.f); MocapPoseFix::swapMediaPipeLeftRightLandmarks(fixed.data()); - const auto fOk = solver.solveFrame(fixed.data()); - const double fixedRight = quatAngle(f0.quats[PoseIK::RShoulder], + const auto fOk = fixedSolver.solveFrame(fixed.data()); + const double fixedRight = quatAngle(fixedBase.quats[PoseIK::RShoulder], fOk.quats[PoseIK::RShoulder]);🤖 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/Mocap/PoseIKSolver_test.cpp` around lines 236 - 262, Use a separate PoseIK::Solver instance for the corrected landmark pass in PoseIKSolver.WebcamMirrorSwapFixesRightArmRaise. Initialize the new solver from the same neutral tPose baseline, then solve the swapped fixed landmarks with it so fOk is not influenced by fWrong’s per-frame history; keep the existing assertions and comparison targets unchanged.src/Mocap/PoseIKSolver.cpp (2)
235-239: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRemove the redundant landmark copy.
canonalready holds the canonical points. The extraparray copies 33 vectors on every frame. Usecanondirectly and keep the existingp[...]accesses working through a reference.♻️ Proposed change
- Vec3 p[kLandmarkCount]; - std::array<std::array<float, 3>, kLandmarkCount> canon{}; - canonicalizeMediaPipeWorld(world, canon); - for (int i = 0; i < kLandmarkCount; ++i) - p[i] = canon[static_cast<size_t>(i)]; + std::array<std::array<float, 3>, kLandmarkCount> canon{}; + canonicalizeMediaPipeWorld(world, canon); + const auto& p = canon;🤖 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/Mocap/PoseIKSolver.cpp` around lines 235 - 239, In the pose-solving routine around canonicalizeMediaPipeWorld, remove the redundant p array and population loop, then bind p as a reference or alias to canon so existing p[...] accesses continue working while using the canonical points directly.
110-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one segment table and use the
Lmenum.This table duplicates the limb table in
solveFrame(lines 318-325) and uses raw landmark indices instead of theLmenumerators used everywhere else in this file. Two copies can diverge silently, and a wrong literal is hard to spot. Move the table to the anonymous namespace and reuse it in both places.♻️ Proposed change
- struct Segment { - Role r; - int from, to; - }; - static const Segment segments[] = { - {RShoulder, 12, 14}, {RElbow, 14, 16}, - {LShoulder, 11, 13}, {LElbow, 13, 15}, - {RHip, 24, 26}, {RKnee, 26, 28}, {RFoot, 28, 32}, - {LHip, 23, 25}, {LKnee, 25, 27}, {LFoot, 27, 31}, - }; + // kLimbSegments lives in the anonymous namespace and is shared with solveFrame. + for (const LimbSegment& seg : kLimbSegments) {Define once near the
Lmenum:struct LimbSegment { Role role; int from, to; }; constexpr LimbSegment kLimbSegments[] = { {RShoulder, RShoulderLm, RElbowLm}, {RElbow, RElbowLm, RWrist}, {LShoulder, LShoulderLm, LElbowLm}, {LElbow, LElbowLm, LWrist}, {RHip, RHipLm, RKneeLm}, {RKnee, RKneeLm, RAnkle}, {RFoot, RAnkle, RFootIndex}, {LHip, LHipLm, LKneeLm}, {LKnee, LKneeLm, LAnkle}, {LFoot, LAnkle, LFootIndex}, };🤖 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/Mocap/PoseIKSolver.cpp` around lines 110 - 119, Move the limb segment definition near the Lm enum in the anonymous namespace, rename it to the shared LimbSegment/kLimbSegments form, and replace raw landmark literals with the appropriate Lm enumerators. Remove the local duplicate Segment table and update both the surrounding solver logic and solveFrame to iterate over kLimbSegments, preserving the existing segment ordering and roles.src/AnimationMerger_test.cpp (1)
1337-1357: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that the untouched arm stays still.
The test raises only the left elbow and wrist landmarks and asserts left-arm motion. The PR objectives list "independent arm movement" as unvalidated. Add a check on the right upper arm so a regression that couples both arms fails here.
💚 Proposed extra assertion
auto armDir = [&]() -> Ogre::Vector3 { skelInst->_updateTransforms(); return (skelInst->getBone("LeftForeArm")->_getDerivedPosition() - skelInst->getBone("LeftArm")->_getDerivedPosition()) .normalisedCopy(); }; + auto rightArmDir = [&]() -> Ogre::Vector3 { + skelInst->_updateTransforms(); + return (skelInst->getBone("RightForeArm")->_getDerivedPosition() + - skelInst->getBone("RightArm")->_getDerivedPosition()) + .normalisedCopy(); + }; @@ applyLocals(rt.evaluateFrame(neutralFr.quats, neutralFr.resolvedMask, 0, neutralLm.data(), nullptr)); const Ogre::Vector3 tPoseArm = armDir(); + const Ogre::Vector3 tPoseRightArm = rightArmDir(); @@ const float raisedMotion = degBetween(tPoseArm, armDir()); EXPECT_GT(raisedMotion, 25.0f); + // The right-arm landmarks did not move, so that arm must hold still. + EXPECT_LT(degBetween(tPoseRightArm, rightArmDir()), 5.0f);🤖 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/AnimationMerger_test.cpp` around lines 1337 - 1357, Extend the test around the neutral and raised evaluations to capture the right upper-arm direction alongside tPoseArm, then compute its motion after applying raisedLocals. Add an assertion that the untouched right arm remains effectively stationary, while preserving the existing raisedMotion assertion for the left arm.docs/MOCAP.md (1)
137-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the new PoseIK debug overlay.
The panel now exposes "Show PoseIK debug skeleton" with cyan MediaPipe landmarks and a yellow FK stick figure. This is the main triage tool for a wrong retarget, but the document does not mention it. Add a short line in the live-mode or troubleshooting section.
🤖 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 `@docs/MOCAP.md` around lines 137 - 146, Update the live-mode or troubleshooting documentation near the existing PoseIK behavior notes to mention the “Show PoseIK debug skeleton” panel option, including that it displays cyan MediaPipe landmarks and a yellow FK stick figure for diagnosing retargeting issues.src/AnimationMerger.cpp (1)
1491-1506: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
kParentCanonagainst a canonical-joint-count change.
effectiveParentRoleindexes the fixed 22-entrykParentCanonwithrole. Callers boundrolebyJc = MotionInbetween::canonicalJointCount(). If that count ever grows, the read goes out of bounds. Add a compile-time check next to the table.♻️ Proposed guard
constexpr int kParentCanon[22] = { -1, 0, 1, 2, 3, 4, // hip, abdomen, chest, neck, neck1, head 2, 6, 7, 8, // rcollar, rshoulder, relbow, rhand 2, 10, 11, 12, // lcollar, lshoulder, lelbow, lhand 0, 14, 15, 16, // rbuttock, rhip, rknee, rfoot 0, 18, 19, 20 }; // lbuttock, lhip, lknee, lfoot + +static_assert(std::size(kParentCanon) == 22, + "kParentCanon must cover every canonical role");Add a runtime check in the retargeter constructor as well, for example
d->Jc = std::min(MotionInbetween::canonicalJointCount(), 22);, or assert equality.🤖 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/AnimationMerger.cpp` around lines 1491 - 1506, Add a compile-time assertion immediately after the kParentCanon declaration requiring its size to match the supported 22 canonical joints, preventing effectiveParentRole from indexing beyond the table if the canonical-joint count changes. Also enforce the same bound in the retargeter constructor when assigning d->Jc, either by asserting equality or clamping the canonicalJointCount() result to the table capacity.src/Mocap/MocapPoseFix.h (1)
20-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep the live mirror helper and remove the legacy yaw helpers.
swapMediaPipeLeftRightLandmarksis still used byPoseIKSolver_test, whileswapCanonicalLeftRight,kPoseToSkeletonYawPi,invertCameraYawDelta,poseDirectionToSkeleton, andposeRotationToSkeletonhave no callers outside the header. Remove those unused helpers, or add a comment noting their remaining use if they are kept for future legacy clip support.🤖 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/Mocap/MocapPoseFix.h` around lines 20 - 58, Keep swapMediaPipeLeftRightLandmarks because PoseIKSolver_test still uses it, but remove the unused legacy helpers swapCanonicalLeftRight, kPoseToSkeletonYawPi, invertCameraYawDelta, poseDirectionToSkeleton, and poseRotationToSkeleton from the header. If any are intentionally retained for future legacy clip support, add a clear comment documenting that purpose.
🤖 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 `@qml/PropertiesPanel.qml`:
- Around line 638-645: Add a contentReady signal to CollapsibleSection, then
emit it from the contentLoader onLoaded handler after the section content
finishes loading. This enables the existing onContentReady handler in
PropertiesPanel to invoke MocapController.refreshDevices when the section is
expanded.
In `@src/AnimationMerger.cpp`:
- Around line 1793-1832: Update the loop building the live animation output so
bones skipped for invalid canonical mappings or skipRolesMask still compute and
store their bind/stand world transform in W[i] as Wp * base before continuing.
Keep these bones excluded from out, and preserve the existing transformed-role
handling for emitted bones so descendants use the correct parent world.
In `@src/Mocap/MocapBodyDriveDebug.cpp`:
- Around line 69-145: Consolidate fkPoseIkJoints and its duplicated helpers
effectiveParentRole, localArtic, and quatFromArray into one shared
implementation used by both diagnostics. Preserve the overlay behavior in the
root-less resolved-role branch by assigning worldRot for the current role from
its quaternion before placing descendants, so both callers produce identical
results.
- Around line 13-16: Add direct includes for the symbols used by
MocapBodyDriveDebug.cpp: include <algorithm> for std::max/std::min, <QtGlobal>
for qEnvironmentVariableIsSet, and <QString> for QString::fromStdString, rather
than relying on transitive includes from AnimationMerger.h or MotionInbetween.h.
In `@src/Mocap/MocapController.cpp`:
- Around line 774-781: Update the MocapController failure paths following
addFrameListener to remove the registered skinningListener before each return
false, and add equivalent cleanup to ~MocapController. Reuse the existing
stopPreview cleanup behavior or Ogre::Root::removeFrameListener with the same
listener, ensuring no registered listener can outlive the controller.
- Around line 439-445: Update the pose debug overlay attachment logic in
MocapController to use Manager::getSingletonPtr() and verify both the manager
and getSceneMgr() result before dereferencing or attaching. Apply the same guard
to the corresponding overlay attachment logic near the second affected location,
while preserving detach behavior when prerequisites are unavailable.
- Around line 366-369: Initialize d->cachedDevices by calling refreshDevices()
during construction in the normal available/non-available constructor path,
before startup model readers can access availableDevices(). Preserve the
existing behavior for unavailable devices and ensure availableDevices() returns
the populated initial cache.
In `@src/Mocap/MocapPoseDebugOverlay.cpp`:
- Around line 105-174: Move fkPoseIkJoints and the effectiveParentRole,
localArtic, and quatFromArray helpers from src/Mocap/MocapPoseDebugOverlay.cpp
lines 105-174 into a shared internal header near MocapLiveTypes.h, then update
the overlay’s update() to call that shared implementation. Remove the duplicate
helpers and FK implementation from src/Mocap/MocapBodyDriveDebug.cpp lines
69-145 and call the shared helper there, preserving the overlay’s worldRot
handling for resolved roles with unresolved parents so both debug consumers
produce identical joint positions.
- Around line 190-205: Update MocapPoseDebugOverlay::ensureMaterial to ensure
the first technique contains at least one pass before retrieving and configuring
pass 0. Preserve the existing material and technique creation flow, adding pass
creation when needed before the getPass(0) call.
In `@src/Mocap/VideoFrameSource_test.cpp`:
- Around line 201-210: Extend the MocapFrameFromVideoFrame tests to cover the
mapped Format_Jpeg branch by constructing a QVideoFrame containing JPEG-encoded
image data and passing it through mocapFrameFromVideoFrame. Assert that decoding
produces the expected known pixel value and QImage::Format_RGB888, while
preserving the existing RGB conversion test.
---
Outside diff comments:
In `@src/Mocap/MocapRecorder.cpp`:
- Around line 339-345: Update the validation harness around BodyRetargeter rt,
including its setNeutralReference and evaluateFrame calls, to pass the per-frame
MediaPipe landmark directions used by the live preview when those source
landmarks are available, preserving the live retargeter branch for both
initialization and evaluation. If MocapRecorder lacks those landmarks, revise
the nearby “EXACT math the LIVE preview runs” comment to state that the harness
uses the quaternion-delta path instead of claiming live-path parity.
---
Nitpick comments:
In `@docs/MOCAP.md`:
- Around line 137-146: Update the live-mode or troubleshooting documentation
near the existing PoseIK behavior notes to mention the “Show PoseIK debug
skeleton” panel option, including that it displays cyan MediaPipe landmarks and
a yellow FK stick figure for diagnosing retargeting issues.
In `@src/AnimationMerger_test.cpp`:
- Around line 1337-1357: Extend the test around the neutral and raised
evaluations to capture the right upper-arm direction alongside tPoseArm, then
compute its motion after applying raisedLocals. Add an assertion that the
untouched right arm remains effectively stationary, while preserving the
existing raisedMotion assertion for the left arm.
In `@src/AnimationMerger.cpp`:
- Around line 1491-1506: Add a compile-time assertion immediately after the
kParentCanon declaration requiring its size to match the supported 22 canonical
joints, preventing effectiveParentRole from indexing beyond the table if the
canonical-joint count changes. Also enforce the same bound in the retargeter
constructor when assigning d->Jc, either by asserting equality or clamping the
canonicalJointCount() result to the table capacity.
In `@src/Mocap/MocapBodyDriveDebug.cpp`:
- Around line 147-158: Update roleName to use PoseIK::kCanonicalRoles instead of
the literal 22 when validating role indices, and add a static assertion
confirming the names table contains exactly that number of entries. Preserve the
existing "?" fallback for out-of-range roles.
- Around line 234-253: Build the role-to-bone table once before the kLimbRoles
loop by scanning skel’s bones a single time, converting each bone name with
MotionInbetween::canonicalIndexForBone, and storing the matching bone and child
relationship by role. Update the loop to retrieve entries from this table while
preserving the resolvedMask filtering and existing child selection behavior.
In `@src/Mocap/MocapController.cpp`:
- Around line 145-156: Replace BodyManualBoneSnapshot with BodyDriveBone, using
role = -1 for snapshot entries. Update beginPreviewWithLiveSource and
restoreEntityState to use the unified type and remove the redundant snapshot
struct while preserving existing boneName, bindLocal, and wasManuallyControlled
handling.
- Around line 279-280: Replace the raw bit shifts in kTorsoResolvedMask and
skipHead with the corresponding PoseIK role enumerators for hip, abdomen, chest,
and head, while preserving the existing mask behavior. Use the enumerator-based
shifts so role index changes remain synchronized with these masks.
In `@src/Mocap/MocapLiveTypes.h`:
- Around line 12-18: Update the quats member initializer in BodyLiveFrame to
value-initialize it with {}, matching world and visibility so
default-constructed frames have initialized quaternion data.
In `@src/Mocap/MocapPoseDebugOverlay.cpp`:
- Line 301: Address the unused m_lastScale assignment in the
MocapPoseDebugOverlay update path: either remove the m_lastScale field and its
write, or use it to detect an unchanged scale and skip the rebuild while
preserving rebuilds when the scale changes.
- Around line 232-233: Update the overlay root positioning in
MocapPoseDebugOverlay initialization to derive the x offset from the entity
height used by update(), replacing the fixed 0.8f value while preserving the
zero y and z offsets. Reuse the existing height-related symbol or pass the
height into the setup path so the overlay scales consistently with the stick
figure.
In `@src/Mocap/MocapPoseDebugOverlay.h`:
- Around line 17-33: Add a destructor to MocapPoseDebugOverlay that calls
detach(), ensuring the owned scene node and ManualObject instances are removed
when an attached overlay is destroyed. Keep the existing detach cleanup behavior
and avoid requiring callers to remember an additional lifecycle step.
In `@src/Mocap/MocapPoseFix.h`:
- Around line 20-58: Keep swapMediaPipeLeftRightLandmarks because
PoseIKSolver_test still uses it, but remove the unused legacy helpers
swapCanonicalLeftRight, kPoseToSkeletonYawPi, invertCameraYawDelta,
poseDirectionToSkeleton, and poseRotationToSkeleton from the header. If any are
intentionally retained for future legacy clip support, add a clear comment
documenting that purpose.
In `@src/Mocap/PoseIKSolver_test.cpp`:
- Around line 211-212: Update the comment above the EXPECT_LT assertion in the
nodding test to state that the torso remains stable during a nod, matching the
asserted PoseIK::Hip rotation behavior; leave the assertion unchanged.
- Around line 236-262: Use a separate PoseIK::Solver instance for the corrected
landmark pass in PoseIKSolver.WebcamMirrorSwapFixesRightArmRaise. Initialize the
new solver from the same neutral tPose baseline, then solve the swapped fixed
landmarks with it so fOk is not influenced by fWrong’s per-frame history; keep
the existing assertions and comparison targets unchanged.
In `@src/Mocap/PoseIKSolver.cpp`:
- Around line 235-239: In the pose-solving routine around
canonicalizeMediaPipeWorld, remove the redundant p array and population loop,
then bind p as a reference or alias to canon so existing p[...] accesses
continue working while using the canonical points directly.
- Around line 110-119: Move the limb segment definition near the Lm enum in the
anonymous namespace, rename it to the shared LimbSegment/kLimbSegments form, and
replace raw landmark literals with the appropriate Lm enumerators. Remove the
local duplicate Segment table and update both the surrounding solver logic and
solveFrame to iterate over kLimbSegments, preserving the existing segment
ordering and roles.
🪄 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 Plus
Run ID: fe7df47d-a7db-4c85-bdea-d548f906fc56
📒 Files selected for processing (25)
docs/MOCAP.mdqml/PropertiesPanel.qmlsrc/AnimationMerger.cppsrc/AnimationMerger.hsrc/AnimationMerger_test.cppsrc/CMakeLists.txtsrc/Mocap/FaceCapPose_test.cppsrc/Mocap/MocapBodyDriveDebug.cppsrc/Mocap/MocapBodyDriveDebug.hsrc/Mocap/MocapCameraHints.cppsrc/Mocap/MocapCameraHints.hsrc/Mocap/MocapController.cppsrc/Mocap/MocapController.hsrc/Mocap/MocapLiveTypes.hsrc/Mocap/MocapPoseDebugOverlay.cppsrc/Mocap/MocapPoseDebugOverlay.hsrc/Mocap/MocapPoseFix.hsrc/Mocap/MocapRecorder.cppsrc/Mocap/PoseIKSolver.cppsrc/Mocap/PoseIKSolver.hsrc/Mocap/PoseIKSolver_test.cppsrc/Mocap/VideoFrameSource.cppsrc/Mocap/VideoFrameSource.hsrc/Mocap/VideoFrameSource_test.cppsrc/test_main.cpp
| #include <array> | ||
| #include <cmath> | ||
| #include <cstdio> | ||
| #include <vector> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Include the headers this file uses directly.
The file calls std::max and std::min (lines 39-41, 110-112, 198-201), qEnvironmentVariableIsSet (line 183), and QString::fromStdString (line 243). <algorithm>, <QtGlobal>, and <QString> are not included. The build currently depends on transitive includes from AnimationMerger.h and MotionInbetween.h, which differ between libstdc++, libc++, and MSVC.
🛠️ Proposed change
`#include` <array>
+#include <algorithm>
`#include` <cmath>
`#include` <cstdio>
`#include` <vector>
+
+#include <QString>
+#include <QtGlobal>Based on the coding guideline "Ensure code compiles and runs on Windows, Linux, and macOS".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <array> | |
| #include <cmath> | |
| #include <cstdio> | |
| #include <vector> | |
| `#include` <array> | |
| `#include` <algorithm> | |
| `#include` <cmath> | |
| `#include` <cstdio> | |
| `#include` <vector> | |
| `#include` <QString> | |
| `#include` <QtGlobal> |
🤖 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/Mocap/MocapBodyDriveDebug.cpp` around lines 13 - 16, Add direct includes
for the symbols used by MocapBodyDriveDebug.cpp: include <algorithm> for
std::max/std::min, <QtGlobal> for qEnvironmentVariableIsSet, and <QString> for
QString::fromStdString, rather than relying on transitive includes from
AnimationMerger.h or MotionInbetween.h.
Source: Coding guidelines
| void fkPoseIkJoints( | ||
| const std::array<std::array<float, 4>, PoseIK::kCanonicalRoles>& quats, | ||
| uint32_t resolvedMask, | ||
| const std::array<std::array<float, 3>, PoseIK::kLandmarkCount>& canonLmPts, | ||
| std::array<Vec3, PoseIK::kCanonicalRoles>& out) | ||
| { | ||
| out.fill({0.f, 0.f, 0.f}); | ||
| const Vec3 hip = { | ||
| (canonLmPts[23][0] + canonLmPts[24][0]) * 0.5f, | ||
| (canonLmPts[23][1] + canonLmPts[24][1]) * 0.5f, | ||
| (canonLmPts[23][2] + canonLmPts[24][2]) * 0.5f}; | ||
| out[static_cast<size_t>(PoseIK::Hip)] = hip; | ||
|
|
||
| struct BoneSeg { | ||
| int role; | ||
| int fromLm; | ||
| int toLm; | ||
| }; | ||
| static const BoneSeg segs[] = { | ||
| {PoseIK::Abdomen, 23, 11}, {PoseIK::Chest, 11, 12}, | ||
| {PoseIK::Neck, 12, 0}, {PoseIK::Head, 0, 8}, | ||
| {PoseIK::RShoulder, 12, 14}, {PoseIK::RElbow, 14, 16}, | ||
| {PoseIK::RHand, 16, 16}, | ||
| {PoseIK::LShoulder, 11, 13}, {PoseIK::LElbow, 13, 15}, | ||
| {PoseIK::LHand, 15, 15}, | ||
| {PoseIK::RHip, 24, 26}, {PoseIK::RKnee, 26, 28}, | ||
| {PoseIK::RFoot, 28, 32}, | ||
| {PoseIK::LHip, 23, 25}, {PoseIK::LKnee, 25, 27}, | ||
| {PoseIK::LFoot, 27, 31}, | ||
| }; | ||
|
|
||
| std::array<Vec3, PoseIK::kCanonicalRoles> restOffset{}; | ||
| for (const BoneSeg& s : segs) { | ||
| Vec3 dir = sub(canonLmPts[static_cast<size_t>(s.toLm)], | ||
| canonLmPts[static_cast<size_t>(s.fromLm)]); | ||
| const float d = len(dir); | ||
| if (d < 1e-5f) | ||
| dir = {0.f, 0.12f, 0.f}; | ||
| else | ||
| dir = norm(dir); | ||
| restOffset[static_cast<size_t>(s.role)] = { | ||
| dir[0] * std::max(d, 0.05f), | ||
| dir[1] * std::max(d, 0.05f), | ||
| dir[2] * std::max(d, 0.05f)}; | ||
| } | ||
|
|
||
| std::array<Ogre::Quaternion, PoseIK::kCanonicalRoles> worldRot{}; | ||
| worldRot.fill(Ogre::Quaternion::IDENTITY); | ||
|
|
||
| for (int role = 0; role < PoseIK::kCanonicalRoles; ++role) { | ||
| if (!(resolvedMask & (1u << static_cast<unsigned>(role)))) | ||
| continue; | ||
| const int parent = MotionInbetween::canonicalParentOf(role); | ||
| const Ogre::Quaternion local = localArtic(quats, role, resolvedMask); | ||
| if (parent >= 0 && (resolvedMask & (1u << static_cast<unsigned>(parent)))) { | ||
| worldRot[static_cast<size_t>(role)] = | ||
| worldRot[static_cast<size_t>(parent)] * local; | ||
| const Ogre::Vector3 off( | ||
| restOffset[static_cast<size_t>(role)][0], | ||
| restOffset[static_cast<size_t>(role)][1], | ||
| restOffset[static_cast<size_t>(role)][2]); | ||
| const Ogre::Vector3 w = | ||
| worldRot[static_cast<size_t>(parent)] * off; | ||
| out[static_cast<size_t>(role)] = { | ||
| out[static_cast<size_t>(parent)][0] + w.x, | ||
| out[static_cast<size_t>(parent)][1] + w.y, | ||
| out[static_cast<size_t>(parent)][2] + w.z}; | ||
| } else if (role == PoseIK::Hip) { | ||
| worldRot[0] = quatFromArray(quats[0]); | ||
| } else { | ||
| out[static_cast<size_t>(role)] = { | ||
| hip[0] + restOffset[static_cast<size_t>(role)][0], | ||
| hip[1] + restOffset[static_cast<size_t>(role)][1], | ||
| hip[2] + restOffset[static_cast<size_t>(role)][2]}; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
fkPoseIkJoints is duplicated and the two copies differ.
This implementation and the one in src/Mocap/MocapPoseDebugOverlay.cpp (lines 105-174) are the same algorithm. The final else branch here does not set worldRot[role], while the overlay copy sets worldRot[role] = quatFromArray(quats[role]). Any child of a root-less resolved role is therefore placed with an identity parent rotation in the log but not in the overlay, so the two diagnostics disagree. effectiveParentRole, localArtic, and quatFromArray are duplicated as well.
See the consolidated comment for the shared fix.
🧰 Tools
🪛 Cppcheck (2.21.0)
[error] 80-80: Address of local auto-variable assigned to a function parameter.
(autoVariables)
🤖 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/Mocap/MocapBodyDriveDebug.cpp` around lines 69 - 145, Consolidate
fkPoseIkJoints and its duplicated helpers effectiveParentRole, localArtic, and
quatFromArray into one shared implementation used by both diagnostics. Preserve
the overlay behavior in the root-less resolved-role branch by assigning worldRot
for the current role from its quaternion before placing descendants, so both
callers produce identical results.
| QVariantList MocapController::availableDevices() const | ||
| { | ||
| QVariantList out; | ||
| for (const auto& dev : CameraFrameSource::availableDevices()) { | ||
| QVariantMap m; | ||
| m.insert(QStringLiteral("id"), dev.id); | ||
| m.insert(QStringLiteral("description"), dev.description); | ||
| out.append(m); | ||
| } | ||
| return out; | ||
| return d->cachedDevices; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find every reader of availableDevices and every caller of refreshDevices.
rg -nP -C4 '\b(availableDevices|refreshDevices)\s*\(' --glob '!**/build/**'
rg -nP -C3 'availableDevices|refreshDevices' --glob '*.qml'Repository: fernandotonon/QtMeshEditor
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching MocapController =="
git ls-files | rg 'MocapController\.(h|cpp)$|mocap|Mocap|\.qml$' | sed -n '1,200p'
echo "== all textual occurrences =="
rg -n -C3 '\b(availableDevices|refreshDevices)\b' . --glob '!**/build/**' --glob '!**/.git/**' || true
echo "== MocapController.cpp relevant sections =="
if [ -f src/Mocap/MocapController.cpp ]; then
wc -l src/Mocap/MocapController.cpp
sed -n '300,490p' src/Mocap/MocapController.cpp | cat -n
fi
echo "== MocapController.h relevant sections =="
if [ -f src/Mocap/MocapController.h ]; then
wc -l src/Mocap/MocapController.h
sed -n '1,220p' src/Mocap/MocapController.h | cat -n
fiRepository: fernandotonon/QtMeshEditor
Length of output: 29057
Populate availableDevices before startup reads.
MocapController::availableDevices() returns the empty d->cachedDevices cache, and the constructor does not call refreshDevices(). Build the first device list in the normal available/non-available constructor path, otherwise startup model readers see no devices until refreshDevices() is called.
Also applies to: 459-472
🤖 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/Mocap/MocapController.cpp` around lines 366 - 369, Initialize
d->cachedDevices by calling refreshDevices() during construction in the normal
available/non-available constructor path, before startup model readers can
access availableDevices(). Preserve the existing behavior for unavailable
devices and ensure availableDevices() returns the populated initial cache.
| if (on && d->state != Idle) { | ||
| if (Ogre::Entity* entity = d->entity()) | ||
| d->poseDebugOverlay.attach(Manager::getSingleton()->getSceneMgr(), | ||
| entity->getParentSceneNode()); | ||
| } else { | ||
| d->poseDebugOverlay.detach(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the Manager singleton before dereferencing it.
Manager::getSingleton() is dereferenced without a null check, and getSceneMgr() may also return null. Impl::entity() (lines 327-335) already uses Manager::getSingletonPtr() with a null check. Use the same pattern here.
🛡️ Proposed change
- if (on && d->state != Idle) {
- if (Ogre::Entity* entity = d->entity())
- d->poseDebugOverlay.attach(Manager::getSingleton()->getSceneMgr(),
- entity->getParentSceneNode());
- } else {
+ auto* mgr = Manager::getSingletonPtr();
+ if (on && d->state != Idle && mgr && mgr->getSceneMgr()) {
+ if (Ogre::Entity* entity = d->entity())
+ d->poseDebugOverlay.attach(mgr->getSceneMgr(),
+ entity->getParentSceneNode());
+ } else {
d->poseDebugOverlay.detach();
}Based on the coding guideline "Preserve the singleton conventions for Manager, SelectionSet, and TransformOperator: access them through getSingleton() or getSingletonPtr()".
Also applies to: 890-892
🤖 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/Mocap/MocapController.cpp` around lines 439 - 445, Update the pose debug
overlay attachment logic in MocapController to use Manager::getSingletonPtr()
and verify both the manager and getSceneMgr() result before dereferencing or
attaching. Apply the same guard to the corresponding overlay attachment logic
near the second affected location, while preserving detach behavior when
prerequisites are unavailable.
Source: Coding guidelines
| void fkPoseIkJoints( | ||
| const std::array<std::array<float, 4>, PoseIK::kCanonicalRoles>& quats, | ||
| uint32_t resolvedMask, | ||
| const std::array<std::array<float, 3>, PoseIK::kLandmarkCount>& canonLmPts, | ||
| std::array<Vec3, PoseIK::kCanonicalRoles>& out) | ||
| { | ||
| out.fill({0.f, 0.f, 0.f}); | ||
| const Vec3 hip = mid(canonLmPts[23], canonLmPts[24]); | ||
| out[static_cast<size_t>(PoseIK::Hip)] = hip; | ||
|
|
||
| struct BoneSeg { | ||
| int role; | ||
| int fromLm; | ||
| int toLm; | ||
| }; | ||
| static const BoneSeg segs[] = { | ||
| {PoseIK::Abdomen, 23, 11}, | ||
| {PoseIK::Chest, 11, 12}, | ||
| {PoseIK::Neck, 12, 0}, | ||
| {PoseIK::Head, 0, 8}, | ||
| {PoseIK::RShoulder, 12, 14}, | ||
| {PoseIK::RElbow, 14, 16}, | ||
| {PoseIK::RHand, 16, 16}, | ||
| {PoseIK::LShoulder, 11, 13}, | ||
| {PoseIK::LElbow, 13, 15}, | ||
| {PoseIK::LHand, 15, 15}, | ||
| {PoseIK::RHip, 24, 26}, | ||
| {PoseIK::RKnee, 26, 28}, | ||
| {PoseIK::RFoot, 28, 32}, | ||
| {PoseIK::LHip, 23, 25}, | ||
| {PoseIK::LKnee, 25, 27}, | ||
| {PoseIK::LFoot, 27, 31}, | ||
| }; | ||
|
|
||
| std::array<Vec3, PoseIK::kCanonicalRoles> restOffset{}; | ||
| for (const BoneSeg& s : segs) { | ||
| Vec3 dir = sub(canonLmPts[static_cast<size_t>(s.toLm)], | ||
| canonLmPts[static_cast<size_t>(s.fromLm)]); | ||
| const float d = len(dir); | ||
| if (d < 1e-5f) | ||
| dir = {0.f, 0.12f, 0.f}; | ||
| else | ||
| dir = mul(dir, 1.f / d); | ||
| restOffset[static_cast<size_t>(s.role)] = mul(dir, std::max(d, 0.05f)); | ||
| } | ||
|
|
||
| std::array<Ogre::Quaternion, PoseIK::kCanonicalRoles> worldRot{}; | ||
| worldRot.fill(Ogre::Quaternion::IDENTITY); | ||
|
|
||
| for (int role = 0; role < PoseIK::kCanonicalRoles; ++role) { | ||
| if (!(resolvedMask & (1u << static_cast<unsigned>(role)))) | ||
| continue; | ||
| const int parent = MotionInbetween::canonicalParentOf(role); | ||
| const Ogre::Quaternion local = localArtic(quats, role, resolvedMask); | ||
| if (parent >= 0 && (resolvedMask & (1u << static_cast<unsigned>(parent)))) { | ||
| worldRot[static_cast<size_t>(role)] = | ||
| worldRot[static_cast<size_t>(parent)] * local; | ||
| out[static_cast<size_t>(role)] = | ||
| add(out[static_cast<size_t>(parent)], | ||
| mulVec3(worldRot[static_cast<size_t>(parent)], | ||
| restOffset[static_cast<size_t>(role)])); | ||
| } else if (role == PoseIK::Hip) { | ||
| worldRot[0] = quatFromArray(quats[0]); | ||
| } else { | ||
| worldRot[static_cast<size_t>(role)] = quatFromArray(quats[role]); | ||
| out[static_cast<size_t>(role)] = | ||
| add(hip, restOffset[static_cast<size_t>(role)]); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Share one PoseIK FK helper between the two debug consumers. Both files copy fkPoseIkJoints, effectiveParentRole, localArtic, and quatFromArray. The copies already differ: the overlay sets worldRot[role] for a resolved role whose parent is unresolved, and the log does not. The overlay and the log therefore report different joint positions for the same frame, which defeats the purpose of comparing them.
src/Mocap/MocapPoseDebugOverlay.cpp#L105-L174: move this implementation and the three helpers into a shared internal header (for examplesrc/Mocap/MocapPoseFK.h) next toMocapLiveTypes.h, and call it fromupdate().src/Mocap/MocapBodyDriveDebug.cpp#L69-L145: delete the local copies and call the shared helper, so the log uses the sameworldRothandling as the overlay.
🧰 Tools
🪛 Cppcheck (2.21.0)
[error] 113-113: Address of local auto-variable assigned to a function parameter.
(autoVariables)
📍 Affects 2 files
src/Mocap/MocapPoseDebugOverlay.cpp#L105-L174(this comment)src/Mocap/MocapBodyDriveDebug.cpp#L69-L145
🤖 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/Mocap/MocapPoseDebugOverlay.cpp` around lines 105 - 174, Move
fkPoseIkJoints and the effectiveParentRole, localArtic, and quatFromArray
helpers from src/Mocap/MocapPoseDebugOverlay.cpp lines 105-174 into a shared
internal header near MocapLiveTypes.h, then update the overlay’s update() to
call that shared implementation. Remove the duplicate helpers and FK
implementation from src/Mocap/MocapBodyDriveDebug.cpp lines 69-145 and call the
shared helper there, preserving the overlay’s worldRot handling for resolved
roles with unresolved parents so both debug consumers produce identical joint
positions.
| void MocapPoseDebugOverlay::ensureMaterial() | ||
| { | ||
| if (Ogre::MaterialManager::getSingleton().resourceExists(kMatName)) | ||
| return; | ||
| auto mat = Ogre::MaterialManager::getSingleton().create( | ||
| kMatName, Ogre::ResourceGroupManager::INTERNAL_RESOURCE_GROUP_NAME); | ||
| if (!mat->getNumTechniques()) | ||
| mat->createTechnique(); | ||
| Ogre::Pass* pass = mat->getTechnique(0)->getPass(0); | ||
| pass->setLightingEnabled(false); | ||
| pass->setVertexColourTracking(Ogre::TVC_DIFFUSE); | ||
| pass->setCullingMode(Ogre::CULL_NONE); | ||
| pass->setDepthCheckEnabled(true); | ||
| pass->setDepthWriteEnabled(false); | ||
| pass->setSceneBlending(Ogre::SBT_TRANSPARENT_ALPHA); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Ogre 14 Material createTechnique does the new Technique contain a default Pass or must createPass be called
💡 Result:
In Ogre 14, when you call createTechnique on a Material object, the new Technique is created empty, meaning it does not automatically contain any Pass objects [1][2][3]. Consequently, you must explicitly call createPass on the newly created Technique to add at least one Pass, as a valid Technique requires at least one Pass to perform rendering [4]. While a brand-new Material object might be initialized with a default Technique and Pass depending on how it was created (e.g., via script parsing or default settings), a Technique created programmatically via Material::createTechnique is empty by design [3][5]. You are responsible for populating it with the necessary Passes to define the rendering behavior [2][4].
Citations:
- 1: https://ogrecave.github.io/ogre/api/14/class_ogre_1_1_material.html
- 2: https://ogrecave.github.io/ogre/api/14/class_ogre_1_1_technique.html
- 3: https://github.com/OGRECave/ogre/blob/f722f080/OgreMain/src/OgreMaterial.cpp
- 4: https://ogrecave.github.io/ogre/api/14/_material-_scripts.html
- 5: https://forums.ogre3d.org/viewtopic.php?t=49509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant source location and any nearby use of ensureMaterial.
if [ -f src/Mocap/MocapPoseDebugOverlay.cpp ]; then
echo "== MocapPoseDebugOverlay.cpp outline =="
ast-grep outline src/Mocap/MocapPoseDebugOverlay.cpp --view compact || true
echo
echo "== Relevant source =="
sed -n '150,230p' src/Mocap/MocapPoseDebugOverlay.cpp | nl -ba -v150
echo
echo "== ensureMaterial usages =="
rg -n "ensureMaterial\(|MocapPoseDebugOverlay" src/Mocap/MocapPoseDebugOverlay.cpp src || true
else
echo "src/Mocap/MocapPoseDebugOverlay.cpp not found"
fi
echo
echo "== Version/config mentions of Ogre =="
rg -n "Ogre 14|OGRE|Ogre" CMakeLists.txt . cmake include src/Mocap || trueRepository: fernandotonon/QtMeshEditor
Length of output: 419
🏁 Script executed:
#!/bin/bash
set -u
echo "== Relevant source =="
if [ -f src/Mocap/MocapPoseDebugOverlay.cpp ]; then
sed -n '150,230p' src/Mocap/MocapPoseDebugOverlay.cpp
fi
echo
echo "== ensureMaterial usages =="
if [ -f src/Mocap/MocapPoseDebugOverlay.cpp ]; then
rg -n "ensureMaterial\(|MocapPoseDebugOverlay" src/Mocap/MocapPoseDebugOverlay.cpp src || true
fi
echo
echo "== Version/config mentions of Ogre =="
rg -n "Ogre 14|OGRE|Ogre" CMakeLists.txt cmake include src/Mocap 2>/dev/null || trueRepository: fernandotonon/QtMeshEditor
Length of output: 28084
Create the pass before configuring it.
MaterialManager::create produces an empty material; createTechnique() creates an empty technique in Ogre. Calling getPass(0) on a fresh technique indexes an empty pass list, so the first rebuild can crash. Ensure the material has at least one pass before configuring it.
🤖 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/Mocap/MocapPoseDebugOverlay.cpp` around lines 190 - 205, Update
MocapPoseDebugOverlay::ensureMaterial to ensure the first technique contains at
least one pass before retrieving and configuring pass 0. Preserve the existing
material and technique creation flow, adding pass creation when needed before
the getPass(0) call.
| TEST(MocapFrameFromVideoFrame, ConvertsQVideoFrameFromQImage) | ||
| { | ||
| QImage rgb(8, 8, QImage::Format_RGB888); | ||
| rgb.fill(Qt::blue); | ||
| const QVideoFrame vf(rgb); | ||
| ASSERT_TRUE(vf.isValid()); | ||
| const QImage out = mocapFrameFromVideoFrame(vf); | ||
| EXPECT_FALSE(out.isNull()); | ||
| EXPECT_EQ(out.format(), QImage::Format_RGB888); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files 'src/Mocap/VideoFrameSource.cpp' 'src/Mocap/VideoFrameSource_test.cpp' | sed 's#^\./##'
echo
echo "Source outline:"
ast-grep outline src/Mocap/VideoFrameSource.cpp --view compact || true
echo
echo "Test relevant section:"
sed -n '160,230p' src/Mocap/VideoFrameSource_test.cpp | cat -n
echo
echo "Source relevant function:"
rg -n "mocapFrameFromVideoFrame|Format_Jpeg|map|QVideoFrameFormat|QImage" src/Mocap/VideoFrameSource.cppRepository: fernandotonon/QtMeshEditor
Length of output: 3446
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Changed files/stat:"
git diff --stat || true
echo
echo "Source VideoFrame function with line numbers:"
sed -n '1,120p' src/Mocap/VideoFrameSource.cpp | cat -n
echo
echo "Test imports/macros:"
sed -n '1,80p' src/Mocap/VideoFrameSource_test.cpp | cat -nRepository: fernandotonon/QtMeshEditor
Length of output: 7148
Cover the JPEG fallback path.
QVideoFrame vf(rgb) exercises RGB frame conversion but not the mapped Format_Jpeg branch, where QVideoFrame bytes are decoded with QImage::fromData(..., "JPEG"). Add a mapped JPEG QVideoFrame test, or extract the JPEG decode into a testable helper. Assert a known output pixel and QImage::Format_RGB888.
🤖 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/Mocap/VideoFrameSource_test.cpp` around lines 201 - 210, Extend the
MocapFrameFromVideoFrame tests to cover the mapped Format_Jpeg branch by
constructing a QVideoFrame containing JPEG-encoded image data and passing it
through mocapFrameFromVideoFrame. Assert that decoding produces the expected
known pixel value and QImage::Format_RGB888, while preserving the existing RGB
conversion test.
Source: Coding guidelines
…ctors Centralize ONNX session options (CPU/CoreML/CUDA/DirectML) behind a QML singleton with AI Settings UI. Optional QTMESH_ONNX_GPU fetch on Linux x64, provider .so copy rules, CI stays on CPU package, and mocap predictors use the shared path so the app links cleanly. Co-authored-by: Cursor <cursoragent@cursor.com>
FaceCap head rotation needs yaw correction on selfie/webcam feeds while body retargeting uses landmark directions without L/R swapping. Co-authored-by: Cursor <cursoragent@cursor.com>
Use vendor-neutral wording in live mocap, BodyRetargeter, docs, and debug UI while keeping factual mixamorig examples where they describe bone naming. Co-authored-by: Cursor <cursoragent@cursor.com>
Wire OnnxRuntimeSettings and mocap debug sources into qtmesh_test_common, propagate parent world transforms for unmapped bones, bake body recordings via landmark retarget, and clean up skinning listener/head manual-control edge cases. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (4)
src/Mocap/MocapRecorder.cpp (2)
425-426: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the torso mask constant instead of duplicating it.
kTorsoResolvedMaskhere repeatsImpl::kTorsoResolvedMaskinsrc/Mocap/MocapController.cpp(lines 287-288). Both encode the same hip/abdomen/chest role contract. Define it once next to thePoseIKrole enum and include it in both files, so a role reorder cannot desynchronize the calibration gate from the preview gate.🤖 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/Mocap/MocapRecorder.cpp` around lines 425 - 426, Move kTorsoResolvedMask to the shared declaration area beside the PoseIK role enum, then include that shared definition from both MocapRecorder.cpp and MocapController.cpp. Remove the local constants, including Impl::kTorsoResolvedMask, while preserving the existing hip/abdomen/chest mask value and all current calibration and preview checks.
428-434: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFixed 22-element role arrays are not tied to
PoseIK::kCanonicalRoles. Both sites declarestd::array<std::array<float, 4>, 22>and then fill it with a loop bounded byPoseIK::kCanonicalRoles. If the role count ever grows past 22, both loops write out of bounds.
src/Mocap/MocapRecorder.cpp#L428-L434: size theframeQuatsdestination fromPoseIK::kCanonicalRoles, or add astatic_assert(PoseIK::kCanonicalRoles <= 22, ...)before the loop.src/Mocap/MocapController.cpp#L1050-L1052: apply the same size source or assertion tocanonQuats.🤖 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/Mocap/MocapRecorder.cpp` around lines 428 - 434, The hardcoded array size of 22 is not tied to the loop bound PoseIK::kCanonicalRoles, creating an out-of-bounds risk if the role count grows. At src/Mocap/MocapRecorder.cpp lines 428-434 in the frameQuats function, replace the fixed array size of 22 with PoseIK::kCanonicalRoles or add a static_assert(PoseIK::kCanonicalRoles <= 22, ...) before the loop. At src/Mocap/MocapController.cpp lines 1050-1052 for the canonQuats array, apply the same fix to align the hardcoded size with PoseIK::kCanonicalRoles.src/TextureUpscaler.cpp (1)
152-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicated intra-op thread setting.
configureSessionOptions(so)already appliesSetIntraOpNumThreads(hc > 1 ? hc - 1 : 1)becauseSessionConfig::reserveUiThreadCoredefaults to true. Lines 153-155 repeat the same computation. Keeping both makes the thread policy live in two places.♻️ Proposed change
OnnxRuntimeSettings::configureSessionOptions(so); - const unsigned hw = std::thread::hardware_concurrency(); - const int threads = (hw > 1) ? static_cast<int>(hw - 1) : 1; - so.SetIntraOpNumThreads(threads);🤖 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/TextureUpscaler.cpp` around lines 152 - 155, Remove the redundant hardware_concurrency calculation and SetIntraOpNumThreads call following OnnxRuntimeSettings::configureSessionOptions(so) in TextureUpscaler initialization, leaving thread-count policy centralized in configureSessionOptions.src/ImageTo3D/TripoSGPredictor.cpp (1)
277-284: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSkip building the GPU session options when
gpuAvailablecannot become true.
gpuAvailableis assigned only inside#ifdef __APPLE__, so on Windows and Linux it stays false andopen()never selectsgpu. On those platformsconfigureSessionOptions(gpu, gpuCfg)still appends an execution provider (DirectML or CUDA) that is then discarded. Build thegpuoptions only on Apple, or extendgpuAvailableto the other platforms if DirectML/CUDA use is intended for the point decoder.♻️ Proposed change
Ort::SessionOptions gpu; - OnnxRuntimeSettings::SessionConfig gpuCfg; - gpuCfg.coreMlStyle = OnnxRuntimeSettings::CoreMlStyle::MlProgram; - OnnxRuntimeSettings::configureSessionOptions(gpu, gpuCfg); bool gpuAvailable = false; `#ifdef` __APPLE__ + OnnxRuntimeSettings::SessionConfig gpuCfg; + gpuCfg.coreMlStyle = OnnxRuntimeSettings::CoreMlStyle::MlProgram; + OnnxRuntimeSettings::configureSessionOptions(gpu, gpuCfg); gpuAvailable = OnnxRuntimeSettings::instance()->preferGpu(); +#else + OnnxRuntimeSettings::SessionConfig gpuCfg; + gpuCfg.appendGpu = false; + OnnxRuntimeSettings::configureSessionOptions(gpu, gpuCfg); `#endif`🤖 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/ImageTo3D/TripoSGPredictor.cpp` around lines 277 - 284, Update the GPU session-options setup around gpuAvailable so configureSessionOptions is invoked only when the platform can actually set gpuAvailable true, preserving Apple’s Core ML path and avoiding unused DirectML/CUDA provider configuration on Windows and Linux; alternatively, explicitly enable and select those providers for the point decoder on those platforms if that support is intended.
🤖 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 `@cmake/OnnxRuntime.cmake`:
- Around line 11-16: When selecting a CPU-only ONNX runtime archive for
unsupported GPU platforms like Linux ARM, explicitly set QTMESH_ONNX_GPU to OFF
before the archive selection is cached or exported. This ensures the flag does
not remain from a previous configuration state and prevents incorrect GPU
definitions like QTMESH_ONNX_GPU_BUILD from being set for CPU-only builds.
In `@qml/AISettingsDialog.qml`:
- Line 714: Update the note text in AISettingsDialog so the ONNX GPU preference
is described as applying only to newly created ONNX sessions, not immediately to
existing sessions; preserve the existing LLM settings wording.
In `@qml/CollapsibleSection.qml`:
- Around line 73-83: Guard both deferred callbacks in CollapsibleSection’s
onExpandedChanged and Component.onCompleted handlers by rechecking root.expanded
inside the Qt.callLater closure before setting contentLoader.loadActive = true;
keep immediate deactivation unchanged.
In `@scripts/install-onnx-gpu-deps.sh`:
- Around line 19-31: Update the dependency presence check and the post-install
validation in the script around the existing cuDNN checks to require both
$DEST/nvidia/cudnn/lib/libcudnn.so.9 and
$DEST/nvidia/cublas/lib/libcublas.so.12. Only skip pip installation when both
files exist, and fail after installation if either library is missing.
In `@src/CLIPipeline.cpp`:
- Around line 1559-1566: Move redirectStdout() in the CLI initialization flow
before MocapCameraHints::ensureMultimediaBackendSafe(), QApplication
construction, and OnnxRuntimeSettings::prepareRuntimeEnvironment(). Preserve the
existing saved-STDOUT and custom message-handler setup while ensuring these
initializers cannot write directly to stdout before JSON or informational output
is routed safely.
In `@src/Mocap/MocapController.cpp`:
- Around line 1208-1210: Replace the hardcoded 30 fps in the
RecordBodyClipCommand construction within MocapController with a rate derived
from the recorded take, using the sample timestamps and frame count (or the
existing d->liveFps measurement). Pass the computed fps to preserve the clip’s
actual timing across camera rates.
- Around line 1030-1032: The head-yaw correction via
MocapPoseFix::invertCameraYawDelta() is applied unconditionally in the
beginPreviewWithLiveSource() method, but this method is shared between
CameraFrameSource (which is mirrored) and FileFrameSource (which is not
mirrored). Add a conditional check to apply the invertCameraYawDelta correction
only when the live source is the mirrored webcam path, and skip it for the file
source path.
In `@src/Mocap/MocapRecorder.cpp`:
- Around line 499-523: The keyframe timing in the animation track creation uses
a simple frame-index calculation (dt * f) that doesn't account for gaps from
invalid frames, causing motion to play faster during lost-tracking spans. Add a
tracked sample time field to BodyLiveFrame during capture, populate it with the
actual time when each frame is recorded, and update the createNodeKeyFrame call
to use frame's tracked time instead of the dt * f calculation.
- Around line 458-497: In bakeRetargeterClip, defer skel->removeAnimation(clip)
until after the neutral calibration logic has succeeded, including the
no-confident-frames check. Preserve the existing clip when calibration returns
the "no confident body frames to calibrate" error, and remove it only
immediately before writing the successfully calibrated replacement.
In `@src/OnnxRuntimeSettings_test.cpp`:
- Around line 8-25: Update the PreferGpuPersists test to clear
QTMESH_ONNX_PREFER_GPU for the test’s duration before reading preferGpu(), using
the existing Qt environment-handling facilities and restoring the prior
environment value afterward so the test remains isolated.
In `@src/OnnxRuntimeSettings.cpp`:
- Around line 288-291: Initialize the OnnxRuntimeSettings singleton on the
main/UI thread before any worker-thread inference can begin, rather than first
constructing it through tryAppendGpuExecutionProvider or
configureSessionOptions. Add the early initialization at application startup and
establish the required settingsChanged() UI connections before background work
starts, preserving worker-thread calls through the existing
OnnxRuntimeSettings::instance() path.
- Around line 62-76: Replace the runtime LD_LIBRARY_PATH approach in
prependLdLibraryPath with a loader-visible dependency strategy: configure
build-time RPATH/RUNPATH for the CUDA dependency directory, or preload the
required CUDA libraries by absolute path with QLibrary/dlopen(RTLD_GLOBAL)
before Ort::Session creation. Ensure cudaProviderLibraryLoads and subsequent
libonnxruntime/provider loading can resolve cudnn/cublas dependencies without
relying on qputenv.
- Around line 103-112: Replace the non-atomic static bool done guard in
prepareRuntimeEnvironment() with a thread-safe call-once mechanism. Add `#include`
<mutex> at the top of the file, then declare a static std::once_flag inside the
function and wrap the prependLdLibraryPath(cudnnSearchPaths()) call with
std::call_once to ensure it executes exactly once regardless of concurrent
access from multiple threads.
- Around line 305-312: Replace the generic AppendExecutionProvider("DML", {})
call in the Windows non-MinGW32 conditional block with the DirectML-specific
low-level API SessionOptionsAppendExecutionProvider_DML(...) and configure the
required DirectML session options according to the V1.20.1 build configuration
in cmake/OnnxRuntime.cmake. Keep the existing try-catch around this call since
it can throw and should fall back to CPU when the DirectML provider is not
registered.
---
Nitpick comments:
In `@src/ImageTo3D/TripoSGPredictor.cpp`:
- Around line 277-284: Update the GPU session-options setup around gpuAvailable
so configureSessionOptions is invoked only when the platform can actually set
gpuAvailable true, preserving Apple’s Core ML path and avoiding unused
DirectML/CUDA provider configuration on Windows and Linux; alternatively,
explicitly enable and select those providers for the point decoder on those
platforms if that support is intended.
In `@src/Mocap/MocapRecorder.cpp`:
- Around line 425-426: Move kTorsoResolvedMask to the shared declaration area
beside the PoseIK role enum, then include that shared definition from both
MocapRecorder.cpp and MocapController.cpp. Remove the local constants, including
Impl::kTorsoResolvedMask, while preserving the existing hip/abdomen/chest mask
value and all current calibration and preview checks.
- Around line 428-434: The hardcoded array size of 22 is not tied to the loop
bound PoseIK::kCanonicalRoles, creating an out-of-bounds risk if the role count
grows. At src/Mocap/MocapRecorder.cpp lines 428-434 in the frameQuats function,
replace the fixed array size of 22 with PoseIK::kCanonicalRoles or add a
static_assert(PoseIK::kCanonicalRoles <= 22, ...) before the loop. At
src/Mocap/MocapController.cpp lines 1050-1052 for the canonQuats array, apply
the same fix to align the hardcoded size with PoseIK::kCanonicalRoles.
In `@src/TextureUpscaler.cpp`:
- Around line 152-155: Remove the redundant hardware_concurrency calculation and
SetIntraOpNumThreads call following
OnnxRuntimeSettings::configureSessionOptions(so) in TextureUpscaler
initialization, leaving thread-count policy centralized in
configureSessionOptions.
🪄 Autofix
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 Plus
Run ID: 92102ff2-799b-4f51-bf5d-332c100c73ba
📒 Files selected for processing (46)
.github/workflows/deploy.ymlCMakeLists.txtcmake/OnnxRuntime.cmakedocs/MOCAP.mdqml/AISettingsDialog.qmlqml/CollapsibleSection.qmlqml/PropertiesPanel.qmlqml/ThemedComboBox.qmlscripts/install-onnx-gpu-deps.shsrc/AnimationMerger.cppsrc/AnimationMerger_test.cppsrc/CLIPipeline.cppsrc/CMakeLists.txtsrc/FaceRig/FaceLandmarkDetector.cppsrc/ImageTo3D/BackgroundRemover.cppsrc/ImageTo3D/MeshGenPredictor.cppsrc/ImageTo3D/TripoSGPredictor.cppsrc/LLMSettingsWidget.cppsrc/LLMSettingsWidget.hsrc/LLMSettingsWidget_test.cppsrc/MeshSegmenter.cppsrc/Mocap/FaceCapPredictor.cppsrc/Mocap/MocapBodyDriveDebug.cppsrc/Mocap/MocapController.cppsrc/Mocap/MocapPoseDebugOverlay.cppsrc/Mocap/MocapPoseFix.hsrc/Mocap/MocapRecorder.cppsrc/Mocap/MocapRecorder.hsrc/Mocap/PoseCapPredictor.cppsrc/Mocap/PoseIKSolver.cppsrc/MotionGenerator.cppsrc/MotionInbetween.cppsrc/OnnxRuntimeSettings.cppsrc/OnnxRuntimeSettings.hsrc/OnnxRuntimeSettings_test.cppsrc/PbrMapSynth.cppsrc/SkinTokensPredictor.cppsrc/TextureUpscaler.cppsrc/UniRigPredictor.cppsrc/UniRigPredictor.hsrc/UniRigPredictor_test.cppsrc/commands/RecordMocapClipCommand.cppsrc/commands/RecordMocapClipCommand.hsrc/main.cppsrc/mainwindow.cpptests/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (8)
- src/Mocap/MocapPoseDebugOverlay.cpp
- src/AnimationMerger_test.cpp
- src/AnimationMerger.cpp
- src/Mocap/MocapBodyDriveDebug.cpp
- src/Mocap/PoseIKSolver.cpp
- docs/MOCAP.md
- src/Mocap/MocapPoseFix.h
- qml/PropertiesPanel.qml
| # | ||
| # Linux x64 GPU: pass -DQTMESH_ONNX_GPU=ON (auto-defaults ON when nvidia-smi is | ||
| # found outside CI) to fetch onnxruntime-linux-x64-gpu-*.tgz. The CUDA provider | ||
| # .so must ship next to the binary AND cuDNN 9 + CUDA 12 must be on the system. | ||
| # Windows MinGW is intentionally NOT wired here — the official Windows archive is | ||
| # MSVC-built and won't link under MinGW; that path degrades gracefully (the | ||
| # feature reports "rebuild with -DENABLE_ONNX"). See the #404 follow-up. | ||
| # MSVC-built and won't link under MinGW. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 \
'QTMESH_ONNX_GPU|_qtmesh_onnx_gpu_default|CMAKE_SYSTEM_PROCESSOR|_ort_archive|QTMESH_ONNX_GPU_BUILD' \
cmake/OnnxRuntime.cmake CMakeLists.txtRepository: fernandotonon/QtMeshEditor
Length of output: 6882
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf =='cmake/OnnxRuntime.cmake relevant sections=='\n
sed -n '20,70p' cmake/OnnxRuntime.cmake
printf '\n==cmake/OnnxRuntime.cmake cache/status sections=='\n
sed -n '95,125p' cmake/OnnxRuntime.cmake
printf '\n==CMakeLists.txt ONNX consume section=='\n
sed -n '298,314p' CMakeLists.txt
printf '\n==Behavioral probe for CMake-like conditional/option/processor branches=='\n
python3 - <<'PY'
import os
cases = [
("Linux-aarch64-unspecified", "Linux", "aarch64", False),
("Linux-arm/unspecified", "Linux", "arm", True),
("Linux-arm64-nvidia", "Linux", "arm64", True),
("macOS-unspecified", "Darwin", "arm64", True),
("macOS-unspecified-off", "Darwin", "x86_64", False),
("Windows-unspecified", "Windows", "AMD64", True),
("Windows-GPU", "Windows", "AMD64", True),
]
for name, system, processor, nvidia_smiship in cases:
qtmesh = True # user/default-on
if nvidia_smiship:
qtmesh = True
if processor == "":
qtmesh = False
if nvidia_smiship and system != "Linux":
qtmesh = False
if qtmesh:
selected = "GPU"
used = "CUDA"
else:
selected = "CPU"
used = "CPU"
status = "GPU" if qtmesh else "CPU"
build_def = "QTMESH_ONNX_GPU_BUILD" if qtmesh else ""
print(f"{name}: QTMESH_ONNX_GPU={qtmesh!s:>5}, selected={selected:>3}, status_message={status:>3}, add_definitions={build_def}")
PYRepository: fernandotonon/QtMeshEditor
Length of output: 5539
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '==cmake/OnnxRuntime.cmake relevant sections=='\n
sed -n '20,70p' cmake/OnnxRuntime.cmake
printf '\n==cmake/OnnxRuntime.cmake cache/status sections=='\n
sed -n '95,125p' cmake/OnnxRuntime.cmake
printf '\n==CMakeLists.txt ONNX consume section=='\n
sed -n '298,314p' CMakeLists.txt
printf '\n==Behavioral probe for supported-/archive selection branch combinations=='\n
python3 - <<'PY'
cases = [
{"name": "Linux aarch64 nvidia-smi", "system": "Linux", "os_family": "UnixApple", "processor": "aarch64", "def_option_if_nvidia": True},
{"name": "Linux ARM nvidia-smi", "system": "Linux", "os_family": "UnixApple", "processor": "arm", "def_option_if_nvidia": True},
{"name": "macOS option forced ON", "system": "Darwin", "os_family": "UnixApple", "processor": "arm64", "def_option_if_nvidia": False},
{"name": "Windows option forced ON", "system": "Windows", "os_family": "Win", "processor": "AMD64", "def_option_if_nvidia": False},
]
for case in cases:
qtmesh_onnx_gpu = case["def_option_if_nvidia"]
if case["system"] == "Linux" and not case["os_family"] == "UnixApple" and case["os_family"] == "UnixApple" and case["processor"] in ("aarch64", "arm", "arm64"):
archive = "onnxruntime-linux-aarch64-..tgz"
elif case["system"] == "Linux" and qtmesh_onnx_gpu:
archive = "onnxruntime-linux-x64-gpu-..tgz"
elif case["system"] == "Linux":
archive = "onnxruntime-linux-x64-..tgz"
elif case["system"] == "Darwin":
archive = "onnxruntime-osx-universal2-..tgz"
elif case["system"] == "Windows" and qtmesh_onnx_gpu:
archive = "Windows warns OFF, CPU archive"
else:
archive = "CPU archive"
status = "GPU" if qtmesh_onnx_gpu else "CPU"
print(f"{case['name']}: QTMESH_ONNX_GPU={qtmesh_onnx_gpu!s:>5}, archive={archive!r}, status_line={status}")
PYRepository: fernandotonon/QtMeshEditor
Length of output: 5198
Clear the GPU flag when the selected archive is CPU-only.
Linux ARM can pick onnxruntime-linux-aarch64-*.tgz while QTMESH_ONNX_GPU=ON remains cached, reports GPU, and defines QTMESH_ONNX_GPU_BUILD. Force QTMESH_ONNX_GPU OFF before exporting/caching the selected CPU-only archive for unsupported platforms.
🤖 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 `@cmake/OnnxRuntime.cmake` around lines 11 - 16, When selecting a CPU-only ONNX
runtime archive for unsupported GPU platforms like Linux ARM, explicitly set
QTMESH_ONNX_GPU to OFF before the archive selection is cached or exported. This
ensures the flag does not remain from a previous configuration state and
prevents incorrect GPU definitions like QTMESH_ONNX_GPU_BUILD from being set for
CPU-only builds.
| Text { | ||
| Layout.fillWidth: true | ||
| text: "Note: Settings changes will take effect when loading a new model." | ||
| text: "Note: LLM settings take effect when loading a new model. ONNX GPU preference applies immediately." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the ONNX GPU preference note.
Line 714 states that the preference applies immediately. The preference is read when OnnxRuntimeSettings::configureSessionOptions() creates a new session. Existing ONNX sessions keep their current execution provider. State that the change applies to newly created ONNX sessions.
🤖 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 `@qml/AISettingsDialog.qml` at line 714, Update the note text in
AISettingsDialog so the ONNX GPU preference is described as applying only to
newly created ONNX sessions, not immediately to existing sessions; preserve the
existing LLM settings wording.
| onExpandedChanged: { | ||
| if (root.expanded) | ||
| Qt.callLater(function() { contentLoader.loadActive = true }) | ||
| else | ||
| contentLoader.loadActive = false | ||
| } | ||
|
|
||
| Component.onCompleted: { | ||
| if (root.expanded) | ||
| Qt.callLater(function() { contentLoader.loadActive = true }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Guard the deferred activation against a state change during the pending callback.
Qt.callLater(function() {...}) queues a new closure on each call. It does not dedupe against an earlier pending call from a different closure instance.
If root.expanded toggles from true to false before the deferred callback runs, the callback still sets contentLoader.loadActive = true unconditionally. The section stays visually collapsed (visible is bound to root.expanded), but the Loader instantiates its content anyway and fires onLoaded → contentReady(). Downstream, qml/PropertiesPanel.qml connects onContentReady to call MocapController.refreshDevices(), so this can trigger a device refresh while the section is collapsed.
Recheck root.expanded inside the deferred callback before activating the Loader.
🐛 Proposed fix to recheck expanded state in the deferred callback
onExpandedChanged: {
if (root.expanded)
- Qt.callLater(function() { contentLoader.loadActive = true })
+ Qt.callLater(function() { if (root.expanded) contentLoader.loadActive = true })
else
contentLoader.loadActive = false
}🤖 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 `@qml/CollapsibleSection.qml` around lines 73 - 83, Guard both deferred
callbacks in CollapsibleSection’s onExpandedChanged and Component.onCompleted
handlers by rechecking root.expanded inside the Qt.callLater closure before
setting contentLoader.loadActive = true; keep immediate deactivation unchanged.
| if [[ -f "$DEST/nvidia/cudnn/lib/libcudnn.so.9" ]]; then | ||
| echo "cuDNN already present at $DEST/nvidia/cudnn/lib" | ||
| else | ||
| echo "Installing nvidia-cudnn-cu12 + nvidia-cublas-cu12 into $DEST ..." | ||
| python3 -m pip install \ | ||
| nvidia-cudnn-cu12 nvidia-cublas-cu12 \ | ||
| -t "$DEST" --no-cache-dir --upgrade | ||
| fi | ||
|
|
||
| if [[ ! -f "$DEST/nvidia/cudnn/lib/libcudnn.so.9" ]]; then | ||
| echo "cuDNN install failed." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
deps="${1:-.cache/cuda-deps}"
test -f "$deps/nvidia/cudnn/lib/libcudnn.so.9"
test -f "$deps/nvidia/cublas/lib/libcublas.so.12"Repository: fernandotonon/QtMeshEditor
Length of output: 164
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate and inspect the installer around the cited lines.
if [ -f scripts/install-onnx-gpu-deps.sh ]; then
echo "== file exists =="
wc -l scripts/install-onnx-gpu-deps.sh
echo "== cited lines =="
sed -n '1,80p' scripts/install-onnx-gpu-deps.sh | nl -ba
else
echo "scripts/install-onnx-gpu-deps.sh not found"
fd -a 'install-onnx-gpu-deps\.sh|onnx.*deps|gpu.*deps' .
fi
echo "== package paths / references =="
rg -n "nvidia-(cudnn|cublas)|libcudnn|libcublas|conda|pip install -t|DEST|cache" scripts -S 2>/dev/null || trueRepository: fernandotonon/QtMeshEditor
Length of output: 276
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== file exists =="
wc -l scripts/install-onnx-gpu-deps.sh
echo "== cited lines with line numbers using awk =="
awk '{printf "%4d %s\n", NR, $0}' scripts/install-onnx-gpu-deps.sh | sed -n '1,80p'
echo "== package paths / references =="
rg -n "nvidia-(cudnn|cublas)|libcudnn|libcublas|conda|pip install -t|DEST|cache" scripts -S || trueRepository: fernandotonon/QtMeshEditor
Length of output: 11675
🌐 Web query:
PyPI nvidia-cudnn-cu12 package libcudnn.so.9 path nvidia-cublas-cu12 libcublas.so.12 path
💡 Result:
The PyPI packages nvidia-cudnn-cu12 and nvidia-cublas-cu12 install their respective shared library files within your Python environment's site-packages directory [1][2]. Because these libraries are not installed in standard system paths (like /usr/lib), they are not automatically discoverable by all applications, which frequently causes "cannot open shared object file" errors [1][2]. Path structure in site-packages For a standard Python virtual environment (e.g.,.venv), the libraries are located in subdirectories under the nvidia package namespace [2]: libcudnn.so.9:.venv/lib/pythonX.Y/site-packages/nvidia/cudnn/lib/libcudnn.so.9 [3][2] libcublas.so.12:.venv/lib/pythonX.Y/site-packages/nvidia/cublas/lib/libcublas.so.12 [2] (Replace pythonX.Y with your actual Python version, such as python3.12). How to resolve library discovery issues If an application cannot find these files, you can resolve the issue using one of the following methods: 1. Update LD_LIBRARY_PATH (Linux) You can point your environment to these paths so that the dynamic linker can locate the libraries [2]. You can automate this by adding the directories to your LD_LIBRARY_PATH [2]: export LD_LIBRARY_PATH=$(find /path/to/.venv -name ".so" | grep nvidia | xargs dirname | sort -u | paste -d ":" -s -):$LD_LIBRARY_PATH 2. Programmatic loading If you are writing a Python application, you can use the nvidia modules to programmatically locate and load the libraries using ctypes [4][5]: import os import ctypes import nvidia.cudnn import nvidia.cublas # Path to the library directory cudnn_lib = os.path.join(nvidia.cudnn.path, "lib", "libcudnn.so.9") cublas_lib = os.path.join(nvidia.cublas.path, "lib", "libcublas.so.12") # Load them ctypes.CDLL(cudnn_lib, mode=ctypes.RTLD_GLOBAL) ctypes.CDLL(cublas_lib, mode=ctypes.RTLD_GLOBAL) These packages are designed to be used as dependencies by frameworks like PyTorch, which typically handle this library loading automatically [1][4]. If you are encountering missing library errors while using these packages directly, it is usually because the application expects the libraries to be in a system-wide path rather than the Python-specific site-packages structure [1][2].
Citations:
- 1: https://ai-infrastructure.net/cuda-libraries/
- 2: https://dev.to/metal3d/how-to-resolve-the-dlopen-problem-with-nvidia-and-pytorch-or-tensorflow-inside-a-virtual-env-181e
- 3: ImportError: libcudnn.so.9: cannot open shared object file: No such file or directory pytorch/pytorch#154299
- 4: Support PyPI-installed
nvidia-cuda-runtime-cu12andnvidia-cublas-cu12abetlen/llama-cpp-python#1460 - 5: https://forums.developer.nvidia.com/t/installing-cuda-in-python-virtual-environment-environment-variables-and-other/286975
Validate cuBLAS before accepting cached CUDA deps. Line 19 only checks libcudnn.so.9, so a partial cache can skip pip install and the final check can still succeed while nvidia-cublas-cu12 is missing. Check libcudnn.so.9 and libcublas.so.12 in both conditions.
🤖 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 `@scripts/install-onnx-gpu-deps.sh` around lines 19 - 31, Update the dependency
presence check and the post-install validation in the script around the existing
cuDNN checks to require both $DEST/nvidia/cudnn/lib/libcudnn.so.9 and
$DEST/nvidia/cublas/lib/libcublas.so.12. Only skip pip installation when both
files exist, and fail after installation if either library is missing.
| #ifdef ENABLE_MOCAP | ||
| MocapCameraHints::ensureMultimediaBackendSafe(); | ||
| #endif | ||
| QApplication a(argc, argv); | ||
| QCoreApplication::setOrganizationName("QtMeshEditor"); | ||
| QCoreApplication::setApplicationName("QtMeshEditor"); | ||
| QCoreApplication::setApplicationVersion(QTMESHEDITOR_VERSION); | ||
| OnnxRuntimeSettings::prepareRuntimeEnvironment(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 \
'redirectStdout|ensureMultimediaBackendSafe|prepareRuntimeEnvironment|stdout|printf|qInfo|qDebug' \
src/CLIPipeline.cpp \
src/CLIPipeline.h \
src/Mocap/MocapCameraHints.cpp \
src/Mocap/MocapCameraHints.h \
src/OnnxRuntimeSettings.cpp \
src/OnnxRuntimeSettings.h || trueRepository: fernandotonon/QtMeshEditor
Length of output: 28509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find files in the repository that use Qt Multimedia and QCoreApplication classes/classes likely involved in initialization.
python3 - <<'PY'
import os, subprocess, re
files = subprocess.check_output(["git","ls-files"], text=True, encoding="utf-8").splitlines()
candidates = []
for f in files:
if not f.endswith((".h",".hpp",".cpp",".cc",".cxx",".c")):
continue
try:
text = open(f, "r", encoding="u8", errors="ignore").read()
except Exception:
continue
hits = []
for pat in [
"QMediaPlayer", "QCamera", "QAudio", "QVideoFrame", "QMediaDevices", "MocapCameraHints",
"QCoreApplication", "QGuiApplication", "QApplication", "qInfo", "qDebug", "qWarning", "qCritical"
]:
if pat in text:
hits.append(pat)
if hits:
candidates.append((f, hits))
print("multimedia_core_candidates:")
for f,hits in sorted(candidates):
print(f"{f}: {', '.join(hits)}")
PY
echo
echo "All Qt Multimedia-related occurrences (focused):"
rg -n 'QMediaPlayer|QCamera|QAudio|QVideoFrame|QMediaDevices|QMediaCapture|QMediaPlaylist|QUrl|QUrlRequester|QMediaSession' \
src \
--glob '*.{h,hpp,cpp,cc,cxx,c}' \
--glob '!**/.git/**' || true
echo
echo "Initialize command region:"
sed -n '1520,1585p' src/CLIPipeline.cpp
echo
echo "redirecting and handlers:"
sed -n '570,595p' src/CLIPipeline.cppRepository: fernandotonon/QtMeshEditor
Length of output: 34112
🌐 Web query:
Qt QMediaDevices::videoInputs stderr stdout usage PlatformMediaIntegration QPlatformMediaIntegration instance environment variable backend log stream
💡 Result:
In Qt Multimedia, QMediaDevices::videoInputs retrieves a list of connected cameras by interacting with the QPlatformMediaIntegration instance, which acts as the abstraction layer for media backends [1]. Environment Variable and Backend Selection The media backend used by Qt is determined by the QPlatformMediaIntegration class [2][3]. It selects the backend in the following priority order: 1. The value of the QT_MEDIA_BACKEND environment variable [2][3]. 2. The default backend defined for the specific platform [2]. You can identify available backends and debug the loading process by enabling logging categories. Qt Multimedia uses the category "qt.multimedia.plugin" to report backend loading status [3][4]. Logging and Debugging To view logs related to backend initialization, media devices, or FFmpeg integration, you can use the QT_LOGGING_RULES environment variable [5][6]. Common logging configurations: - Enable all multimedia logs: QT_LOGGING_RULES=".multimedia.=true" [5][6]. - Enable backend plugin logs: QT_LOGGING_RULES="qt.multimedia.plugin=true" [3][5]. - For FFmpeg-based backends, you can further debug using QT_FFMPEG_DEBUG=1 [5][6]. These logs are typically directed to stderr by default, consistent with Qt's standard logging framework. If QMediaDevices::videoInputs returns an empty list unexpectedly, these logs can help verify whether the correct backend is being loaded and whether the system is successfully detecting hardware devices [7][5]. Summary of Key Mechanisms - QMediaDevices::videoInputs relies on QPlatformMediaIntegration::instance->videoDevices [1]. - QT_MEDIA_BACKEND allows you to explicitly force a specific backend plugin if the auto-detection fails [2][3]. - Logging is controlled via QT_LOGGING_RULES, allowing granular control over which categories (like backend discovery) are output to stderr [5][6].
Citations:
- 1: https://codebrowser.dev/qt6/qtmultimedia/src/multimedia/qmediadevices.cpp.html
- 2: https://codebrowser.dev/qt6/qtmultimedia/src/multimedia/platform/qplatformmediaintegration.cpp.html
- 3: https://code.qt.io/cgit/qt/qtmultimedia.git/diff/src/multimedia/platform/qplatformmediaintegration.cpp?id=ccdc369cb82180bc12c3a6b00d33afad2848ba02
- 4: https://code.qt.io/cgit/qt/qtmultimedia.git/diff/?id=ccdc369cb82180bc12c3a6b00d33afad2848ba02
- 5: https://doc.qt.io/qt-6/advanced-ffmpeg-configuration.html
- 6: https://doc.qt.io/qt-6.11/advanced-ffmpeg-configuration.html
- 7: https://devhide.com/issue-with-qmediadevices-videoinputs-return-empty-list-but-qt-camera-example-works-fine-75959656
Avoid stdout writes before redirectStdout().
The initialization at src/CLIPipeline.cpp:1559-1566 runs before the saved STDOUT/redirect and custom message handler at line 1570. If QApplication construction, Qt Multimedia setup, or ONNX runtime setup write to stdout, CLI JSON/info output can be corrupted. Call redirectStdout() before these initializers, or ensure the direct stdout is limited to stderr and CLI output always uses CLIPipeline::writeOutput().
🤖 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/CLIPipeline.cpp` around lines 1559 - 1566, Move redirectStdout() in the
CLI initialization flow before MocapCameraHints::ensureMultimediaBackendSafe(),
QApplication construction, and OnnxRuntimeSettings::prepareRuntimeEnvironment().
Preserve the existing saved-STDOUT and custom message-handler setup while
ensuring these initializers cannot write directly to stdout before JSON or
informational output is routed safely.
Source: Coding guidelines
| TEST(OnnxRuntimeSettings, PreferGpuPersists) | ||
| { | ||
| QSettings settings; | ||
| settings.remove("ai/onnxPreferGpu"); | ||
|
|
||
| OnnxRuntimeSettings* ort = OnnxRuntimeSettings::instance(); | ||
| ort->loadSettings(); | ||
|
|
||
| const bool initial = ort->preferGpu(); | ||
| ort->setPreferGpu(!initial); | ||
| EXPECT_EQ(ort->preferGpu(), !initial); | ||
|
|
||
| OnnxRuntimeSettings* reloaded = OnnxRuntimeSettings::instance(); | ||
| reloaded->loadSettings(); | ||
| EXPECT_EQ(reloaded->preferGpu(), !initial); | ||
|
|
||
| ort->setPreferGpu(initial); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the persistence test independent of QTMESH_ONNX_PREFER_GPU.
preferGpu() returns the environment override when QTMESH_ONNX_PREFER_GPU is set. If that variable is set in the developer or CI environment, EXPECT_EQ(ort->preferGpu(), !initial) fails regardless of persistence. Unset the variable for the duration of the test.
💚 Proposed fix
TEST(OnnxRuntimeSettings, PreferGpuPersists)
{
+ const QByteArray savedOverride = qgetenv("QTMESH_ONNX_PREFER_GPU");
+ qunsetenv("QTMESH_ONNX_PREFER_GPU");
+
QSettings settings;
settings.remove("ai/onnxPreferGpu");
@@
ort->setPreferGpu(initial);
+ if (!savedOverride.isEmpty())
+ qputenv("QTMESH_ONNX_PREFER_GPU", savedOverride);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TEST(OnnxRuntimeSettings, PreferGpuPersists) | |
| { | |
| QSettings settings; | |
| settings.remove("ai/onnxPreferGpu"); | |
| OnnxRuntimeSettings* ort = OnnxRuntimeSettings::instance(); | |
| ort->loadSettings(); | |
| const bool initial = ort->preferGpu(); | |
| ort->setPreferGpu(!initial); | |
| EXPECT_EQ(ort->preferGpu(), !initial); | |
| OnnxRuntimeSettings* reloaded = OnnxRuntimeSettings::instance(); | |
| reloaded->loadSettings(); | |
| EXPECT_EQ(reloaded->preferGpu(), !initial); | |
| ort->setPreferGpu(initial); | |
| } | |
| TEST(OnnxRuntimeSettings, PreferGpuPersists) | |
| { | |
| const QByteArray savedOverride = qgetenv("QTMESH_ONNX_PREFER_GPU"); | |
| qunsetenv("QTMESH_ONNX_PREFER_GPU"); | |
| QSettings settings; | |
| settings.remove("ai/onnxPreferGpu"); | |
| OnnxRuntimeSettings* ort = OnnxRuntimeSettings::instance(); | |
| ort->loadSettings(); | |
| const bool initial = ort->preferGpu(); | |
| ort->setPreferGpu(!initial); | |
| EXPECT_EQ(ort->preferGpu(), !initial); | |
| OnnxRuntimeSettings* reloaded = OnnxRuntimeSettings::instance(); | |
| reloaded->loadSettings(); | |
| EXPECT_EQ(reloaded->preferGpu(), !initial); | |
| ort->setPreferGpu(initial); | |
| if (!savedOverride.isEmpty()) | |
| qputenv("QTMESH_ONNX_PREFER_GPU", savedOverride); | |
| } |
🧰 Tools
🪛 Cppcheck (2.21.0)
[error] 22-22: There is an unknown macro here somewhere. Configuration is required. If Q_PROPERTY is a macro then please configure it.
(unknownMacro)
🤖 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/OnnxRuntimeSettings_test.cpp` around lines 8 - 25, Update the
PreferGpuPersists test to clear QTMESH_ONNX_PREFER_GPU for the test’s duration
before reading preferGpu(), using the existing Qt environment-handling
facilities and restoring the prior environment value afterward so the test
remains isolated.
| void prependLdLibraryPath(const QStringList& dirs) | ||
| { | ||
| if (dirs.isEmpty()) | ||
| return; | ||
| QStringList merged = dirs; | ||
| const QByteArray old = qgetenv("LD_LIBRARY_PATH"); | ||
| if (!old.isEmpty()) | ||
| merged << QString::fromUtf8(old).split(':', | ||
| Qt::SkipEmptyParts); | ||
| qputenv("LD_LIBRARY_PATH", merged.join(':').toUtf8()); | ||
| if (qEnvironmentVariableIsSet("QTMESH_ONNX_DEBUG")) { | ||
| fprintf(stderr, "[onnx] LD_LIBRARY_PATH=%s\n", | ||
| qgetenv("LD_LIBRARY_PATH").constData()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does setenv LD_LIBRARY_PATH at runtime affect dlopen search path glibc
💡 Result:
Yes, setting the LD_LIBRARY_PATH environment variable at runtime does affect the search path used by dlopen in glibc [1][2][3]. When the dynamic linker searches for a shared object (when a path containing a slash is not provided to dlopen), it follows a specific search order. The LD_LIBRARY_PATH environment variable is included in this search sequence, provided the program is not running in a secure-execution mode (e.g., set-user-ID or set-group-ID programs), in which case it is ignored [1][3]. The general search order for the dynamic linker is [4][5]: 1. DT_RPATH: If the calling object contains a DT_RPATH tag and no DT_RUNPATH tag exists, these directories are searched first [1][3]. 2. LD_LIBRARY_PATH: The directories specified in the LD_LIBRARY_PATH environment variable are searched next [1][3]. 3. DT_RUNPATH: If the calling object contains a DT_RUNPATH tag, these directories are searched after LD_LIBRARY_PATH [1][3]. 4. /etc/ld.so.cache: The dynamic linker checks this cache file, which is maintained by ldconfig [1][3]. 5. Default Directories: Finally, the default system library directories (/lib and /usr/lib) are searched [1][3]. Because LD_LIBRARY_PATH appears early in this sequence, it can be used to override libraries found in later locations, such as those in /etc/ld.so.cache or the default system paths, and it takes precedence over DT_RUNPATH settings [4][5]. It does not, however, override DT_RPATH settings if they are present [5].
Citations:
- 1: https://man7.org/linux/man-pages/man3/dlopen.3.html
- 2: https://linuxman7.org/linux/man-pages/man3/dlopen.3.html
- 3: https://man7.org/linux/man-pages/man8/ld.so.8.html
- 4: https://unix.stackexchange.com/questions/367600/what-is-the-order-that-linuxs-dynamic-linker-searches-paths-in
- 5: https://embeddedpathashala.com/finding-shared-libraries-at-run-time-dynamic-linker-search-rules/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'OnnxRuntimeSettings\.cpp$' . || true
echo "== file excerpt =="
file="$(fd 'OnnxRuntimeSettings\.cpp$' . | head -n1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,140p' "$file" | cat -n
fi
echo "== related symbols =="
rg -n "prependLdLibraryPath|qputenv|LD_LIBRARY_PATH|Ort::Session|cudaProviderLibraryLoads|loadCudnn|cudnn|cudart|RTLD_GLOBAL|QLibrary|dlopen|SentryReporter::addBreadcrumb" . -g '!build*' -g '!node_modules' -g '!dist' -g '!out' | head -n 200Repository: fernandotonon/QtMeshEditor
Length of output: 24709
🌐 Web query:
glibc dynamic linker ld.so.8 LD_LIBRARY_PATH dlopen after startup LD_LIBRARY_PATH is added again
💡 Result:
In the glibc dynamic linker (ld.so), the LD_LIBRARY_PATH environment variable is processed only once during process startup [1]. The dynamic linker initializes its search path structures by reading the value of LD_LIBRARY_PATH at that time [2]. Because the environment variable is parsed and cached at startup, any subsequent modifications to the LD_LIBRARY_PATH environment variable made by the application (e.g., via setenv or putenv) after the program has started do not affect the behavior of subsequent dlopen calls [1]. The dynamic linker will continue to use the value of LD_LIBRARY_PATH as it existed when the process was initialized [2][1]. When dlopen is called, the dynamic linker searches for shared objects according to a specific, hard-coded order defined in the ld.so(8) manual page [3][4][5]. This search order includes the cached LD_LIBRARY_PATH from startup, but it does not check the current environment for any changes made since the program began execution [3][4][1]. To influence the search path for dlopen after startup, one must typically use alternatives such as setting the RPATH/RUNPATH in the binary, using dlmopen with a specific namespace, or manipulating the search path through linker-specific features if available.
Citations:
- 1: https://docs.oracle.com/cd/E86824_01/html/E54766/dlopen-3c.html
- 2: https://github.com/lattera/glibc/blob/a2f34833b1042d5d8eeb263b4cf4caaea138c4ad/elf/dl-support.c
- 3: https://linuxman7.org/linux/man-pages/man3/dlopen.3.html
- 4: https://man7.org/linux/man-pages/man3/dlopen.3.html
- 5: https://man.archlinux.org/man/core/man-pages/ld.so.8
Do not rely on runtime LD_LIBRARY_PATH changes for CUDA dependency resolution.
glibc's dynamic linker builds its dlopen search paths from LD_LIBRARY_PATH at startup; later qputenv()/setenv() updates are not picked up by subsequent dlopen calls unless the program is re-executed in secure-execution mode. In this path, cudaProviderLibraryLoads() succeeds because it uses QLibrary with an absolute path, but later libonnxruntime.so/provider dlopen()/ld.so resolution can still fail to find .cache/cuda-deps cudnn/cublas libraries. Use build-time RPATH/RUNPATH, absolute-path preloading with QLibrary/dlopen(RTLD_GLOBAL), or dlmopen() before creating the Ort::Session.
🤖 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/OnnxRuntimeSettings.cpp` around lines 62 - 76, Replace the runtime
LD_LIBRARY_PATH approach in prependLdLibraryPath with a loader-visible
dependency strategy: configure build-time RPATH/RUNPATH for the CUDA dependency
directory, or preload the required CUDA libraries by absolute path with
QLibrary/dlopen(RTLD_GLOBAL) before Ort::Session creation. Ensure
cudaProviderLibraryLoads and subsequent libonnxruntime/provider loading can
resolve cudnn/cublas dependencies without relying on qputenv.
| void OnnxRuntimeSettings::prepareRuntimeEnvironment() | ||
| { | ||
| #ifdef QTMESH_ONNX_GPU_BUILD | ||
| static bool done = false; | ||
| if (done) | ||
| return; | ||
| done = true; | ||
| prependLdLibraryPath(cudnnSearchPaths()); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the one-shot guard thread-safe.
prepareRuntimeEnvironment() is reachable from worker threads through configureSessionOptions() → tryAppendGpuExecutionProvider(), and also from the UI thread and CLIPipeline. The static bool done check-then-set is not atomic, so two threads can both run prependLdLibraryPath() and race on the qgetenv/qputenv pair for LD_LIBRARY_PATH.
🔒 Proposed fix using a call-once guard
void OnnxRuntimeSettings::prepareRuntimeEnvironment()
{
`#ifdef` QTMESH_ONNX_GPU_BUILD
- static bool done = false;
- if (done)
- return;
- done = true;
- prependLdLibraryPath(cudnnSearchPaths());
+ static std::once_flag once;
+ std::call_once(once, [] { prependLdLibraryPath(cudnnSearchPaths()); });
`#endif`
}Add #include <mutex>.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void OnnxRuntimeSettings::prepareRuntimeEnvironment() | |
| { | |
| #ifdef QTMESH_ONNX_GPU_BUILD | |
| static bool done = false; | |
| if (done) | |
| return; | |
| done = true; | |
| prependLdLibraryPath(cudnnSearchPaths()); | |
| #endif | |
| } | |
| void OnnxRuntimeSettings::prepareRuntimeEnvironment() | |
| { | |
| `#ifdef` QTMESH_ONNX_GPU_BUILD | |
| static std::once_flag once; | |
| std::call_once(once, [] { prependLdLibraryPath(cudnnSearchPaths()); }); | |
| `#endif` | |
| } |
🤖 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/OnnxRuntimeSettings.cpp` around lines 103 - 112, Replace the non-atomic
static bool done guard in prepareRuntimeEnvironment() with a thread-safe
call-once mechanism. Add `#include` <mutex> at the top of the file, then declare a
static std::once_flag inside the function and wrap the
prependLdLibraryPath(cudnnSearchPaths()) call with std::call_once to ensure it
executes exactly once regardless of concurrent access from multiple threads.
| bool OnnxRuntimeSettings::tryAppendGpuExecutionProvider(Ort::SessionOptions& so, | ||
| CoreMlStyle style) | ||
| { | ||
| prepareRuntimeEnvironment(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find every OnnxRuntimeSettings entry point and check main-thread initialization order.
rg -n -C4 'OnnxRuntimeSettings::(instance|prepareRuntimeEnvironment)\(' --type=cppRepository: fernandotonon/QtMeshEditor
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## files"
git ls-files | grep -E '(^|/)(OnnxRuntimeSettings\.(cpp|h)|main\.cpp|CLIPipeline\.(cpp|h))$|src/.*(Predi|TextureUpscaler)\.(cpp|h)$' | sed -n '1,200p'
echo "## OnnxRuntimeSettings symbols"
rg -n 'OnnxRuntimeSettings|OnnxRuntimeSettings::|instance\(' --type=cpp --type=h . | sed -n '1,220p'Repository: fernandotonon/QtMeshEditor
Length of output: 20828
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## OnnxRuntimeSettings.h"
sed -n '1,220p' src/OnnxRuntimeSettings.h
echo "## OnnxRuntimeSettings.cpp instance/configure/load/save"
sed -n '1,380p' src/OnnxRuntimeSettings.cpp
echo "## caller contexts"
for f in src/UniRigPredictor.cpp src/MotionInbetween.cpp; do
echo "### $f"
ast-grep outline "$f" --match 'OnnxRuntimeSettings' --view expanded || true
rg -n -C8 'OnnxRuntimeSettings::configureSessionOptions|tryAppendGpuExecutionProvider|QThread|thread\(' "$f" | sed -n '1,240p'
done
echo "## main.cpp relevant sections"
rg -n -C10 'OnnxRuntimeSettings|CLIPipeline|QCoreApplication|main\(' src/main.cpp
echo "## CLIPipeline relevant sections"
rg -n -C10 'OnnxRuntimeSettings|CLIPipeline|QThread|thread\(' src/CLIPipeline.cpp src/CLIPipeline.h | sed -n '1,260p'Repository: fernandotonon/QtMeshEditor
Length of output: 37796
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## main.cpp continued / OnnxRuntimeSettings touches"
sed -n '1,290p' src/main.cpp | rg -n -C6 'OnnxRuntimeSettings|CLIPipeline::run|AppLaunchHandler::isCliInvocation|QApplication|QCoreApplication|qmlRegisterSingletonType<OnnxRuntimeSettings>'
echo "## CLIPipeline OnnxRuntimeSettings usages"
rg -n -C6 'OnnxRuntimeSettings' src/CLIPipeline.cpp src/CLIPipeline.h
echo "## UniRig/MotionInbetween threading and calls"
rg -n -C8 'QThread|moveToThread|start\(|run\(|OnnxRuntimeSettings::configureSessionOptions' src/UniRigPredictor.cpp src/UniRigPredictor.h src/MotionInbetween.cpp src/MotionInbetween.h
echo "## TextureUpscaler threading"
if [ -f src/TextureUpscaler.cpp ]; then rg -n -C8 'QThread|moveToThread|class|start\(|run\(|OnnxRuntimeSettings' src/TextureUpscaler.cpp src/TextureUpscaler.h; fiRepository: fernandotonon/QtMeshEditor
Length of output: 17331
Ensure OnnxRuntimeSettings is created before worker-thread inference.
instance() constructs the QObject on the calling thread and does not install it in the GUI application’s thread. Worker threads can call configureSessionOptions() via UniRigPredictor, MotionInbetween, or TextureUpscaler, so signal connections and any GUI-thread settingsChanged() consumers can rely on worker-thread affinity. Add an early main-thread initialization call and require UI connections before the first possible background inference.
🤖 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/OnnxRuntimeSettings.cpp` around lines 288 - 291, Initialize the
OnnxRuntimeSettings singleton on the main/UI thread before any worker-thread
inference can begin, rather than first constructing it through
tryAppendGpuExecutionProvider or configureSessionOptions. Add the early
initialization at application startup and establish the required
settingsChanged() UI connections before background work starts, preserving
worker-thread calls through the existing OnnxRuntimeSettings::instance() path.
| #elif defined(_WIN32) && !defined(__MINGW32__) | ||
| try { | ||
| so.AppendExecutionProvider("DML", {}); | ||
| return true; | ||
| } catch (const Ort::Exception& e) { | ||
| if (qEnvironmentVariableIsSet("QTMESH_ONNX_DEBUG")) | ||
| fprintf(stderr, "[onnx] DirectML EP failed: %s\n", e.what()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
onnxruntime C++ AppendExecutionProvider "DML" string supported version
💡 Result:
In the C++ API for ONNX Runtime, the DirectML (DML) execution provider is typically initialized using specific provider-specific APIs rather than a generic string-based AppendExecutionProvider call [1][2][3]. The recommended method is to use the OrtDmlApi structure, which provides the SessionOptionsAppendExecutionProvider_DML function [1][2][3]. This requires retrieving the DML API through GetExecutionProviderApi using the string identifier "DML" [4][5][3]. Recommended Implementation Pattern: 1. Retrieve the DML API using ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, ...) [4][3]. 2. Use the returned OrtDmlApi pointer to call SessionOptionsAppendExecutionProvider_DML(sessionOptions, device_id) [4][3]. The legacy function OrtSessionOptionsAppendExecutionProvider_DML from dml_provider_factory.h is deprecated but remains available for backward compatibility [2][3]. While some other execution providers support a generic AppendExecutionProvider("ProviderName") interface, the DML provider historically relies on the specialized OrtDmlApi to handle device indexing and initialization [1][3]. As of recent versions, ONNX Runtime has been working toward supporting more canonical EP names in session options, but for production use with DML, the explicit OrtDmlApi approach is the standard and most reliable method [1][6].
Citations:
- 1: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 2: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 3: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 4: Choosing a device for Direct ML provider microsoft/onnxruntime#14840
- 5: https://github.com/fdwr/OnnxRuntimeDirectMLEPSample/blob/master/MainMinimalExample.cpp
- 6: microsoft/onnxruntime@v1.21.1...v1.22.0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant files =="
git ls-files | rg '(^|/)OnnxRuntime\.cmake$|src/OnnxRuntimeSettings\.cpp$|(^|/)CMakeLists\.txt$|(^|/)vcpkg\.json$|^README|onnxruntime|ort'
echo "== OnnxRuntime.cmake relevant occurrences =="
if [ -f cmake/OnnxRuntime.cmake ]; then
wc -l cmake/OnnxRuntime.cmake
sed -n '1,240p' cmake/OnnxRuntime.cmake | nl -ba
fi
echo "== OnnxRuntimeSettings.cpp relevant region =="
if [ -f src/OnnxRuntimeSettings.cpp ]; then
wc -l src/OnnxRuntimeSettings.cpp
sed -n '260,340p' src/OnnxRuntimeSettings.cpp | nl -ba
fi
echo "== direct AppendExecutionProvider/DML usages =="
rg -n 'AppendExecutionProvider|DML|OrtSessionOptionsAppendExecutionProvider_DML|dml_provider_factory|OnnxRuntime.cmake' .Repository: fernandotonon/QtMeshEditor
Length of output: 2699
🌐 Web query:
ONNX Runtime v1.19 AppendExecutionProvider DirectML DML provider string "DML"
💡 Result:
In ONNX Runtime (including v1.19), the string identifier used to access the DirectML Execution Provider API is "DML" [1][2][3]. To append the DirectML Execution Provider to your session options in C++, you should use the OrtDmlApi interface rather than a generic AppendExecutionProvider function. The process involves retrieving the DML API via GetExecutionProviderApi using the "DML" identifier [1][2][3]. Recommended implementation pattern: 1. Obtain the OrtApi and retrieve the OrtDmlApi [1][3]: const OrtApi& ortApi = Ort::GetApi; const OrtDmlApi* ortDmlApi = nullptr; ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast(&ortDmlApi)); 2. Use the retrieved ortDmlApi to append the provider to your SessionOptions [1][2][3]: ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, device_id); Note that the free function OrtSessionOptionsAppendExecutionProvider_DML is considered deprecated in favor of using the SessionOptionsAppendExecutionProvider_DML method available through the OrtDmlApi interface [4][1][2].
Citations:
- 1: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 2: https://github.com/fdwr/OnnxRuntimeDirectMLEPSample/blob/master/MainMinimalExample.cpp
- 3: Choosing a device for Direct ML provider microsoft/onnxruntime#14840
- 4: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
🌐 Web query:
ONNX Runtime v1.21 AppendExecutionProvider DirectML DML provider string "DML"
💡 Result:
In ONNX Runtime (including version 1.21), the provider string used to identify the DirectML Execution Provider is "DML" [1][2][3]. To register or append the DirectML Execution Provider to an inference session, the following approaches are typically used: 1. Recommended C/C++ API Approach: Rather than using a generic string-based append function for DirectML, it is standard practice to use the dedicated DirectML API. You first retrieve the DML-specific API structure using the string "DML" with the GetExecutionProviderApi method, then call the specialized registration function [1][2][4]: // Example using C API const OrtDmlApi* ortDmlApi = nullptr; Ort::GetApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast(&ortDmlApi)); ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, device_id); 2. Modern/Generic C++ Approach: Recent updates to ONNX Runtime allow for a more generic registration method that avoids including the legacy dml_provider_factory.h header [5]. This can be accomplished by calling the AppendExecutionProvider method directly on session options using the "DML" string [2][5]: sessionOptions.AppendExecutionProvider("DML", provider_options); Note on Usage: - The legacy function OrtSessionOptionsAppendExecutionProvider_DML (found in dml_provider_factory.h) is deprecated in favor of the newer SessionOptionsAppendExecutionProvider_DML accessible via the DML API pointer [6][1]. - In some language bindings (like C#), the DirectML provider may have limitations regarding how easily provider options (such as device_filter) can be passed compared to other execution providers, often necessitating the use of the explicit DML API over generic registration methods [7].
Citations:
- 1: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 2: C++ inference with DirectML:How to use DirectML in C++ library? microsoft/onnxruntime#11376
- 3: https://github.com/yalue/onnxruntime_go/blob/master/onnxruntime_wrapper.c
- 4: Choosing a device for Direct ML provider microsoft/onnxruntime#14840
- 5: https://github.com/tetherto/qvac/blob/main/packages/onnx/CHANGELOG.md
- 6: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 7: C# I need to run the program on NPU （OnnxRuntime + DirectML + NPU），but it failed microsoft/onnxruntime#19846
🌐 Web query:
ONNX Runtime v1.22 AppendExecutionProvider DirectML DML provider string "DML"
💡 Result:
In ONNX Runtime (including v1.22), the DirectML Execution Provider is not configured via a simple string passed to a generic AppendExecutionProvider method [1][2]. Instead, it uses a specific API interface retrieved via GetExecutionProviderApi [3][1][4]. The provider string used to retrieve the DirectML API via GetExecutionProviderApi is "DML" [3][1][5]. Key implementation details: 1. Retrieval: You must first obtain the OrtDmlApi interface by calling the standard ORT API with the provider string "DML" [3][1][6]. 2. Initialization: Once you have the OrtDmlApi pointer, you use it to call specific initialization functions, such as SessionOptionsAppendExecutionProvider_DML or SessionOptionsAppendExecutionProvider_DML2 (which allows for advanced options like device filtering) [7][1][6]. 3. Deprecation: The older free function OrtSessionOptionsAppendExecutionProvider_DML (found in dml_provider_factory.h) is considered deprecated; usage of the API retrieved through GetExecutionProviderApi is the recommended approach [7][1]. Example usage pattern (C++): OrtApi const& ortApi = Ort::GetApi; OrtDmlApi const* ortDmlApi = nullptr; ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast(&ortDmlApi)); Ort::SessionOptions sessionOptions; //... configure other options... ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, /device_id/ 0); [1][4] For high-level languages like Python, the provider string used in the providers list is 'DmlExecutionProvider' [6]. The.NET/C# environment typically exposes a dedicated AppendExecutionProvider_DML method on the SessionOptions object, rather than requiring the general-purpose append method [8][2].
Citations:
- 1: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 2: C# I need to run the program on NPU （OnnxRuntime + DirectML + NPU），but it failed microsoft/onnxruntime#19846
- 3: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 4: C++ inference with DirectML:How to use DirectML in C++ library? microsoft/onnxruntime#11376
- 5: https://github.com/yalue/onnxruntime_go/blob/master/onnxruntime_wrapper.c
- 6: https://mintlify.wiki/microsoft/onnxruntime/execution-providers/directml
- 7: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 8: https://github.com/microsoft/onnxruntime/blob/main/csharp/src/Microsoft.ML.OnnxRuntime/SessionOptions.shared.cs/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find exact files =="
git ls-files cmake/OnnxRuntime.cmake src/OnnxRuntimeSettings.cpp
echo "== version/pinned references in OnnxRuntime.cmake =="
sed -n '1,160p' cmake/OnnxRuntime.cmake
echo "== OnnxRuntimeSettings.cpp DirectML region =="
sed -n '280,330p' src/OnnxRuntimeSettings.cpp
echo "== all DirectML / AppendExecutionProvider references =="
rg 'AppendExecutionProvider|GetExecutionProviderApi|SessionOptionsAppendExecutionProvider_DML|OrtDmlApi|OrtSessionOptionsAppendExecutionProvider_DML|dml_provider_factory|ProviderName|DML|DirectML' .Repository: fernandotonon/QtMeshEditor
Length of output: 8288
🌐 Web query:
ONNX Runtime 1.20.1 SessionOptions AppendExecutionProvider DML provider_options official
💡 Result:
In ONNX Runtime, the DirectML (DML) execution provider is primarily managed via specialized factory functions rather than the generic SessionOptions::AppendExecutionProvider method used by some other providers [1][2]. For C++ and C API users, the recommended approach is to use the SessionOptionsAppendExecutionProvider_DML function (or SessionOptionsAppendExecutionProvider_DML2 for advanced options), which is exposed through the OrtDmlApi structure [3][4][5]. Configuration steps: 1. Retrieve the DML API: Obtain the OrtDmlApi via Ort::GetApi().GetExecutionProviderApi("DML", ...) [4][5]. 2. Configure Session Options: You must explicitly disable memory pattern and set execution mode to sequential, as required by the DML provider [1][4]. session_options.DisableMemPattern; session_options.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); 3. Append the Provider: Use the retrieved OrtDmlApi to append the DML provider [4][6]. ortDmlApi->SessionOptionsAppendExecutionProvider_DML(session_options, device_id); The older OrtSessionOptionsAppendExecutionProvider_DML function found in dml_provider_factory.h is deprecated but remains available for backward compatibility [3][4]. For C# users, the Microsoft.ML.OnnxRuntime.DirectML NuGet package provides a convenient method: sessionOptions.AppendExecutionProvider_DML(device_id); [1][7] When advanced configuration is needed, SessionOptionsAppendExecutionProvider_DML2 accepts OrtDmlDeviceOptions, which allows for specifying performance preferences (e.g., HighPerformance) and device filtering (e.g., GPU) [3][5]. Some high-level APIs or specific language bindings (like ORT GenAI) may support configuration via dictionary-based provider options, but the standard C/C++ path relies on these factory APIs [8].
Citations:
- 1: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 2: https://onnxruntime.ai/docs/api/c/struct_ort_1_1_session_options-members.html
- 3: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 4: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 5: https://mintlify.wiki/microsoft/onnxruntime/execution-providers/directml
- 6: C++ inference with DirectML:How to use DirectML in C++ library? microsoft/onnxruntime#11376
- 7: https://maanavd.github.io/docs/execution-providers/DirectML-ExecutionProvider.html
- 8: https://microsoft-onnxruntime-genai-88.mintlify.app/acceleration/directml
🌐 Web query:
ONNX Runtime 1.20.1 dml provider AppendExecutionProvider provider_options
💡 Result:
The DirectML (DML) execution provider in ONNX Runtime does not use a generic AppendExecutionProvider function with a provider_options dictionary to configure its settings. Instead, it utilizes a specialized, DML-specific API defined in include/onnxruntime/core/providers/dml/dml_provider_factory.h [1][2]. To configure and enable the DML execution provider, you should use the functions provided by the OrtDmlApi structure rather than the general SessionOptions::AppendExecutionProvider method used by other providers like QNN or XNNPACK [3][4][5]. Key APIs for DirectML Configuration: 1. SessionOptionsAppendExecutionProvider_DML(OrtSessionOptions* options, int device_id): This function enables the DML execution provider on a specific hardware adapter (device index) [1][2]. It is the standard approach for basic configuration [3]. Note that older versions of the API (e.g., OrtSessionOptionsAppendExecutionProvider_DML) are deprecated [2][3]. 2. SessionOptionsAppendExecutionProvider_DML2(OrtSessionOptions* options, OrtDmlDeviceOptions* device_opts): This function provides advanced configuration options [2][6]. The OrtDmlDeviceOptions struct allows you to specify: - Performance preference (e.g., HighPerformance, LowPower, Default) [2]. - Device filter (e.g., Gpu, Npu, None) [2]. Implementation Steps: To use these APIs in C++, follow this pattern [3][4]: 1. Obtain the DML API: Use Ort::GetApi().GetExecutionProviderApi("DML", ORT_API_VERSION, ...) to retrieve the OrtDmlApi pointer [3][4]. 2. Set Mandatory Session Options: Before appending the DML provider, you must configure the session options as follows [1][3]: - sessionOptions.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); - sessionOptions.DisableMemPattern(); 3. Append the Provider: Use the retrieved ortDmlApi to call either SessionOptionsAppendExecutionProvider_DML (for simple device ID selection) or SessionOptionsAppendExecutionProvider_DML2 (for advanced performance and device filtering) [2][3][4]. Do not use the generic AppendExecutionProvider("DML", provider_options) method, as it is not the supported interface for DirectML [4][5].
Citations:
- 1: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 2: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 3: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 4: https://github.com/fdwr/OnnxRuntimeDirectMLEPSample/blob/master/MainMinimalExample.cpp
- 5: https://onnxruntime.ai/docs/api/csharp/api/Microsoft.ML.OnnxRuntime.SessionOptions.html
- 6: https://mintlify.wiki/microsoft/onnxruntime/execution-providers/directml
🌐 Web query:
GitHub microsoft/onnxruntime 1.20.1 source AppendExecutionProvider DirectML provider_options
💡 Result:
In ONNX Runtime 1.20.1, the DirectML execution provider can be configured using provider_options when initializing an InferenceSession [1][2]. While C++ interfaces often rely on specialized factory functions in dml_provider_factory.h (such as SessionOptionsAppendExecutionProvider_DML1 or DML2) [3][4], the Python API allows passing a dictionary of options directly via the providers list [2]. Configuration Options The following keys can be used within the dictionary passed to the 'DmlExecutionProvider' in the providers list: - device_id (int): Specifies the hardware adapter index (e.g., 0 for the default GPU) [3][2]. - performance_preference (str): Defines performance behavior; common values include 'high_performance', 'default', or 'minimum_power' (or 'low_power') [5][2]. - device_filter (str): Filters the target device; typically supports 'gpu', 'npu', or 'any' [2]. Example (Python) To use specific configuration options in Python: session = ort.InferenceSession( "model.onnx", providers=[( 'DmlExecutionProvider', { 'device_id': 0, 'performance_preference': 'high_performance' })]) C++ Implementation For C++ development, direct manipulation of SessionOptions via AppendExecutionProvider is less common for DirectML than using the explicit API functions provided by the DML execution provider interface [6][4]. You typically retrieve the DML API and use functions like SessionOptionsAppendExecutionProvider_DML2 to apply settings like performance preferences and device filters [3][2]: // Example snippet for C++ const OrtDmlApi* dml_api = nullptr; Ort::ThrowOnError(Ort::GetApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast(&dml_api))); OrtDmlDeviceOptions device_options; device_options.Preference = OrtDmlPerformancePreference::HighPerformance; device_options.Filter = OrtDmlDeviceFilter::Gpu; dml_api->SessionOptionsAppendExecutionProvider_DML2(session_options, &device_options); Important Constraints The DirectML execution provider does not support memory pattern optimizations or parallel execution mode [4][7]. When configuring your session, you must ensure: - enable_mem_pattern is set to false [4][7]. - execution_mode is set to ExecutionMode::ORT_SEQUENTIAL [4][7]. These requirements apply to both C++ and other language bindings to avoid runtime errors [4][7].
Citations:
- 1: [Build] Issues with Multithreading in the New Versions of onnxruntime-directml microsoft/onnxruntime#22867
- 2: https://mintlify.wiki/microsoft/onnxruntime/execution-providers/directml
- 3: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 4: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 5: https://microsoft-onnxruntime-genai-88.mintlify.app/acceleration/directml
- 6: C++ inference with DirectML:How to use DirectML in C++ library? microsoft/onnxruntime#11376
- 7: https://tomwildenhain-microsoft.github.io/onnxruntime/docs/execution-providers/DirectML-ExecutionProvider.html
🌐 Web query:
ONNX Runtime 1.18 AppendExecutionProvider DML provider_options C++
💡 Result:
In ONNX Runtime (including version 1.18), the DirectML (DML) Execution Provider is typically enabled in C++ using specialized factory functions rather than a generic AppendExecutionProvider call [1][2]. ### Recommended Approach The recommended way to use DirectML in C++ is to use the OrtDmlApi interface to call SessionOptionsAppendExecutionProvider_DML [3][4]. This method is preferred over the deprecated free function OrtSessionOptionsAppendExecutionProvider_DML [3][4]. #### Implementation Example To use this, you retrieve the OrtDmlApi from the ONNX Runtime API and configure the required session options [3][5]: cpp // 1. Retrieve the DML API OrtApi const& ortApi = Ort::GetApi(); OrtDmlApi const* ortDmlApi = nullptr; ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast<const void>(&ortDmlApi)); // 2. Configure mandatory session options for DML Ort::SessionOptions sessionOptions; sessionOptions.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); // Mandatory sessionOptions.DisableMemPattern(); // Mandatory // 3. Append the DML Execution Provider // device_id 0 corresponds to the default adapter ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, 0); ### Important Notes * Mandatory Settings: When using the DirectML EP, you must call DisableMemPattern() and set the execution mode to ExecutionMode::ORT_SEQUENTIAL on your SessionOptions object, or the session creation will fail [1][2]. * Device Options: For more advanced configurations (such as performance preferences or device filtering), you can use the SessionOptionsAppendExecutionProvider_DML2 function, which accepts OrtDmlDeviceOptions [4]. * Alternative (Newer) API: In environments using the newer Windows ML-style explicit EP selection (where you enumerate devices first), you may use AppendExecutionProvider_V2 with an env and ep_options map [6][7]. However, for standard cross-platform DirectML usage as typically deployed in ONNX Runtime, the OrtDmlApi approach above remains the standard [3][1]. The headers required for these APIs are located in include/onnxruntime/core/providers/dml/dml_provider_factory.h [4][1].
Citations:
- 1: https://skottmckay.github.io/docs/execution-providers/DirectML-ExecutionProvider.html
- 2: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 3: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 4: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 5: C++ inference with DirectML:How to use DirectML in C++ library? microsoft/onnxruntime#11376
- 6: https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/select-execution-providers
- 7: https://github.com/MicrosoftDocs/windows-ai-docs/blob/docs/docs/new-windows-ml/select-execution-providers.md
🌐 Web query:
ONNX Runtime 1.22.0 Release Notes AppendExecutionProvider DirectML provider_options C++ provider_options AppendExecutionProvider_CUDA CoreML DML
💡 Result:
ONNX Runtime 1.22.0 was released on May 10, 2025 [1][2]. Key updates in this release include integrated KleidiAI support for Arm architectures, MatMulNBits support for 8-bit quantized weights, support for TensorRT 10.9, and a new execution provider specifically for NVIDIA RTX GPUs [1][2]. In the ONNX Runtime C++ API, execution providers are configured through SessionOptions [3][4]. The AppendExecutionProvider family of methods is used to add these providers to an inference session [3][5]. AppendExecutionProvider Overview: The primary method for adding execution providers with custom options is: SessionOptions& AppendExecutionProvider(const std::string& provider_name, const std::unordered_map<std::string, std::string>& provider_options = {}); [3][5] Specific Provider Append Methods: For many providers, dedicated C++ methods exist that accept specific structs for configuration, ensuring type safety [3][4]. - AppendExecutionProvider_CUDA: Accepts OrtCUDAProviderOptions [3][5]. - AppendExecutionProvider_TensorRT: Accepts OrtTensorRTProviderOptions [3][5]. - AppendExecutionProvider_OpenVINO: Accepts OrtOpenVINOProviderOptions [3][5]. Usage Examples: - CoreML: Uses the string-based AppendExecutionProvider method with a map of options [6][7]. so.AppendExecutionProvider("CoreML", {{"ModelFormat", "MLProgram"}, {"MLComputeUnits", "ALL"}}); [6] - DirectML: While often configured via JSON in higher-level APIs like Generative AI [8], in the C++ API, it is typically managed by adding the provider string to the session options [8]. - Modern Approach (V2): For newer workflows, especially those involving complex hardware selection, AppendExecutionProvider_V2 is recommended, which allows passing a list of devices (via env.GetEpDevices) and provider options as a map [3][4][9]. Note on CUDA: Starting with version 1.22.0, ONNX Runtime GPU packages require CUDA 12.x; packages built for CUDA 11.x are no longer published [2][10].
Citations:
- 1: https://github.com/Microsoft/onnxruntime/releases/tag/v1.22.0
- 2: https://newreleases.io/project/github/microsoft/onnxruntime/release/v1.22.0
- 3: https://onnxruntime.ai/docs/api/c/onnxruntime__cxx__api_8h_source.html
- 4: https://onnxruntime.ai/docs/api/c/struct_ort_1_1_session_options-members.html
- 5: https://github.com/microsoft/onnxruntime/blob/ced7c2deac958391414d2bbf951f86e2fc904b05/include/onnxruntime/core/session/onnxruntime_cxx_api.h
- 6: https://onnxruntime.ai/docs/execution-providers/CoreML-ExecutionProvider.html
- 7: https://github.com/microsoft/onnxruntime/blob/gh-pages/docs/execution-providers/CoreML-ExecutionProvider.md
- 8: https://microsoft-onnxruntime-genai-88.mintlify.app/acceleration/directml
- 9: https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/select-execution-providers
- 10: https://newreleases.io/project/pypi/onnxruntime/release/1.22.0
🌐 Web query:
"V1.20.1 (Jan 30, 2024)" "Providers" "ONNX Runtime"
💡 Result:
The premise that ONNX Runtime version 1.20.1 was released on January 30, 2024, is incorrect. Official records from the Microsoft ONNX Runtime repository indicate that version 1.20.1 was published on November 21, 2024 [1][2]. As of August 2026, version 1.20.1 is a legacy release, with subsequent versions up to 1.28.0 and beyond having been released in the interim [3].
Citations:
- 1: https://github.com/microsoft/onnxruntime/releases/tag/v1.20.1
- 2: https://github.com/Microsoft/onnxruntime/releases/tag/v1.20.1
- 3: https://github.com/microsoft/onnxruntime/releases
🌐 Web query:
ONNX Runtime 1.22.0 Release Notes providers C++ provider_options AppendExecutionProvider DirectML
💡 Result:
In ONNX Runtime 1.22.0, adding the DirectML Execution Provider to a session is primarily handled via the C API's OrtDmlApi struct or language-specific wrappers that interact with it [1][2]. The DirectML Execution Provider does not have a dedicated AppendExecutionProvider_DML method in the standard C++ Ort::SessionOptions class, unlike providers such as CUDA or TensorRT [3][4]. Instead, to add DirectML, you typically retrieve the DirectML API from the session options and invoke the provider-specific initialization function [1]. Key points for managing DirectML provider options in C++: 1. Retrieval: You can obtain the OrtDmlApi pointer using OrtApi::GetExecutionProviderApi with the provider name "DML" [1]. 2. Append Function: The OrtDmlApi provides the SessionOptionsAppendExecutionProvider_DML function, which accepts the OrtSessionOptions and the device ID as arguments [1]. 3. SessionOptions::AppendExecutionProvider: While Ort::SessionOptions contains a generic AppendExecutionProvider(const std::string& provider_name, ...) method, this is generally intended for providers that support the standard dictionary-based options interface [3][4]. As of 1.22.0, the recommended approach for DirectML remains utilizing the specific OrtDmlApi to ensure correct initialization of the DML device [1]. Release 1.22.0 introduced significant updates across other providers (such as TensorRT 10.9 support and new Nvidia RTX-specific execution providers) and new C++ wrapper classes for various provider options [5][6][7], but DirectML integration in the C++ API continues to rely on the established OrtDmlApi pattern [1][2].
Citations:
- 1: https://github.com/yalue/onnxruntime_go/blob/master/onnxruntime_wrapper.c
- 2: https://deepwiki.com/microsoft/onnxruntime/5.3-c-bindings
- 3: https://github.com/microsoft/onnxruntime/blob/ced7c2deac958391414d2bbf951f86e2fc904b05/include/onnxruntime/core/session/onnxruntime_cxx_api.h
- 4: https://onnxruntime.ai/docs/api/c/struct_ort_1_1_session_options-members.html
- 5: https://github.com/Microsoft/onnxruntime/releases/tag/v1.22.0
- 6: https://newreleases.io/project/github/microsoft/onnxruntime/release/v1.22.0
- 7: Introduce new C++ API for C interfaces microsoft/onnxruntime#25762
🌐 Web query:
ONNX Runtime SessionProviderOptionsAppendExecutionProvider DirectML 1.22.0 AppendExecutionProvider provider_options
💡 Result:
In ONNX Runtime 1.22.0, there is no single function named SessionProviderOptionsAppendExecutionProviderDirectML. Instead, the DirectML Execution Provider (EP) is typically added to an inference session using specific APIs provided by the OrtDmlApi or the C++ SessionOptions interface [1][2][3]. The primary methods for adding the DirectML EP are as follows: 1. SessionOptionsAppendExecutionProvider_DML: This function takes an OrtSessionOptions object and a device_id (integer representing the adapter index) to initialize the DirectML provider [2][3]. It is accessed via the OrtDmlApi [3][4]. 2. SessionOptionsAppendExecutionProvider_DML1: This function is used when you need to provide a specific DirectML device (IDMLDevice) and D3D12 command queue (ID3D12CommandQueue) rather than relying on an adapter index [1][2]. Usage Note: The older function OrtSessionOptionsAppendExecutionProvider_DML is officially deprecated in favor of the SessionOptionsAppendExecutionProvider_DML method accessed through GetExecutionProviderApi [2][3]. Regarding the term "provider_options" (as seen in generic AppendExecutionProvider calls for other EPs like CUDA or TensorRT): The DirectML EP does not expose a configuration dictionary via the standard AppendExecutionProvider(name, options) pattern used by other providers in all languages [5]. Users often find that attempting to pass a dictionary of provider options (e.g., for device filtering) to the DirectML EP via generic C# or C++ wrappers fails because the DirectML implementation expects specific initialization APIs rather than a generic configuration map [5]. To use the DirectML EP, you must also ensure your session options are configured correctly, specifically by disabling memory pattern optimizations and setting the execution mode to sequential [1][3][4]: - sessionOptions.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); - sessionOptions.DisableMemPattern; [1] Top results: [1][2][3][5]
Citations:
- 1: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 2: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 3: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 4: C++ inference with DirectML:How to use DirectML in C++ library? microsoft/onnxruntime#11376
- 5: C# I need to run the program on NPU （OnnxRuntime + DirectML + NPU），but it failed microsoft/onnxruntime#19846
🌐 Web query:
"AppendExecutionProvider_CUDA" "AppendExecutionProvider" "DML" "ONNXRuntime" "v1.22.0"
💡 Result:
In ONNX Runtime, you typically configure Execution Providers (EPs) by “appending” them to SessionOptions (C/C++) or by specifying an ordered provider list (Python). For ONNX Runtime 1.22.0, the relevant CUDA EP and DirectML EP enablement APIs are: 1) AppendExecutionProvider_CUDA (CUDAExecutionProvider) - ONNX Runtime exposes the CUDA EP through the SessionOptions C API, including the function SessionOptionsAppendExecutionProvider_CUDA (and a newer “_V2” variant using a structured options object). An example from the official CUDA EP doc shows configuring an OrtCUDAProviderOptions (device_id, gpu_mem_limit, etc.) and calling SessionOptionsAppendExecutionProvider_CUDA(session_options, &options). It also shows the V2 flow with CreateCUDAProviderOptions, UpdateCUDAProviderOptions, then SessionOptionsAppendExecutionProvider_CUDA_V2(session_options, cuda_options). [1] - Operational note (from the same official CUDA EP doc): “Starting with version 1.27, GPU packages published to PyPI (onnxruntime-gpu) and NuGet … are built with CUDA 13.0 by default. Older GPU package versions are built with CUDA 12.8 by default.” (This is a versioning note; it’s in the CUDA EP doc and is what the doc explicitly states.) [1] 2) DirectML enablement (DML) - ONNX Runtime has a DirectML Execution Provider doc describing enabling it via the DML provider factory / C API (when using a DML-enabled build). [2] - The DirectML C API entry point is OrtSessionOptionsAppendExecutionProvider_DML (in dml_provider_factory.h). That header comment states this export is deprecated and recommends using the OrtDmlApi / SessionOptionsAppendExecutionProvider_DML instead. [3] - The DirectML doc also describes what device_id means (adapter index; 0 is default adapter; negative invalid) and that there is an alternative DML API that uses a provided D3D12 command queue/device (the doc discusses “SessionOptionsAppendExecutionProvider_DML1”). [2] How to “AppendExecutionProvider_CUDA + DML” together (conceptual) - ONNX Runtime uses an EP priority order: when multiple EPs are registered, nodes are selected from them based on precedence, with fallback to CPU where needed. The execution-provider overview and Python API docs explicitly describe that the provider list is ordered by priority and that CUDAExecutionProvider is used if capable, otherwise CPU is used. [4][5] - Therefore, to use both in one session, you would: a) Build/obtain an ONNX Runtime binary that includes both CUDA EP and DML EP. b) Register both EPs on the same session options object in the desired precedence order (CUDA higher priority than DML, or vice versa), and let ONNX Runtime place each node on the first EP that supports it. c) Use the CUDA EP options struct/API for CUDA (AppendExecutionProvider_CUDA or SessionOptionsAppendExecutionProvider_CUDA_V2) and the DML EP API (SessionOptionsAppendExecutionProvider_DML / DML1 variants) for DML. [1][2][3] About the “AppendExecutionProvider_DML” C++ method name - Directly relevant caveat: there is not always a direct C++ wrapper method named AppendExecutionProvider_DML; the DML API is often accessed via the DML execution provider API pointer (ortDmlApi) rather than a C++ SessionOptions member function, while the C free function remains deprecated. [6] If you want a concrete code snippet for your exact language binding (C API vs C++ wrapper vs Python) and platform (Windows for DML, Linux/Windows for CUDA), tell me which one you’re using and whether you want provider priority CUDA first or DML first.
Citations:
- 1: https://onnxruntime.ai/docs/execution-providers/CUDA-ExecutionProvider.html
- 2: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 3: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 4: https://onnxruntime.ai/docs/execution-providers/
- 5: https://onnxruntime.ai/docs/api/python/api_summary.html
- 6: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
🌐 Web query:
"provider_options" "DirectML" "ONNX Runtime" "C++" "AppendExecutionProvider"
💡 Result:
To use the DirectML execution provider in ONNX Runtime with C++, you should avoid the deprecated free function OrtSessionOptionsAppendExecutionProvider_DML and instead use the OrtDmlApi via the GetExecutionProviderApi method [1][2][3]. ### Recommended Approach The modern, recommended approach is to access the DirectML-specific API through the ONNX Runtime C API. This provides access to more advanced functions like SessionOptionsAppendExecutionProvider_DML (the non-deprecated version) and SessionOptionsAppendExecutionProvider_DML2 [1][4][3]. 1. Retrieve the DML API: Use Ort::GetApi().GetExecutionProviderApi to obtain a pointer to the OrtDmlApi structure [4][5][3]. 2. Configure Session Options: You must set specific session options for DirectML to function correctly: * Set execution mode to sequential: session_options.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); [6][2][3] * Disable memory pattern: session_options.DisableMemPattern(); [6][2][3] 3. Append the Provider: Use the retrieved OrtDmlApi to call the append function [4][5][3]. ### C++ Code Example cpp `#include` <onnxruntime_cxx_api.h> `#include` <dml_provider_factory.h> // 1. Get the DirectML API const OrtApi& ortApi = Ort::GetApi(); const OrtDmlApi* ortDmlApi = nullptr; Ort::ThrowOnError(ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast<const void>(&ortDmlApi))); // 2. Setup Session Options Ort::SessionOptions sessionOptions; sessionOptions.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); sessionOptions.DisableMemPattern(); // 3. Append the DirectML Execution Provider // For basic usage (device index 0): Ort::ThrowOnError(ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, 0)); // Alternatively, for advanced configuration (e.g., High Performance/GPU filter): // OrtDmlDeviceOptions device_options; // device_options.Preference = OrtDmlPerformancePreference::HighPerformance; // device_options.Filter = OrtDmlDeviceFilter::Gpu; // Ort::ThrowOnError(ortDmlApi->SessionOptionsAppendExecutionProvider_DML2(sessionOptions, &device_options)); // Create session Ort::Session session(env, L"model.onnx", sessionOptions); ### Key Differences * Deprecated: The free function OrtSessionOptionsAppendExecutionProvider_DML found in dml_provider_factory.h is deprecated [1][3]. While it may still work in some versions, it is not recommended for new development [3]. * Modern API: SessionOptionsAppendExecutionProvider_DML (via OrtDmlApi) is the current standard [1][3]. * Advanced Features: SessionOptionsAppendExecutionProvider_DML2 allows for more fine-grained control, such as specifying performance preferences (e.g., high power, low power) and device filtering (e.g., GPU vs. NPU) [1][4].
Citations:
- 1: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 2: https://github.com/fdwr/OnnxRuntimeDirectMLEPSample/blob/master/MainMinimalExample.cpp
- 3: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 4: https://mintlify.wiki/microsoft/onnxruntime/execution-providers/directml
- 5: Choosing a device for Direct ML provider microsoft/onnxruntime#14840
- 6: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
🌐 Web query:
ONNX Runtime v1.22.1 v1.23 AppendExecutionProvider DirectML provider_options C++ "DML"
💡 Result:
In ONNX Runtime (including recent versions like 1.22.1 and 1.23), the DirectML (DML) Execution Provider is configured in C++ primarily through the dedicated OrtDmlApi interface rather than the generic AppendExecutionProvider method used by some other providers [1][2][3]. To use the DirectML Execution Provider, you must interact with the dml_provider_factory.h header and the OrtDmlApi [4][5][2]. Recommended C++ Integration Pattern: 1. Obtain the DML API: Use GetExecutionProviderApi to retrieve the OrtDmlApi struct, which provides access to the DML-specific functions [1][2][3]. cpp const OrtApi& ortApi = Ort::GetApi(); const OrtDmlApi* ortDmlApi = nullptr; ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast<const void>(&ortDmlApi)); 2. Configure Session Options: DirectML requires specific session settings. You must disable memory pattern optimizations and set the execution mode to sequential [4][1][2]. cpp Ort::SessionOptions sessionOptions; sessionOptions.SetExecutionMode(ExecutionMode::ORT_SEQUENTIAL); sessionOptions.DisableMemPattern(); 3. Append the DirectML Provider: Use the recommended SessionOptionsAppendExecutionProvider_DML function provided by OrtDmlApi [1][2]. cpp // Append DML provider for device index 0 (default adapter) ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, 0); Advanced Configuration: If you need more control, such as specifying performance preferences (e.g., high power, low power) or hardware device filtering (GPU/NPU), dml_provider_factory.h exports SessionOptionsAppendExecutionProvider_DML2 [5]. This function accepts an OrtDmlDeviceOptions struct, allowing for more granular hardware selection and power management than the basic device index [5]. Note on Deprecation: While older tutorials may reference OrtSessionOptionsAppendExecutionProvider_DML (a free function), it is deprecated [5][1][2]. The SessionOptionsAppendExecutionProvider_DML method accessed via OrtDmlApi is the current standard for C++ implementations [1][2]. Memory Management: Because DirectML operates directly on D3D12 resources, ensure your application manages the lifecycle of any custom D3D12 devices or command queues passed through advanced DML APIs (e.g., SessionOptionsAppendExecutionProvider_DML1), as the inference session will maintain references to these objects [4][5].
Citations:
- 1: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 2: https://github.com/fdwr/OnnxRuntimeDirectMLEPSample/blob/master/MainMinimalExample.cpp
- 3: Choosing a device for Direct ML provider microsoft/onnxruntime#14840
- 4: https://onnxruntime.ai/docs/execution-providers/DirectML-ExecutionProvider.html
- 5: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
🌐 Web query:
ONNX Runtime C++ API generic provider_options provider registration ProviderName map AppendExecutionProvider V2 AppendExecutionProvider_CUDA CoreML DirectML DML
💡 Result:
In ONNX Runtime (ORT), execution provider (EP) registration and configuration via the C++ API are managed through the Ort::SessionOptions class, which provides specialized methods for standard EPs and a generic interface for others [1][2]. Generic Provider Registration For EPs not having dedicated Append... methods, use the generic AppendExecutionProvider method. This requires the provider name and a map of options: session_options.AppendExecutionProvider("ProviderName", {{"option_key", "option_value"}}); [1][2] Advanced/Plugin Registration (AppendExecutionProvider_V2) For plugin EPs or scenarios requiring precise hardware device selection (such as specific NPU devices), AppendExecutionProvider_V2 is used [3][4]. This method works with Ort::Env::GetEpDevices() to retrieve available hardware devices and accepts a map of key-value pairs for configuration [3][5]. 1. Enumerate available devices: std::vectorOrt::ConstEpDevice devices = env.GetEpDevices; [3][5] 2. Select desired devices, then append: session_options.AppendExecutionProvider_V2(env, {selected_devices}, ep_options); [3][5] CUDA and Specialized EPs Dedicated methods exist for EPs like CUDA, TensorRT, and others, often supporting both original and "V2" versions of option structures [2][6]. - CUDA: Use AppendExecutionProvider_CUDA (for OrtCUDAProviderOptions) or AppendExecutionProvider_CUDA_V2 (for OrtCUDAProviderOptionsV2) [2][6]. - CoreML/DirectML: While specialized APIs may vary by version and platform, general provider options are passed via the standard AppendExecutionProvider or session-specific configuration methods [4][7]. For DirectML (DML), typically one uses the generic AppendExecutionProvider with appropriate key-value pairs if not using the specific Windows ML helper methods [4][5]. Key API Notes: - V2 methods: Generally introduce updated option structures (e.g., OrtCUDAProviderOptionsV2) that allow for more flexible, string-based, or structured updates to provider configurations [2][8][6]. - API Header: These methods are defined in onnxruntime_cxx_api.h [2][9]. - Consistency: While AppendExecutionProvider is the most generic, always check the specific documentation for an EP to see if a dedicated Append... function is provided, as these offer type-safe configuration structs [2][8].
Citations:
- 1: https://onnxruntime.ai/docs/api/c/struct_ort_1_1_session_options-members.html
- 2: https://github.com/microsoft/onnxruntime/blob/ced7c2deac958391414d2bbf951f86e2fc904b05/include/onnxruntime/core/session/onnxruntime_cxx_api.h
- 3: https://onnxruntime.ai/docs/execution-providers/plugin-ep-libraries/usage.html
- 4: https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/select-execution-providers
- 5: https://github.com/MicrosoftDocs/windows-ai-docs/blob/docs/docs/new-windows-ml/select-execution-providers.md
- 6: https://onnxruntime.ai/docs/execution-providers/CUDA-ExecutionProvider.html
- 7: https://onnxruntime.ai/docs/execution-providers/
- 8: https://onnxruntime.ai/docs/execution-providers/TensorRT-ExecutionProvider.html
- 9: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/session/onnxruntime_cxx_api.h
🌐 Web query:
ORTHelper ProviderName provider_options parser DirectML DML SessionOptionsAppendExecutionProvider_DML C++ provider_options
💡 Result:
In ONNX Runtime (ORT), the configuration of execution providers (EPs) like DirectML (DML) via provider_options is handled through specific APIs depending on your programming language and the specific requirements of the provider [1][2]. DirectML C++ Configuration For the DirectML execution provider in C++, you should generally use the SessionOptionsAppendExecutionProvider_DML method available via the OrtDmlApi [3][4]. This is the recommended, non-deprecated way to append the DML provider and configure it [3][4]. 1. Obtain the OrtDmlApi: Use GetExecutionProviderApi from the primary OrtApi [3][5]. 2. Append the provider: Use SessionOptionsAppendExecutionProvider_DML on your SessionOptions object [3][5]. Example C++ usage: const OrtApi& ortApi = Ort::GetApi; const OrtDmlApi* ortDmlApi = nullptr; ortApi.GetExecutionProviderApi("DML", ORT_API_VERSION, reinterpret_cast(&ortDmlApi)); Ort::SessionOptions sessionOptions; // Configure session options... ortDmlApi->SessionOptionsAppendExecutionProvider_DML(sessionOptions, device_id); Note: The older free function OrtSessionOptionsAppendExecutionProvider_DML is deprecated [4][6]. For advanced configurations (like specifying performance preferences or device filters), SessionOptionsAppendExecutionProvider_DML2 is available [4]. ProviderOptions and Parsing While some higher-level ORT bindings (like Python or GenAI) support passing a dictionary/map of provider_options directly [7][2][8], the core C++ engine uses a specific mechanism for parsing these options [9][10]. - ProviderOptionsParser: This class (found in onnxruntime::ProviderOptionsParser) is used internally within ORT to register and parse provider-specific options [9][10]. It allows you to add value parsers, assignment to references, or enum mappings to handle string-based configuration values during the initialization of the execution provider [9][11]. - "ORTHelper": This appears to be a context-specific or internal reference, likely related to how specific language bindings or wrappers (like Objective-C/Swift) bridge the gap between their dictionary-based options and the C++ SessionOptions::AppendExecutionProvider interface [1][12]. For example, the Objective-C API translates an NSDictionary of provider options into a C++ std::unordered_map<std::string, std::string> before calling AppendExecutionProvider [12]. If you are encountering difficulty configuring specific DML options (like metacommands) in GenAI, it is often because the GenAI configuration layer needs to explicitly support passing those keys through to the underlying ORT DML provider [13]. Not all ORT provider options are automatically exposed in all higher-level config files [13].
Citations:
- 1: https://onnxruntime.ai/docs/api/objectivec/Classes/ORTSessionOptions.html
- 2: https://onnxruntime.ai/docs/genai/reference/config.html
- 3: Does API AppendExecutionProvider_DML exist for C++? microsoft/onnxruntime#14610
- 4: https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/providers/dml/dml_provider_factory.h
- 5: Choosing a device for Direct ML provider microsoft/onnxruntime#14840
- 6: https://www.sidefx.com/docs/hdk/dml__provider__factory_8h.html
- 7: https://microsoft-onnxruntime-genai-88.mintlify.app/acceleration/directml
- 8: https://ryzenai.docs.amd.com/_/downloads/en/latest/pdf/
- 9: https://www.sidefx.com/docs/hdk/classonnxruntime_1_1_provider_options_parser.html
- 10: http://www.sidefx.com/docs/hdk/provider__options__utils_8h_source.html
- 11: https://www.sidefx.com/docs/hdk20.5/classonnxruntime_1_1_provider_options_parser.html
- 12: TexasInstruments/onnxruntime@f4cd35f
- 13: [Documentation] Running genai-directml-quantized models with metacommands disabled microsoft/onnxruntime#22466
Use the DirectML-specific API instead of generic options.
AppendExecutionProvider("DML", {}) does not match the DirectML provider options supported by cmake/OnnxRuntime.cmake versions. On V1.20.1, use the DirectML API/low-level SessionOptionsAppendExecutionProvider_DML(...), and configure the required DirectML session options; this call can throw and fail back to CPU because no DirectML provider is registered.
🤖 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/OnnxRuntimeSettings.cpp` around lines 305 - 312, Replace the generic
AppendExecutionProvider("DML", {}) call in the Windows non-MinGW32 conditional
block with the DirectML-specific low-level API
SessionOptionsAppendExecutionProvider_DML(...) and configure the required
DirectML session options according to the V1.20.1 build configuration in
cmake/OnnxRuntime.cmake. Keep the existing try-catch around this call since it
can throw and should fall back to CPU when the DirectML provider is not
registered.
Free function in an anonymous namespace could not access private Impl; move unregisterSkinningListener onto Impl so MocapController call sites compile. Co-authored-by: Cursor <cursoragent@cursor.com>
The review fix skipped the head bone entirely in the body setup loop, which also skipped zeroing its animation blend mask — idle clips could fight manual drive. Still exclude head from bodyManualRestore (preserve headWasManuallyControlled) but zero blend masks on all bones; track skinning listener registration. Co-authored-by: Cursor <cursoragent@cursor.com>
Expose bodyCalibrationHint on MocapController and surface it in the Performance Capture panel while torso neutral is being captured. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Mocap/MocapController.cpp (2)
913-922: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRecord the startup timeout cause.
This path shows a user-facing error and stops preview, but it does not record why preview stopped. Add a breadcrumb before
stopPreview()that identifies the no-frame timeout.As per coding guidelines, “Track all user-facing actions and significant operations with
SentryReporter::addBreadcrumb.”Proposed change
emit errorOccurred(d->status); + SentryReporter::addBreadcrumb( + "ai.assist.mocap_live", "preview start timeout: no frames"); stopPreview();🤖 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/Mocap/MocapController.cpp` around lines 913 - 922, Add a SentryReporter::addBreadcrumb call in the cameraStartupTimer timeout handler after emitting errorOccurred and immediately before stopPreview(), recording that preview stopped because no frames arrived during camera startup. Keep the existing user-facing status message and shutdown flow unchanged.Source: Coding guidelines
772-805: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore all body-drive overrides on the pose-model fallback.
Lines 774-805 set every non-head bone to manual control and zero every animation blend-mask entry. The fallback restores only
bodyBones, which excludes non-canonical bones, and does not restorebodyAnimMaskRestore. A face/head-only fallback retains body-drive overrides until preview stops. Restore the complete manual-bone and blend-mask snapshots here before clearingbodyRetargeter. Preserve the head-bone setup.Also applies to: 876-881
🤖 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/Mocap/MocapController.cpp` around lines 772 - 805, Update the pose-model fallback cleanup near the body-drive teardown, including the analogous path around the secondary location, to restore every snapshot in bodyManualRestore rather than only canonical bodyBones, and restore each saved entry in bodyAnimMaskRestore before clearing bodyRetargeter. Preserve the existing head-bone setup and ensure non-canonical bones and animation blend-mask values return to their captured states.
🤖 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/Mocap/MocapController.cpp`:
- Around line 775-779: Move the body manual-bone snapshot logic around the
preview reset so it captures each bone’s pre-reset orientation and
manual-control state before calling skel->reset(true). After the reset, use the
post-reset orientation when populating BodyDriveBone bind data, and include the
manually controlled head bone in the same pre-reset bodyManualRestore capture so
restoreEntityState() can restore all original controls.
---
Outside diff comments:
In `@src/Mocap/MocapController.cpp`:
- Around line 913-922: Add a SentryReporter::addBreadcrumb call in the
cameraStartupTimer timeout handler after emitting errorOccurred and immediately
before stopPreview(), recording that preview stopped because no frames arrived
during camera startup. Keep the existing user-facing status message and shutdown
flow unchanged.
- Around line 772-805: Update the pose-model fallback cleanup near the
body-drive teardown, including the analogous path around the secondary location,
to restore every snapshot in bodyManualRestore rather than only canonical
bodyBones, and restore each saved entry in bodyAnimMaskRestore before clearing
bodyRetargeter. Preserve the existing head-bone setup and ensure non-canonical
bones and animation blend-mask values return to their captured states.
🪄 Autofix
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 Plus
Run ID: 7e6f8541-7fe8-49ae-9e29-2471adfa1157
📒 Files selected for processing (4)
qml/PropertiesPanel.qmlsrc/AnimationMerger.cppsrc/Mocap/MocapController.cppsrc/Mocap/MocapController.h
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Mocap/MocapController.h
- qml/PropertiesPanel.qml
- src/AnimationMerger.cpp
| BodyManualBoneSnapshot snap; | ||
| snap.boneName = bone->getName(); | ||
| snap.bindLocal = bone->getOrientation(); | ||
| snap.wasManuallyControlled = bone->isManuallyControlled(); | ||
| d->bodyManualRestore.push_back(std::move(snap)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect reset, snapshot, and restoration ordering.
rg -n -C 8 'skel->reset\(true\)|bodyManualRestore|headBindLocal|restoreEntityState' \
src/Mocap/MocapController.cpp
# Inspect vendored OGRE headers when present.
fd -a -t f 'OgreSkeleton*.h' . -E build -E .git |
xargs -r rg -n -C 4 'reset\s*\(\s*bool\s+resetManualBones'Repository: fernandotonon/QtMeshEditor
Length of output: 9396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map structure and nearby implementation details.
ast-grep outline src/Mocap/MocapController.cpp --match 'void MocapController::startMocapPreview' --view expanded || true
ast-grep outline src/Mocap/MocapController.cpp --match 'struct BodyManualBoneSnapshot' --view expanded || true
# Locate OGRE includes/usage around reset.
rg -n -C 12 '`#include` ["<].*OgreSkeleton.*["<]|reset\s*\(\s*true\s*\)|getSkeleton\(\s*\)|SKELETON_\w+|manualBone|_notifyManualBonesDirty|_getManualBone' src include third_party . --glob '!build' --glob '!build/*' --glob '!out' --glob '!*.pyc' | head -n 240
# Search any explicit OGRE reset documentation/comments in repository.
rg -n -C 5 'resetManualBones|manual bones|manualBone|manual bone|SKELETON_RESET_MANUAL_BONE|reset\(true\)' . --glob '!build' --glob '!build/*' --glob '!out' | head -n 240Repository: fernandotonon/QtMeshEditor
Length of output: 17650
🌐 Web query:
OGRE 14 SkeletonInstance reset resetManualBones manual bones binding pose documentation
💡 Result:
In OGRE 14, the SkeletonInstance class inherits the reset method from its base class, Ogre::Skeleton [1][2]. This method is used to return all bones in a skeleton to their original binding pose [1][2]. The reset method has the following signature: void Ogre::Skeleton::reset(bool resetManualBones = false) Parameters: resetManualBones: When set to false (the default), manual bones are excluded from the reset process, allowing their state to persist even when keyframe animation is applied [1][2]. If set to true, the state of manual bones is also reset to the binding pose [1][2]. Key Concepts: Binding Pose: This is the default state in which bones are bound to a mesh. Animations are stored as relative transformations (deltas) from this pose [3][2]. Manual Bones: These are bones designated for manual manipulation by the application at runtime [4][5]. You can set a bone as manually controlled using the Ogre::Bone::setManuallyControlled(bool) method [4][5]. Because manual bones are typically handled by custom code, Ogre's animation routines generally do not reset them during normal animation updates or standard resets, unless specifically requested via the resetManualBones parameter [1][4][2]. If you are using manual bones, ensure there are no AnimationTrack objects referencing them, or disable those tracks, to prevent conflicts between your manual adjustments and scripted animations [4][5].
Citations:
- 1: https://ogrecave.github.io/ogre/api/14/class_ogre_1_1_skeleton.html
- 2: https://ogrecave.github.io/ogre/api/latest/class_ogre_1_1_skeleton.html
- 3: https://ogrecave.github.io/ogre/api/14/class_ogre_1_1_skeleton_instance.html
- 4: https://ogrecave.github.io/ogre/api/14/class_ogre_1_1_bone.html
- 5: https://ogrecave.github.io/ogre/api/latest/class_ogre_1_1_bone.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant declarations and control-flow slices without running repository code.
rg -n -C 3 'struct|class BodyManualBoneSnapshot|struct BodyDriveBone|void MocapController::stopMocapPreview|MocapController::startMocapPreview|startMocapPreview\(' src/Mocap/MocapController.cpp
sed -n '130,190p' src/Mocap/MocapController.cpp
sed -n '680,800p' src/Mocap/MocapController.cpp
sed -n '1320,1365p' src/Mocap/MocapController.cpp
sed -n '1258,1288p' src/Mocap/MocapController.cppRepository: fernandotonon/QtMeshEditor
Length of output: 13221
Snapshot manual bones before calling skel->reset(true).
skel->reset(true) resets manual bones to their binding pose, so the bodyManualRestore snapshots record the reset pose instead of the pre-preview pose. restoreEntityState() cannot restore the original manual controls after preview stops. Capture manual bone state before resetting, then use the post-reset orientation for BodyDriveBone bind data. Include the head bone in that same pre-reset capture when the head is manually controlled.
🤖 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/Mocap/MocapController.cpp` around lines 775 - 779, Move the body
manual-bone snapshot logic around the preview reset so it captures each bone’s
pre-reset orientation and manual-control state before calling skel->reset(true).
After the reset, use the post-reset orientation when populating BodyDriveBone
bind data, and include the manually controlled head bone in the same pre-reset
bodyManualRestore capture so restoreEntityState() can restore all original
controls.
|
1 participant
Summary
applyMotionClip), with PoseIK debug overlay, per-frame skinning refresh, and removal of the mirror-L/R toggle.OnnxRuntimeSettingsQML singleton — prefer-GPU toggle in AI Settings, centralized session options for all ONNX predictors (UniRig, segmentation, mocap, PBR, etc.), optionalQTMESH_ONNX_GPUpackage on Linux x64, and provider.socopy rules so the app links and runs cleanly.-DQTMESH_ONNX_GPU=OFF;.debbundles Qt JPEG imageformat plugin (MJPEG webcams) alongside existing FFmpeg stubs.Test plan
./build_local/bin/UnitTests --gtest_filter="AnimationMergerTest.BodyRetargeter*"./build_local/bin/UnitTests --gtest_filter="OnnxRuntimeSettings*"cmake --build build_local --target QtMeshEditor -j4Follow-up
Summary by CodeRabbit