From ddffa5764ccffd1de9094904aacd4b22c96c0ffe Mon Sep 17 00:00:00 2001 From: indigo Date: Wed, 24 Jun 2026 09:28:09 +0800 Subject: [PATCH] Curve editor: fix repeated Apply after bake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs prevented Apply from working correctly after the first bake: 1. m_needsRefresh spuriously set during bake ClearAllTimeSamples/WriteChannelValue fire OnObjectsChanged synchronously. Since the channel is dirty at that moment, it was skipped, setting anyMatchedChannel=false and triggering m_needsRefresh=true. This caused RefreshFromStage() on the next frame, clearing m_selection and rebuilding m_channels — dropping the user's edit context. Fixed with m_suppressNotice: set true during USD writes so OnObjectsChanged is a no-op during bake. 2. Prim-level metadata notice triggered rebuild SaveBezierToMetadata calls SetCustomDataByKey, which fires a notice with changed==primPath. No attribute channel matched, so m_needsRefresh=true again. Fixed in OnObjectsChanged: skip changed==primPath (metadata-only changes), and mark a path as "known" even if the channel is dirty (so the refresh gate only fires for genuinely new/unknown attributes). 3. Dangling chPtr in undo/redo closures After RefreshFromStage() cleared and rebuilt m_channels, the raw pointer captured in closures pointed to freed memory. Replaced with a findCh lambda that searches m_channels by attr path + component at call time. Falls back to m_needsRefresh=true if the channel no longer exists. Co-Authored-By: Claude Sonnet 4.6 --- src/ui/CurveEditorPanel.cpp | 97 +++++++++++++++++++++++-------------- src/ui/CurveEditorPanel.h | 5 +- 2 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/ui/CurveEditorPanel.cpp b/src/ui/CurveEditorPanel.cpp index c39410c..5ab4c29 100644 --- a/src/ui/CurveEditorPanel.cpp +++ b/src/ui/CurveEditorPanel.cpp @@ -168,6 +168,7 @@ void CurveEditorPanel::OnObjectsChanged( const UsdNotice::ObjectsChanged& notice, const UsdStageWeakPtr& /*sender*/) { + if (m_suppressNotice) return; if (m_selectedPrimPath.empty()) return; SdfPath primPath(m_selectedPrimPath); @@ -179,26 +180,24 @@ void CurveEditorPanel::OnObjectsChanged( } } - // Value-only change → re-fit individual channels that were affected and are clean - bool anyMatchedChannel = false; + // Value-only change → re-fit clean channels; ignore prim-level metadata changes for (const SdfPath& changed : notice.GetChangedInfoOnlyPaths()) { - // changed is an attribute path like /Prim.attrName - // or a prim path /Prim when multiple attrs changed at once - bool belongsToSelectedPrim = - (changed.GetPrimPath() == primPath) || (changed == primPath); - if (!belongsToSelectedPrim) continue; + // Prim-level path means a metadata change (e.g. customData) — not an attr write + if (changed == primPath) continue; + if (changed.GetPrimPath() != primPath) continue; + + bool knownChannel = false; for (auto& ch : m_channels) { - if (ch.dirty) continue; // preserve unsaved edits - if (changed == ch.attr.GetPath() || changed == primPath) { - FitCurveFromSamples(ch); - anyMatchedChannel = true; + if (changed == ch.attr.GetPath()) { + knownChannel = true; + if (!ch.dirty) FitCurveFromSamples(ch); // preserve unsaved edits + break; } } - // A change on the selected prim that didn't match any known channel - // may mean a new animated attribute was added → full refresh - if (!anyMatchedChannel) + // Changed attribute not in our list → may be a newly animated attr → full refresh + if (!knownChannel) m_needsRefresh = true; } } @@ -311,15 +310,15 @@ std::string CurveEditorPanel::ChannelMetadataKey(const CurveChannel& ch) } bool CurveEditorPanel::SaveBezierToMetadata(const UsdPrim& prim, - const CurveChannel& ch) const + const std::string& channelKey, + const std::vector& keys) const { - if (!prim || ch.keys.empty()) return false; + if (!prim || keys.empty()) return false; - // Build flat arrays from keys VtArray times, values, inDt, inDv, outDt, outDv; VtArray broken; - times.reserve(ch.keys.size()); - for (const BezierKey& k : ch.keys) { + times.reserve(keys.size()); + for (const BezierKey& k : keys) { times .push_back(k.time); values.push_back(k.value); inDt .push_back(k.inTangentDt); @@ -338,13 +337,11 @@ bool CurveEditorPanel::SaveBezierToMetadata(const UsdPrim& prim, keyData["outDv"] = VtValue(outDv); keyData["broken"] = VtValue(broken); - // Read existing curveEditor dict VtDictionary ceDict; VtValue existing = prim.GetCustomDataByKey(TfToken("curveEditor")); if (existing.IsHolding()) ceDict = existing.UncheckedGet(); - // Read / create channels sub-dict VtDictionary channels; { auto it = ceDict.find("channels"); @@ -352,12 +349,18 @@ bool CurveEditorPanel::SaveBezierToMetadata(const UsdPrim& prim, channels = it->second.UncheckedGet(); } - channels[ChannelMetadataKey(ch)] = VtValue(keyData); + channels[channelKey] = VtValue(keyData); ceDict["channels"] = VtValue(channels); prim.SetCustomDataByKey(TfToken("curveEditor"), VtValue(ceDict)); return true; } +bool CurveEditorPanel::SaveBezierToMetadata(const UsdPrim& prim, + const CurveChannel& ch) const +{ + return SaveBezierToMetadata(prim, ChannelMetadataKey(ch), ch.keys); +} + bool CurveEditorPanel::LoadBezierFromMetadata(const UsdPrim& prim, CurveChannel& ch) const { @@ -611,52 +614,72 @@ void CurveEditorPanel::BakeChannelToUsd(CurveChannel& ch) UsdAttribute attr = ch.attr; int comp = ch.component; - // Snapshot current Bezier keys and old prim metadata for undo auto newKeys = ch.keys; + std::string channelKey = ChannelMetadataKey(ch); UsdPrim prim = m_stage ? m_stage->GetPrimAtPath(SdfPath(m_selectedPrimPath)) : UsdPrim{}; VtValue oldCEMeta; if (prim) oldCEMeta = prim.GetCustomDataByKey(TfToken("curveEditor")); + // Safe channel finder: looks up by attr path + component at closure call time, + // so it remains valid even if m_channels is rebuilt between bake and undo. + SdfPath chanAttrPath = attr.GetPath(); + auto findCh = [this, chanAttrPath, comp]() -> CurveChannel* { + for (auto& c : m_channels) + if (c.attr.GetPath() == chanAttrPath && c.component == comp) return &c; + return nullptr; + }; + if (!m_commandHistory) { + m_suppressNotice = true; ClearAllTimeSamples(attr); for (int i = 0; i < int(newTimes.size()); ++i) WriteChannelValue(attr, comp, newTimes[i], newValues[i]); + m_suppressNotice = false; ch.origTimes = newTimes; ch.origValues = newValues; ch.dirty = false; - if (prim) SaveBezierToMetadata(prim, ch); + if (prim) SaveBezierToMetadata(prim, channelKey, newKeys); return; } - // Capture for closure - CurveChannel* chPtr = &ch; m_commandHistory->Push(std::make_unique( "Bake F-Curve: " + ch.displayName, - [this, attr, comp, newTimes, newValues, newKeys, prim, chPtr]() { + [this, attr, comp, newTimes, newValues, newKeys, channelKey, prim, findCh]() { + m_suppressNotice = true; ClearAllTimeSamples(attr); for (int i = 0; i < int(newTimes.size()); ++i) WriteChannelValue(attr, comp, newTimes[i], newValues[i]); - chPtr->origTimes = newTimes; - chPtr->origValues = newValues; - chPtr->keys = newKeys; - chPtr->dirty = false; - if (prim) SaveBezierToMetadata(prim, *chPtr); + m_suppressNotice = false; + if (prim) SaveBezierToMetadata(prim, channelKey, newKeys); + if (auto* c = findCh()) { + c->origTimes = newTimes; + c->origValues = newValues; + c->keys = newKeys; + c->dirty = false; + } else { + m_needsRefresh = true; + } }, - [this, attr, comp, oldTimes, oldValues, oldCEMeta, prim, chPtr]() { + [this, attr, comp, oldTimes, oldValues, oldCEMeta, prim, findCh]() { + m_suppressNotice = true; ClearAllTimeSamples(attr); for (int i = 0; i < int(oldTimes.size()); ++i) WriteChannelValue(attr, comp, oldTimes[i], oldValues[i]); - // Restore metadata snapshot + m_suppressNotice = false; if (prim) { if (oldCEMeta.IsEmpty()) prim.ClearCustomDataByKey(TfToken("curveEditor")); else prim.SetCustomDataByKey(TfToken("curveEditor"), oldCEMeta); } - chPtr->origTimes = oldTimes; - chPtr->origValues = oldValues; - FitCurveFromSamples(*chPtr); + if (auto* c = findCh()) { + c->origTimes = oldTimes; + c->origValues = oldValues; + FitCurveFromSamples(*c); + } else { + m_needsRefresh = true; + } } )); } diff --git a/src/ui/CurveEditorPanel.h b/src/ui/CurveEditorPanel.h index a1e1b81..56f5cd8 100644 --- a/src/ui/CurveEditorPanel.h +++ b/src/ui/CurveEditorPanel.h @@ -102,6 +102,8 @@ private: // Bezier metadata persistence (stored in prim customData["curveEditor"]) static std::string ChannelMetadataKey(const CurveChannel& ch); bool SaveBezierToMetadata(const pxr::UsdPrim& prim, const CurveChannel& ch) const; + bool SaveBezierToMetadata(const pxr::UsdPrim& prim, const std::string& channelKey, + const std::vector& keys) const; bool LoadBezierFromMetadata(const pxr::UsdPrim& prim, CurveChannel& ch) const; // ── State ──────────────────────────────────────────────────────────────── @@ -116,7 +118,8 @@ private: std::vector m_selection; int m_activeChannel = -1; // for double-click-to-add - bool m_needsRefresh = true; + bool m_needsRefresh = true; + bool m_suppressNotice = false; // set during bake writes to silence OnObjectsChanged // USD change notification pxr::TfNotice::Key m_stageChangeKey;