Curve editor: fix repeated Apply after bake
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 <noreply@anthropic.com>
This commit is contained in:
+60
-37
@@ -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<BezierKey>& keys) const
|
||||
{
|
||||
if (!prim || ch.keys.empty()) return false;
|
||||
if (!prim || keys.empty()) return false;
|
||||
|
||||
// Build flat arrays from keys
|
||||
VtArray<double> times, values, inDt, inDv, outDt, outDv;
|
||||
VtArray<int> 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<VtDictionary>())
|
||||
ceDict = existing.UncheckedGet<VtDictionary>();
|
||||
|
||||
// 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<VtDictionary>();
|
||||
}
|
||||
|
||||
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<AttributeSetCommand>(
|
||||
"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;
|
||||
}
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
@@ -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<BezierKey>& keys) const;
|
||||
bool LoadBezierFromMetadata(const pxr::UsdPrim& prim, CurveChannel& ch) const;
|
||||
|
||||
// ── State ────────────────────────────────────────────────────────────────
|
||||
@@ -116,7 +118,8 @@ private:
|
||||
std::vector<SelectedHandle> 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;
|
||||
|
||||
Reference in New Issue
Block a user