From 614fd3a0f9ffedc2425190537e1e89ac9e373916 Mon Sep 17 00:00:00 2001 From: Roman Telezhynskyi Date: Wed, 18 Oct 2017 16:40:20 +0300 Subject: [PATCH] Improve Delete piece undocommand. Performance improvement. --HG-- branch : develop --- src/libs/vtools/tools/vabstracttool.cpp | 49 +++++-- src/libs/vtools/tools/vabstracttool.h | 3 + src/libs/vtools/tools/vtoolseamallowance.cpp | 136 +++++++++++-------- src/libs/vtools/tools/vtoolseamallowance.h | 3 + src/libs/vtools/tools/vtooluniondetails.cpp | 3 - src/libs/vtools/undocommands/deletepiece.cpp | 49 +++++-- src/libs/vtools/undocommands/deletepiece.h | 13 +- 7 files changed, 173 insertions(+), 83 deletions(-) diff --git a/src/libs/vtools/tools/vabstracttool.cpp b/src/libs/vtools/tools/vabstracttool.cpp index 7ef23a839..ad97f153c 100644 --- a/src/libs/vtools/tools/vabstracttool.cpp +++ b/src/libs/vtools/tools/vabstracttool.cpp @@ -423,16 +423,37 @@ void VAbstractTool::ToolCreation(const Source &typeCreation) } //--------------------------------------------------------------------------------------------------------------------- -/** - * @brief AddRecord add record about tool in history. - * @param id object id in container - * @param toolType tool type - * @param doc dom document container - */ -void VAbstractTool::AddRecord(const quint32 id, const Tool &toolType, VAbstractPattern *doc) +VToolRecord VAbstractTool::GetRecord(const quint32 id, const Tool &toolType, VAbstractPattern *doc) +{ + QVector *history = doc->getHistory(); + for(int i = 0; i < history->size(); ++i) + { + if (history->at(i).getId() == id && history->at(i).getTypeTool() == toolType) + { + return history->at(i); + } + } + return VToolRecord(); +} + +//--------------------------------------------------------------------------------------------------------------------- +void VAbstractTool::RemoveRecord(const VToolRecord &record, VAbstractPattern *doc) +{ + QVector *history = doc->getHistory(); + for(int i = 0; i < history->size(); ++i) + { + if (history->at(i) == record) + { + history->remove(i); + return; + } + } +} + +//--------------------------------------------------------------------------------------------------------------------- +void VAbstractTool::AddRecord(const VToolRecord &record, VAbstractPattern *doc) { QVector *history = doc->getHistory(); - VToolRecord record = VToolRecord(id, toolType, doc->GetNameActivPP()); if (history->contains(record)) { return; @@ -459,6 +480,18 @@ void VAbstractTool::AddRecord(const quint32 id, const Tool &toolType, VAbstractP } } +//--------------------------------------------------------------------------------------------------------------------- +/** + * @brief AddRecord add record about tool in history. + * @param id object id in container + * @param toolType tool type + * @param doc dom document container + */ +void VAbstractTool::AddRecord(const quint32 id, const Tool &toolType, VAbstractPattern *doc) +{ + AddRecord(VToolRecord(id, toolType, doc->GetNameActivPP()), doc); +} + //--------------------------------------------------------------------------------------------------------------------- void VAbstractTool::AddNodes(VAbstractPattern *doc, QDomElement &domElement, const VPiecePath &path) { diff --git a/src/libs/vtools/tools/vabstracttool.h b/src/libs/vtools/tools/vabstracttool.h index 2aefe5396..0757083bb 100644 --- a/src/libs/vtools/tools/vabstracttool.h +++ b/src/libs/vtools/tools/vabstracttool.h @@ -96,6 +96,9 @@ public: static const QStringList Colors(); static QMap ColorsList(); + static VToolRecord GetRecord(const quint32 id, const Tool &toolType, VAbstractPattern *doc); + static void RemoveRecord(const VToolRecord &record, VAbstractPattern *doc); + static void AddRecord(const VToolRecord &record, VAbstractPattern *doc); static void AddRecord(const quint32 id, const Tool &toolType, VAbstractPattern *doc); static void AddNodes(VAbstractPattern *doc, QDomElement &domElement, const VPiecePath &path); static void AddNodes(VAbstractPattern *doc, QDomElement &domElement, const VPiece &piece); diff --git a/src/libs/vtools/tools/vtoolseamallowance.cpp b/src/libs/vtools/tools/vtoolseamallowance.cpp index a46d87d25..188b2454e 100644 --- a/src/libs/vtools/tools/vtoolseamallowance.cpp +++ b/src/libs/vtools/tools/vtoolseamallowance.cpp @@ -146,11 +146,6 @@ VToolSeamAllowance *VToolSeamAllowance::Create(VToolSeamAllowanceInitData &initD VAbstractTool::AddRecord(initData.id, Tool::Piece, initData.doc); piece = new VToolSeamAllowance(initData); initData.scene->addItem(piece); - connect(piece, &VToolSeamAllowance::ChoosedTool, initData.scene, &VMainGraphicsScene::ChoosedItem); - connect(initData.scene, &VMainGraphicsScene::EnableDetailItemHover, piece, &VToolSeamAllowance::AllowHover); - connect(initData.scene, &VMainGraphicsScene::EnableDetailItemSelection, piece, - &VToolSeamAllowance::AllowSelecting); - connect(initData.scene, &VMainGraphicsScene::HighlightDetail, piece, &VToolSeamAllowance::Highlight); VAbstractPattern::AddTool(initData.id, piece); } //Very important to delete it. Only this tool need this special variable. @@ -461,9 +456,68 @@ void VToolSeamAllowance::Update(const VPiece &piece) setFlag(QGraphicsItem::ItemSendsGeometryChanges, false); VAbstractTool::data.UpdatePiece(m_id, piece); RefreshGeometry(); + VMainGraphicsView::NewSceneRect(m_sceneDetails, qApp->getSceneView()); setFlag(QGraphicsItem::ItemSendsGeometryChanges, true); } +//--------------------------------------------------------------------------------------------------------------------- +void VToolSeamAllowance::DisconnectOutsideSignals() +{ + // If UnionDetails tool delete the detail this object will be deleted only after full parse. + // Deleting inside UnionDetails cause crash. + // Because this object should be inactive from no one we disconnect all signals that may cause a crash + // KEEP THIS LIST ACTUALL!!! + disconnect(doc, nullptr, this, nullptr); + if (QGraphicsScene *toolScene = scene()) + { + disconnect(toolScene, nullptr, this, nullptr); + } + disconnect(m_dataLabel, nullptr, this, nullptr); + disconnect(m_patternInfo, nullptr, this, nullptr); + disconnect(m_grainLine, nullptr, this, nullptr); + disconnect(m_sceneDetails, nullptr, this, nullptr); +} + +//--------------------------------------------------------------------------------------------------------------------- +void VToolSeamAllowance::ConnectOutsideSignals() +{ + connect(m_dataLabel, &VTextGraphicsItem::SignalMoved, this, &VToolSeamAllowance::SaveMoveDetail); + connect(m_dataLabel, &VTextGraphicsItem::SignalResized, this, &VToolSeamAllowance::SaveResizeDetail); + connect(m_dataLabel, &VTextGraphicsItem::SignalRotated, this, &VToolSeamAllowance::SaveRotationDetail); + + connect(m_patternInfo, &VTextGraphicsItem::SignalMoved, this, &VToolSeamAllowance::SaveMovePattern); + connect(m_patternInfo, &VTextGraphicsItem::SignalResized, this, &VToolSeamAllowance::SaveResizePattern); + connect(m_patternInfo, &VTextGraphicsItem::SignalRotated, this, &VToolSeamAllowance::SaveRotationPattern); + + connect(m_grainLine, &VGrainlineItem::SignalMoved, this, &VToolSeamAllowance::SaveMoveGrainline); + connect(m_grainLine, &VGrainlineItem::SignalResized, this, &VToolSeamAllowance::SaveResizeGrainline); + connect(m_grainLine, &VGrainlineItem::SignalRotated, this, &VToolSeamAllowance::SaveRotateGrainline); + + connect(doc, &VAbstractPattern::UpdatePatternLabel, this, &VToolSeamAllowance::UpdatePatternInfo); + connect(doc, &VAbstractPattern::UpdatePatternLabel, this, &VToolSeamAllowance::UpdateDetailLabel); + connect(doc, &VAbstractPattern::CheckLayout, this, &VToolSeamAllowance::UpdateDetailLabel); + connect(doc, &VAbstractPattern::CheckLayout, this, &VToolSeamAllowance::UpdatePatternInfo); + connect(doc, &VAbstractPattern::CheckLayout, this, &VToolSeamAllowance::UpdateGrainline); + + connect(m_sceneDetails, &VMainGraphicsScene::EnableToolMove, this, &VToolSeamAllowance::EnableToolMove); + connect(m_sceneDetails, &VMainGraphicsScene::ItemClicked, this, &VToolSeamAllowance::ResetChildren); + connect(m_sceneDetails, &VMainGraphicsScene::DimensionsChanged, this, &VToolSeamAllowance::UpdateDetailLabel); + connect(m_sceneDetails, &VMainGraphicsScene::DimensionsChanged, this, &VToolSeamAllowance::UpdatePatternInfo); + connect(m_sceneDetails, &VMainGraphicsScene::LanguageChanged, this, &VToolSeamAllowance::retranslateUi); + connect(m_sceneDetails, &VMainGraphicsScene::EnableDetailItemHover, this, &VToolSeamAllowance::AllowHover); + connect(m_sceneDetails, &VMainGraphicsScene::EnableDetailItemSelection, this, &VToolSeamAllowance::AllowSelecting); + connect(m_sceneDetails, &VMainGraphicsScene::HighlightDetail, this, &VToolSeamAllowance::Highlight); +} + +//--------------------------------------------------------------------------------------------------------------------- +void VToolSeamAllowance::ReinitInternals(const VPiece &detail, VMainGraphicsScene *scene) +{ + InitNodes(detail, scene); + InitCSAPaths(detail); + InitInternalPaths(detail); + InitPins(detail); +} + //--------------------------------------------------------------------------------------------------------------------- QString VToolSeamAllowance::getTagName() const { @@ -1137,10 +1191,7 @@ VToolSeamAllowance::VToolSeamAllowance(const VToolSeamAllowanceInitData &initDat m_passmarks(new QGraphicsPathItem(this)) { VPiece detail = initData.data->GetPiece(initData.id); - InitNodes(detail, initData.scene); - InitCSAPaths(detail); - InitInternalPaths(detail); - InitPins(detail); + ReinitInternals(detail, m_sceneDetails); this->setFlag(QGraphicsItem::ItemIsMovable, true); this->setFlag(QGraphicsItem::ItemIsSelectable, true); RefreshGeometry(); @@ -1148,32 +1199,12 @@ VToolSeamAllowance::VToolSeamAllowance(const VToolSeamAllowanceInitData &initDat this->setFlag(QGraphicsItem::ItemSendsGeometryChanges, true); this->setFlag(QGraphicsItem::ItemIsFocusable, true);// For keyboard input focus - connect(initData.scene, &VMainGraphicsScene::EnableToolMove, this, &VToolSeamAllowance::EnableToolMove); - connect(initData.scene, &VMainGraphicsScene::ItemClicked, this, &VToolSeamAllowance::ResetChildren); ToolCreation(initData.typeCreation); setAcceptHoverEvents(true); - connect(m_dataLabel, &VTextGraphicsItem::SignalMoved, this, &VToolSeamAllowance::SaveMoveDetail); - connect(m_dataLabel, &VTextGraphicsItem::SignalResized, this, &VToolSeamAllowance::SaveResizeDetail); - connect(m_dataLabel, &VTextGraphicsItem::SignalRotated, this, &VToolSeamAllowance::SaveRotationDetail); + connect(this, &VToolSeamAllowance::ChoosedTool, m_sceneDetails, &VMainGraphicsScene::ChoosedItem); - connect(m_patternInfo, &VTextGraphicsItem::SignalMoved, this, &VToolSeamAllowance::SaveMovePattern); - connect(m_patternInfo, &VTextGraphicsItem::SignalResized, this, &VToolSeamAllowance::SaveResizePattern); - connect(m_patternInfo, &VTextGraphicsItem::SignalRotated, this, &VToolSeamAllowance::SaveRotationPattern); - - connect(m_grainLine, &VGrainlineItem::SignalMoved, this, &VToolSeamAllowance::SaveMoveGrainline); - connect(m_grainLine, &VGrainlineItem::SignalResized, this, &VToolSeamAllowance::SaveResizeGrainline); - connect(m_grainLine, &VGrainlineItem::SignalRotated, this, &VToolSeamAllowance::SaveRotateGrainline); - - connect(doc, &VAbstractPattern::UpdatePatternLabel, this, &VToolSeamAllowance::UpdatePatternInfo); - connect(doc, &VAbstractPattern::UpdatePatternLabel, this, &VToolSeamAllowance::UpdateDetailLabel); - connect(doc, &VAbstractPattern::CheckLayout, this, &VToolSeamAllowance::UpdateDetailLabel); - connect(doc, &VAbstractPattern::CheckLayout, this, &VToolSeamAllowance::UpdatePatternInfo); - connect(doc, &VAbstractPattern::CheckLayout, this, &VToolSeamAllowance::UpdateGrainline); - - connect(m_sceneDetails, &VMainGraphicsScene::DimensionsChanged, this, &VToolSeamAllowance::UpdateDetailLabel); - connect(m_sceneDetails, &VMainGraphicsScene::DimensionsChanged, this, &VToolSeamAllowance::UpdatePatternInfo); - connect(m_sceneDetails, &VMainGraphicsScene::LanguageChanged, this, &VToolSeamAllowance::retranslateUi); + ConnectOutsideSignals(); UpdateDetailLabel(); UpdatePatternInfo(); @@ -1478,13 +1509,16 @@ void VToolSeamAllowance::InitNode(const VPieceNode &node, VMainGraphicsScene *sc VNodePoint *tool = qobject_cast(VAbstractPattern::getTool(node.GetId())); SCASSERT(tool != nullptr); - connect(tool, &VNodePoint::ShowContextMenu, parent, &VToolSeamAllowance::contextMenuEvent); - connect(tool, &VNodePoint::ChoosedTool, scene, &VMainGraphicsScene::ChoosedItem); - tool->setParentItem(parent); - tool->SetParentType(ParentType::Item); - tool->SetExluded(node.IsExcluded()); + if (tool->parent() != parent) + { + connect(tool, &VNodePoint::ShowContextMenu, parent, &VToolSeamAllowance::contextMenuEvent); + connect(tool, &VNodePoint::ChoosedTool, scene, &VMainGraphicsScene::ChoosedItem); + tool->setParentItem(parent); + tool->SetParentType(ParentType::Item); + tool->SetExluded(node.IsExcluded()); + doc->IncrementReferens(node.GetId()); + } tool->setVisible(not node.IsExcluded());//Hide excluded point - doc->IncrementReferens(node.GetId()); break; } case (Tool::NodeArc): @@ -1515,8 +1549,12 @@ void VToolSeamAllowance::InitInternalPaths(const VPiece &detail) { auto *tool = qobject_cast(VAbstractPattern::getTool(detail.GetInternalPaths().at(i))); SCASSERT(tool != nullptr); - tool->setParentItem(this); - tool->SetParentType(ParentType::Item); + + if (tool->parent() != this) + { + tool->setParentItem(this); + tool->SetParentType(ParentType::Item); + } tool->show(); doc->IncrementReferens(detail.GetInternalPaths().at(i)); } @@ -1534,33 +1572,15 @@ void VToolSeamAllowance::InitPins(const VPiece &detail) //--------------------------------------------------------------------------------------------------------------------- void VToolSeamAllowance::DeleteTool(bool ask) { - QScopedPointer delDet(new DeletePiece(doc, m_id, VAbstractTool::data.GetPiece(m_id))); + QScopedPointer delDet(new DeletePiece(doc, m_id, VAbstractTool::data, m_sceneDetails)); if (ask) { if (ConfirmDeletion() == QMessageBox::No) { return; } - /* If UnionDetails tool delete detail no need emit FullParsing.*/ - connect(delDet.data(), &DeletePiece::NeedFullParsing, doc, &VAbstractPattern::NeedFullParsing); } - // If UnionDetails tool delete the detail this object will be deleted only after full parse. - // Deleting inside UnionDetails cause crash. - // Because this object should be inactive from no one we disconnect all signals that may cause a crash - // KEEP THIS LIST ACTUALL!!! - disconnect(doc, nullptr, this, nullptr); - if (QGraphicsScene *toolScene = scene()) - { - disconnect(toolScene, nullptr, this, nullptr); - } - disconnect(m_dataLabel, nullptr, this, nullptr); - disconnect(m_patternInfo, nullptr, this, nullptr); - disconnect(m_grainLine, nullptr, this, nullptr); - disconnect(m_sceneDetails, nullptr, this, nullptr); - - hide();// User shouldn't see this object - qApp->getUndoStack()->push(delDet.take()); // Throw exception, this will help prevent case when we forget to immediately quit function. diff --git a/src/libs/vtools/tools/vtoolseamallowance.h b/src/libs/vtools/tools/vtoolseamallowance.h index c3b5e9187..3a353d0b4 100644 --- a/src/libs/vtools/tools/vtoolseamallowance.h +++ b/src/libs/vtools/tools/vtoolseamallowance.h @@ -102,6 +102,9 @@ public: void Move(qreal x, qreal y); void Update(const VPiece &piece); + void DisconnectOutsideSignals(); + void ConnectOutsideSignals(); + void ReinitInternals(const VPiece &detail, VMainGraphicsScene *scene); virtual int type() const Q_DECL_OVERRIDE {return Type;} enum { Type = UserType + static_cast(Tool::Piece)}; diff --git a/src/libs/vtools/tools/vtooluniondetails.cpp b/src/libs/vtools/tools/vtooluniondetails.cpp index 05eeb1434..94dea8dd1 100644 --- a/src/libs/vtools/tools/vtooluniondetails.cpp +++ b/src/libs/vtools/tools/vtooluniondetails.cpp @@ -1369,9 +1369,6 @@ void CreateUnitedDetail(const VToolUnionDetailsInitData &initData, qreal dx, qre SCASSERT(toolDet != nullptr); bool ask = false; toolDet->Remove(ask); - // We do not call full parse, so will need more to do more cleaning than usually - initData.doc->RemoveTool(id); - initData.data->RemovePiece(id); }; if (not initData.retainPieces) diff --git a/src/libs/vtools/undocommands/deletepiece.cpp b/src/libs/vtools/undocommands/deletepiece.cpp index 7f6261e56..0183ac43c 100644 --- a/src/libs/vtools/undocommands/deletepiece.cpp +++ b/src/libs/vtools/undocommands/deletepiece.cpp @@ -40,13 +40,19 @@ #include "vundocommand.h" #include "../vpatterndb/vpiecenode.h" #include "../vpatterndb/vpiecepath.h" +#include "../vwidgets/vmaingraphicsview.h" //--------------------------------------------------------------------------------------------------------------------- -DeletePiece::DeletePiece(VAbstractPattern *doc, quint32 id, const VPiece &detail, QUndoCommand *parent) +DeletePiece::DeletePiece(VAbstractPattern *doc, quint32 id, VContainer data, VMainGraphicsScene *scene, + QUndoCommand *parent) : VUndoCommand(QDomElement(), doc, parent), m_parentNode(), m_siblingId(NULL_ID), - m_detail(detail) + m_detail(data.GetPiece(id)), + m_data(data), + m_scene(scene), + m_tool(), + m_record(VAbstractTool::GetRecord(id, Tool::Piece, doc)) { setText(tr("delete tool")); nodeId = id; @@ -74,7 +80,12 @@ DeletePiece::DeletePiece(VAbstractPattern *doc, quint32 id, const VPiece &detail //--------------------------------------------------------------------------------------------------------------------- DeletePiece::~DeletePiece() -{} +{ + if (not m_tool.isNull()) + { + delete m_tool; + } +} //--------------------------------------------------------------------------------------------------------------------- void DeletePiece::undo() @@ -82,7 +93,19 @@ void DeletePiece::undo() qCDebug(vUndo, "Undo."); UndoDeleteAfterSibling(m_parentNode, m_siblingId); - emit NeedFullParsing(); + + VAbstractPattern::AddTool(nodeId, m_tool); + m_data.UpdatePiece(nodeId, m_detail); + + m_tool->ReinitInternals(m_detail, m_scene); + + VAbstractTool::AddRecord(m_record, doc); + m_scene->addItem(m_tool); + m_tool->ConnectOutsideSignals(); + m_tool->show(); + m_tool.clear(); + + VMainGraphicsView::NewSceneRect(m_scene, qApp->getSceneView()); } //--------------------------------------------------------------------------------------------------------------------- @@ -95,17 +118,23 @@ void DeletePiece::redo() { m_parentNode.removeChild(domElement); - // UnionDetails delete two old details and create one new. - // So when UnionDetail delete detail we can't use FullParsing. So we hide detail on scene directly. - VToolSeamAllowance *toolDet = qobject_cast(VAbstractPattern::getTool(nodeId)); - SCASSERT(toolDet != nullptr); - toolDet->hide(); + m_tool = qobject_cast(VAbstractPattern::getTool(nodeId)); + SCASSERT(not m_tool.isNull()); + m_tool->DisconnectOutsideSignals(); + m_tool->hide(); + + m_scene->removeItem(m_tool); + + VAbstractPattern::RemoveTool(nodeId); + m_data.RemovePiece(nodeId); + VAbstractTool::RemoveRecord(m_record, doc); DecrementReferences(m_detail.GetPath().GetNodes()); DecrementReferences(m_detail.GetCustomSARecords()); DecrementReferences(m_detail.GetInternalPaths()); DecrementReferences(m_detail.GetPins()); - emit NeedFullParsing(); // Doesn't work when UnionDetail delete detail. + + VMainGraphicsView::NewSceneRect(m_scene, qApp->getSceneView()); } else { diff --git a/src/libs/vtools/undocommands/deletepiece.h b/src/libs/vtools/undocommands/deletepiece.h index eab5788c6..02b70af05 100644 --- a/src/libs/vtools/undocommands/deletepiece.h +++ b/src/libs/vtools/undocommands/deletepiece.h @@ -39,7 +39,8 @@ class DeletePiece : public VUndoCommand { Q_OBJECT public: - DeletePiece(VAbstractPattern *doc, quint32 id, const VPiece &detail, QUndoCommand *parent = nullptr); + DeletePiece(VAbstractPattern *doc, quint32 id, VContainer data, VMainGraphicsScene *scene, + QUndoCommand *parent = nullptr); virtual ~DeletePiece(); virtual void undo() Q_DECL_OVERRIDE; @@ -47,9 +48,13 @@ public: private: Q_DISABLE_COPY(DeletePiece) - QDomNode m_parentNode; - quint32 m_siblingId; - VPiece m_detail; + QDomNode m_parentNode; + quint32 m_siblingId; + VPiece m_detail; + VContainer m_data; + VMainGraphicsScene *m_scene; + QPointer m_tool; + VToolRecord m_record; }; #endif // DELETEPIECE_H