birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #3428 from aws-lumberyard-dev/cgalvan/FixedTrackViewNodeCrash

Browse Source 
Fixed memory stomping leading to Track View nodes with invalid labels and crash.
This commit is contained in:
Chris Galvan
2021-08-25 19:14:26 -05:00
committed by GitHub
parent 8d2383e408 4b672a0a32
commit 486157f89b
57 changed files with 144 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Editor/TrackView/TVSequenceProps.cpp
+2 -2
View File
@@ -53,7 +53,7 @@ CTVSequenceProps::~CTVSequenceProps()
// CTVSequenceProps message handlers
bool CTVSequenceProps::OnInitDialog()
{
ui->NAME->setText(m_pSequence->GetName());
ui->NAME->setText(m_pSequence->GetName().c_str());
int seqFlags = m_pSequence->GetFlags();
ui->ALWAYS_PLAY->setChecked((seqFlags & IAnimSequence::eSeqFlags_PlayOnReset));
@@ -141,7 +141,7 @@ void CTVSequenceProps::UpdateSequenceProps(const QString& name)
ac->UpdateTimeRange();
}
QString seqName = m_pSequence->GetName();
QString seqName = QString::fromUtf8(m_pSequence->GetName().c_str());
if (name != seqName)
{
// Rename sequence.
Code/Editor/TrackView/TrackViewAnimNode.cpp
+9 -10
View File
@@ -423,7 +423,7 @@ CTrackViewAnimNode* CTrackViewAnimNode::CreateSubNode(
AZStd::string::format(
"Failed to add '%s' to sequence '%s', could not find associated entity. "
"Please try adding the entity associated with '%s'.",
originalNameStr.constData(), director->GetName(), originalNameStr.constData()));
originalNameStr.constData(), director->GetName().c_str(), originalNameStr.constData()));
return nullptr;
}
@@ -472,7 +472,7 @@ CTrackViewAnimNode* CTrackViewAnimNode::CreateSubNode(
{
GetIEditor()->GetMovieSystem()->LogUserNotificationMsg(
AZStd::string::format("'%s' already exists in sequence '%s', skipping...",
originalNameStr.constData(), director2->GetName()));
originalNameStr.constData(), director2->GetName().c_str()));
return nullptr;
}
@@ -488,7 +488,7 @@ CTrackViewAnimNode* CTrackViewAnimNode::CreateSubNode(
if (!newAnimNode)
{
GetIEditor()->GetMovieSystem()->LogUserNotificationMsg(
AZStd::string::format("Failed to add '%s' to sequence '%s'.", nameStr.constData(), director->GetName()));
AZStd::string::format("Failed to add '%s' to sequence '%s'.", nameStr.constData(), director->GetName().c_str()));
return nullptr;
}
@@ -1195,7 +1195,7 @@ CTrackViewAnimNodeBundle CTrackViewAnimNode::GetAnimNodesByName(const char* pNam
{
CTrackViewAnimNodeBundle bundle;
QString nodeName = GetName();
QString nodeName = QString::fromUtf8(GetName().c_str());
if (GetNodeType() == eTVNT_AnimNode && QString::compare(pName, nodeName, Qt::CaseInsensitive) == 0)
{
bundle.AppendAnimNode(this);
@@ -1215,10 +1215,9 @@ CTrackViewAnimNodeBundle CTrackViewAnimNode::GetAnimNodesByName(const char* pNam
}
//////////////////////////////////////////////////////////////////////////
const char* CTrackViewAnimNode::GetParamName(const CAnimParamType& paramType) const
AZStd::string CTrackViewAnimNode::GetParamName(const CAnimParamType& paramType) const
{
const char* pName = m_animNode->GetParamName(paramType);
return pName ? pName : "";
return m_animNode->GetParamName(paramType);
}
//////////////////////////////////////////////////////////////////////////
@@ -1274,7 +1273,7 @@ CTrackViewAnimNodeBundle CTrackViewAnimNode::AddSelectedEntities(const AZStd::ve
if (existingNode->GetDirector() == GetDirector())
{
GetIEditor()->GetMovieSystem()->LogUserNotificationMsg(AZStd::string::format(
"'%s' was already added to '%s', skipping...", entity->GetName().c_str(), GetDirector()->GetName()));
"'%s' was already added to '%s', skipping...", entity->GetName().c_str(), GetDirector()->GetName().c_str()));
continue;
}
@@ -1377,7 +1376,7 @@ void CTrackViewAnimNode::UpdateDynamicParams()
void CTrackViewAnimNode::CopyKeysToClipboard(XmlNodeRef& xmlNode, const bool bOnlySelectedKeys, const bool bOnlyFromSelectedTracks)
{
XmlNodeRef childNode = xmlNode->createNode("Node");
childNode->setAttr("name", GetName());
childNode->setAttr("name", GetName().c_str());
childNode->setAttr("type", static_cast<int>(GetType()));
for (auto iter = m_childNodes.begin(); iter != m_childNodes.end(); ++iter)
@@ -1683,7 +1682,7 @@ bool CTrackViewAnimNode::IsValidReparentingTo(CTrackViewAnimNode* pNewParent)
}
// Check if the new parent already contains a node with this name
CTrackViewAnimNodeBundle foundNodes = pNewParent->GetAnimNodesByName(GetName());
CTrackViewAnimNodeBundle foundNodes = pNewParent->GetAnimNodesByName(GetName().c_str());
if (foundNodes.GetCount() > 1 || (foundNodes.GetCount() == 1 && foundNodes.GetNode(0) != this))
{
return false;
Code/Editor/TrackView/TrackViewAnimNode.h
+2 -2
View File
@@ -124,7 +124,7 @@ public:
virtual void SetAsViewCamera();
// Name setter/getter
virtual const char* GetName() const override { return m_animNode->GetName(); }
AZStd::string GetName() const override { return m_animNode->GetName(); }
virtual bool SetName(const char* pName) override;
virtual bool CanBeRenamed() const override;
@@ -187,7 +187,7 @@ public:
// Param
unsigned int GetParamCount() const;
CAnimParamType GetParamType(unsigned int index) const;
const char* GetParamName(const CAnimParamType& paramType) const;
AZStd::string GetParamName(const CAnimParamType& paramType) const;
bool IsParamValid(const CAnimParamType& param) const;
IAnimNode::ESupportedParamFlags GetParamFlags(const CAnimParamType& paramType) const;
AnimValueType GetParamValueType(const CAnimParamType& paramType) const;
Code/Editor/TrackView/TrackViewDialog.cpp
+3 -3
View File
@@ -1125,7 +1125,7 @@ void CTrackViewDialog::ReloadSequencesComboBox()
{
CTrackViewSequence* sequence = pSequenceManager->GetSequenceByIndex(k);
QString entityIdString = GetEntityIdAsString(sequence->GetSequenceComponentEntityId());
m_sequencesComboBox->addItem(sequence->GetName(), entityIdString);
m_sequencesComboBox->addItem(QString::fromUtf8(sequence->GetName().c_str()), entityIdString);
}
}
@@ -2033,7 +2033,7 @@ void CTrackViewDialog::UpdateTracksToolBar()
continue;
}
name = pAnimNode->GetParamName(paramType);
name = QString::fromUtf8(pAnimNode->GetParamName(paramType).c_str());
QString sToolTipText("Add " + name + " Track");
QIcon hIcon = m_wndNodesCtrl->GetIconForTrack(pTrack);
@@ -2309,7 +2309,7 @@ void CTrackViewDialog::SaveCurrentSequenceToFBX()
return;
}
QString selectedSequenceFBXStr = QString(sequence->GetName()) + ".fbx";
QString selectedSequenceFBXStr = QString::fromUtf8(sequence->GetName().c_str()) + ".fbx";
CExportManager* pExportManager = static_cast<CExportManager*>(GetIEditor()->GetExportManager());
const char szFilters[] = "FBX Files (*.fbx)";
Code/Editor/TrackView/TrackViewDopeSheetBase.cpp
+1 -1
View File
@@ -3453,7 +3453,7 @@ void CTrackViewDopeSheetBase::DrawNodeTrack(CTrackViewAnimNode* animNode, QPaint
const QRect textRect = trackRect.adjusted(4, 0, -4, 0);
QString sAnimNodeName = animNode->GetName();
QString sAnimNodeName = QString::fromUtf8(animNode->GetName().c_str());
const bool hasObsoleteTrack = animNode->HasObsoleteTrack();
if (hasObsoleteTrack)
Code/Editor/TrackView/TrackViewNode.cpp
+2 -2
View File
@@ -626,7 +626,7 @@ bool CTrackViewNode::operator<(const CTrackViewNode& otherNode) const
if (thisTypeOrder == otherTypeOrder)
{
// Same node type, sort by name
return azstricmp(thisAnimNode.GetName(), otherAnimNode.GetName()) < 0;
return thisAnimNode.GetName() < otherAnimNode.GetName();
}
return thisTypeOrder < otherTypeOrder;
@@ -638,7 +638,7 @@ bool CTrackViewNode::operator<(const CTrackViewNode& otherNode) const
if (thisTrack.GetParameterType() == otherTrack.GetParameterType())
{
// Same parameter type, sort by name
return azstricmp(thisTrack.GetName(), otherTrack.GetName()) < 0;
return thisTrack.GetName() < otherTrack.GetName();
}
return thisTrack.GetParameterType() < otherTrack.GetParameterType();
Code/Editor/TrackView/TrackViewNode.h
+1 -1
View File
@@ -159,7 +159,7 @@ public:
virtual ~CTrackViewNode() {}
// Name
virtual const char* GetName() const = 0;
virtual AZStd::string GetName() const = 0;
virtual bool SetName([[maybe_unused]] const char* pName) { return false; };
virtual bool CanBeRenamed() const { return false; }
Code/Editor/TrackView/TrackViewNodes.cpp
+21 -22
View File
@@ -616,7 +616,7 @@ CTrackViewNodesCtrl::CRecord* CTrackViewNodesCtrl::AddAnimNodeRecord(CRecord* pP
{
CRecord* pNewRecord = new CRecord(animNode);
pNewRecord->setText(0, animNode->GetName());
pNewRecord->setText(0, QString::fromUtf8(animNode->GetName().c_str()));
UpdateAnimNodeRecord(pNewRecord, animNode);
pParentRecord->insertChild(GetInsertPosition(pParentRecord, animNode), pNewRecord);
FillNodesRec(pNewRecord, animNode);
@@ -629,7 +629,7 @@ CTrackViewNodesCtrl::CRecord* CTrackViewNodesCtrl::AddTrackRecord(CRecord* pPare
{
CRecord* pNewTrackRecord = new CRecord(pTrack);
pNewTrackRecord->setSizeHint(0, QSize(30, 18));
pNewTrackRecord->setText(0, pTrack->GetName());
pNewTrackRecord->setText(0, QString::fromUtf8(pTrack->GetName().c_str()));
UpdateTrackRecord(pNewTrackRecord, pTrack);
pParentRecord->insertChild(GetInsertPosition(pParentRecord, pTrack), pNewTrackRecord);
FillNodesRec(pNewTrackRecord, pTrack);
@@ -860,7 +860,7 @@ void CTrackViewNodesCtrl::OnFillItems()
m_nodeToRecordMap.clear();
CRecord* pRootGroupRec = new CRecord(sequence);
pRootGroupRec->setText(0, sequence->GetName());
pRootGroupRec->setText(0, QString::fromUtf8(sequence->GetName().c_str()));
QFont f = font();
f.setBold(true);
pRootGroupRec->setData(0, Qt::FontRole, f);
@@ -1032,8 +1032,8 @@ void CTrackViewNodesCtrl::OnNMRclick(QPoint point)
return;
}
QString file = QString(sequence2->GetName()) + QString(".fbx");
QString selectedSequenceFBXStr = QString(sequence2->GetName()) + ".fbx";
QString file = QString::fromUtf8(sequence2->GetName().c_str()) + QString(".fbx");
QString selectedSequenceFBXStr = QString::fromUtf8(sequence2->GetName().c_str()) + ".fbx";
if (numSelectedNodes > 1)
{
@@ -1041,7 +1041,7 @@ void CTrackViewNodesCtrl::OnNMRclick(QPoint point)
}
else
{
file = QString(selectedNodes.GetNode(0)->GetName()) + QString(".fbx");
file = QString::fromUtf8(selectedNodes.GetNode(0)->GetName().c_str()) + QString(".fbx");
}
QString path = QFileDialog::getSaveFileName(this, tr("Export Selected Nodes To FBX File"), QString(), tr("FBX Files (*.fbx)"));
@@ -1338,7 +1338,7 @@ void CTrackViewNodesCtrl::OnNMRclick(QPoint point)
if (animNode || groupNode)
{
CTrackViewAnimNode* animNode2 = static_cast<CTrackViewAnimNode*>(pNode);
QString oldName = animNode2->GetName();
QString oldName = QString::fromUtf8(animNode2->GetName().c_str());
StringDlg dlg(tr("Rename Node"));
dlg.SetString(oldName);
@@ -1494,7 +1494,7 @@ void CTrackViewNodesCtrl::OnNMRclick(QPoint point)
if (animNode)
{
QString matName;
GetMatNameAndSubMtlIndexFromName(matName, animNode->GetName());
GetMatNameAndSubMtlIndexFromName(matName, animNode->GetName().c_str());
QString newMatName;
newMatName = tr("%1.[%2]").arg(matName).arg(cmd - eMI_SelectSubmaterialBase + 1);
CUndo undo("Rename TrackView node");
@@ -1576,7 +1576,7 @@ CTrackViewTrack* CTrackViewNodesCtrl::GetTrackViewTrack(const Export::EntityAnim
for (unsigned int trackID = 0; trackID < trackBundle.GetCount(); ++trackID)
{
CTrackViewTrack* pTrack = trackBundle.GetTrack(trackID);
const QString bundleTrackName = pTrack->GetAnimNode()->GetName();
const QString bundleTrackName = QString::fromUtf8(pTrack->GetAnimNode()->GetName().c_str());
if (bundleTrackName.compare(nodeName, Qt::CaseInsensitive) != 0)
{
@@ -2164,7 +2164,7 @@ int CTrackViewNodesCtrl::ShowPopupMenuSingleSelection(SContextMenu& contextMenu,
if (bOnNode && !pNode->IsGroupNode())
{
AddMenuSeperatorConditional(contextMenu.main, bAppended);
QString string = QString("%1 Tracks").arg(animNode->GetName());
QString string = QString("%1 Tracks").arg(animNode->GetName().c_str());
contextMenu.main.addAction(string)->setEnabled(false);
bool bAppendedTrackFlag = false;
@@ -2182,7 +2182,7 @@ int CTrackViewNodesCtrl::ShowPopupMenuSingleSelection(SContextMenu& contextMenu,
continue;
}
QAction* a = contextMenu.main.addAction(QString(" %1").arg(pTrack2->GetName()));
QAction* a = contextMenu.main.addAction(QString(" %1").arg(pTrack2->GetName().c_str()));
a->setData(eMI_ShowHideBase + childIndex);
a->setCheckable(true);
a->setChecked(!pTrack2->IsHidden());
@@ -2348,13 +2348,12 @@ bool CTrackViewNodesCtrl::FillAddTrackMenu(STrackMenuTreeNode& menuAddTrack, con
continue;
}
}
name = animNode->GetParamName(paramType);
QStringList splittedName = name.split("/", Qt::SkipEmptyParts);
name = QString::fromUtf8(animNode->GetParamName(paramType).c_str());
QStringList splitName = name.split("/", Qt::SkipEmptyParts);
STrackMenuTreeNode* pCurrentNode = &menuAddTrack;
for (int j = 0; j < splittedName.size() - 1; ++j)
for (const QString& segment : splitName)
{
const QString& segment = splittedName[j];
auto findIter = pCurrentNode->children.find(segment);
if (findIter != pCurrentNode->children.end())
{
@@ -2370,10 +2369,10 @@ bool CTrackViewNodesCtrl::FillAddTrackMenu(STrackMenuTreeNode& menuAddTrack, con
// only add tracks to the that STrackMenuTreeNode tree that haven't already been added
CTrackViewTrackBundle matchedTracks = animNode->GetTracksByParam(paramType);
if (matchedTracks.GetCount() == 0)
if (matchedTracks.GetCount() == 0 && !splitName.isEmpty())
{
STrackMenuTreeNode* pParamNode = new STrackMenuTreeNode;
pCurrentNode->children[splittedName.back()] = std::unique_ptr<STrackMenuTreeNode>(pParamNode);
pCurrentNode->children[splitName.back()] = std::unique_ptr<STrackMenuTreeNode>(pParamNode);
pParamNode->paramType = paramType;
bTracksToAdd = true;
@@ -2464,7 +2463,7 @@ void CTrackViewNodesCtrl::FillAutoCompletionListForFilter()
for (unsigned int i = 0; i < animNodeCount; ++i)
{
strings << animNodes.GetNode(i)->GetName();
strings << QString::fromUtf8(animNodes.GetNode(i)->GetName().c_str());
}
}
else
@@ -2580,10 +2579,10 @@ void CTrackViewNodesCtrl::Update()
{
const CTrackViewAnimNode* track = static_cast<const CTrackViewAnimNode*>(node);
if (track)
{
record->setText(0, track->GetName());
{
record->setText(0, QString::fromUtf8(track->GetName().c_str()));
}
}
}
}
}
}
@@ -2855,7 +2854,7 @@ void CTrackViewNodesCtrl::OnNodeRenamed(CTrackViewNode* pNode, [[maybe_unused]]
if (!m_bIgnoreNotifications)
{
CRecord* pNodeRecord = GetNodeRecord(pNode);
pNodeRecord->setText(0, pNode->GetName());
pNodeRecord->setText(0, QString::fromUtf8(pNode->GetName().c_str()));
update();
}
Code/Editor/TrackView/TrackViewNodes.h
+1 -1
View File
@@ -66,7 +66,7 @@ public:
CRecord(CTrackViewNode* pNode = nullptr);
CTrackViewNode* GetNode() const { return m_pNode; }
bool IsGroup() const { return m_pNode->GetChildCount() != 0; }
const QString GetName() const { return m_pNode->GetName(); }
const QString GetName() const { return QString::fromUtf8(m_pNode->GetName().c_str()); }
// Workaround: CXTPReportRecord::IsVisible is
// unreliable after the last visible element
Code/Editor/TrackView/TrackViewPythonFuncs.cpp
+2 -2
View File
@@ -293,8 +293,8 @@ namespace
CTrackViewTrack* pTrack = pNode->GetTrackForParameter(paramType);
if (!pTrack || (paramFlags & IAnimNode::eSupportedParamFlags_MultipleTracks))
{
const char* name = pNode->GetParamName(paramType);
if (_stricmp(name, paramName) == 0)
AZStd::string name = pNode->GetParamName(paramType);
if (name == paramName)
{
CUndo undo("Create track");
if (!pNode->CreateTrack(paramType))
Code/Editor/TrackView/TrackViewSequence.cpp
+3 -3
View File
@@ -894,14 +894,14 @@ bool CTrackViewSequence::SetName(const char* name)
return false;
}
const char* oldName = GetName();
if (0 != strcmp(name, oldName))
AZStd::string oldName = GetName();
if (name != oldName)
{
m_pAnimSequence->SetName(name);
MarkAsModified();
AzToolsFramework::ScopedUndoBatch undoBatch("Rename Sequence");
GetSequence()->OnNodeRenamed(this, oldName);
GetSequence()->OnNodeRenamed(this, oldName.c_str());
undoBatch.MarkEntityDirty(m_pAnimSequence->GetSequenceEntityId());
}
Code/Editor/TrackView/TrackViewSequence.h
+1 -1
View File
@@ -102,7 +102,7 @@ public:
// ITrackViewNode
virtual ETrackViewNodeType GetNodeType() const override { return eTVNT_Sequence; }
virtual const char* GetName() const override { return m_pAnimSequence->GetName(); }
virtual AZStd::string GetName() const override { return m_pAnimSequence->GetName(); }
virtual bool SetName(const char* pName) override;
virtual bool CanBeRenamed() const override { return true; }
Code/Editor/TrackView/TrackViewSequenceManager.cpp
+3 -3
View File
@@ -75,7 +75,7 @@ CTrackViewSequence* CTrackViewSequenceManager::GetSequenceByName(QString name) c
{
CTrackViewSequence* sequence = (*iter).get();
if (sequence->GetName() == name)
if (QString::fromUtf8(sequence->GetName().c_str()) == name)
{
return sequence;
}
@@ -371,8 +371,8 @@ void CTrackViewSequenceManager::SortSequences()
std::stable_sort(m_sequences.begin(), m_sequences.end(),
[](const std::unique_ptr<CTrackViewSequence>& a, const std::unique_ptr<CTrackViewSequence>& b) -> bool
{
QString aName = a.get()->GetName();
QString bName = b.get()->GetName();
QString aName = QString::fromUtf8(a.get()->GetName().c_str());
QString bName = QString::fromUtf8(b.get()->GetName().c_str());
return aName < bName;
});
}
Code/Editor/TrackView/TrackViewTrack.cpp
+2 -2
View File
@@ -472,7 +472,7 @@ void CTrackViewTrack::RestoreFromMemento(const CTrackViewTrackMemento& memento)
}
//////////////////////////////////////////////////////////////////////////
const char* CTrackViewTrack::GetName() const
AZStd::string CTrackViewTrack::GetName() const
{
CTrackViewNode* pParentNode = GetParentNode();
@@ -810,7 +810,7 @@ void CTrackViewTrack::CopyKeysToClipboard(XmlNodeRef& xmlNode, const bool bOnlyS
}
XmlNodeRef childNode = xmlNode->newChild("Track");
childNode->setAttr("name", GetName());
childNode->setAttr("name", GetName().c_str());
GetParameterType().SaveToXml(childNode);
childNode->setAttr("valueType", static_cast<int>(GetValueType()));
Code/Editor/TrackView/TrackViewTrack.h
+1 -1
View File
@@ -80,7 +80,7 @@ public:
CTrackViewAnimNode* GetAnimNode() const;
// Name getter
virtual const char* GetName() const;
AZStd::string GetName() const override;
// CTrackViewNode
virtual ETrackViewNodeType GetNodeType() const override { return eTVNT_Track; }
Code/Editor/TrackView/TrackViewUndo.cpp
+1 -1
View File
@@ -75,7 +75,7 @@ CTrackViewTrack* CUndoComponentEntityTrackObject::FindTrack(CTrackViewSequence*
CTrackViewTrack* curTrack = allTracks.GetTrack(trackIndex);
if (curTrack->GetAnimNode() && curTrack->GetAnimNode()->GetComponentId() == m_trackComponentId)
{
if (0 == azstricmp(curTrack->GetName(), m_trackName.c_str()))
if (curTrack->GetName() == m_trackName)
{
CTrackViewAnimNode* parentAnimNode = static_cast<CTrackViewAnimNode*>(curTrack->GetAnimNode()->GetParentNode());
if (parentAnimNode && parentAnimNode->GetAzEntityId() == m_entityId)