From 9dd796cb8cd3ea181494acec71125fabb2235953 Mon Sep 17 00:00:00 2001 From: Roman Telezhynskyi Date: Mon, 9 Mar 2020 20:07:17 +0200 Subject: [PATCH] Improve preparing history list. Basically when we have cleared by garbage collector modeling objects Valentina will try to add them to list, because they still present in tool record list. Additionally this patch brings using multithreading support. --- src/app/valentina/dialogs/dialoghistory.cpp | 253 ++++++++++++-------- src/app/valentina/dialogs/dialoghistory.h | 12 +- src/app/valentina/xml/vpattern.cpp | 16 ++ src/libs/ifc/xml/vdomdocument.cpp | 14 +- src/libs/ifc/xml/vdomdocument.h | 2 +- 5 files changed, 181 insertions(+), 116 deletions(-) diff --git a/src/app/valentina/dialogs/dialoghistory.cpp b/src/app/valentina/dialogs/dialoghistory.cpp index 7d8f1fb37..9e791ca62 100644 --- a/src/app/valentina/dialogs/dialoghistory.cpp +++ b/src/app/valentina/dialogs/dialoghistory.cpp @@ -40,7 +40,9 @@ #include "../vtools/tools/drawTools/toolpoint/toolsinglepoint/toolcut/vtoolcutarc.h" #include "../xml/vpattern.h" #include "../vmisc/diagnostic.h" + #include +#include //--------------------------------------------------------------------------------------------------------------------- /** @@ -165,26 +167,30 @@ void DialogHistory::FillTable() qint32 currentRow = -1; qint32 count = 0; ui->tableWidget->setRowCount(history.size());//Make row count max possible number - for (qint32 i = 0; i< history.size(); ++i) + + std::function CreateRecord = [this](const VToolRecord &tool) { - const VToolRecord tool = history.at(i); - const QString historyRecord = Record(tool); - if (not historyRecord.isEmpty()) + return Record(tool); + }; + + QVector historyRecords = QtConcurrent::blockingMapped(history, CreateRecord); + + for (auto &record : historyRecords) + { + if (not record.description.isEmpty()) { currentRow++; { QTableWidgetItem *item = new QTableWidgetItem(QString()); item->setTextAlignment(Qt::AlignHCenter); - item->setData(Qt::UserRole, tool.getId()); + item->setData(Qt::UserRole, record.id); item->setFlags(item->flags() ^ Qt::ItemIsEditable); ui->tableWidget->setItem(currentRow, 0, item); } - QTableWidgetItem *item = new QTableWidgetItem(historyRecord); - QFont font = item->font(); - font.setBold(true); - item->setFont(font); + QTableWidgetItem *item = new QTableWidgetItem(record.description); + item->setFont(QFont("Times", 12, QFont::Bold)); item->setFlags(item->flags() ^ Qt::ItemIsEditable); ui->tableWidget->setItem(currentRow, 1, item); ++count; @@ -211,16 +217,20 @@ QT_WARNING_DISABLE_GCC("-Wswitch-default") * @param tool record data * @return description */ -QString DialogHistory::Record(const VToolRecord &tool) +HistoryRecord DialogHistory::Record(const VToolRecord &tool) const { // This check helps to find missed tools in the switch Q_STATIC_ASSERT_X(static_cast(Tool::LAST_ONE_DO_NOT_USE) == 55, "Not all tools were used in history."); - const QDomElement domElem = doc->elementById(tool.getId()); + HistoryRecord record; + record.id = tool.getId(); + + bool updateCache = false; + const QDomElement domElem = doc->elementById(tool.getId(), QString(), updateCache); if (domElem.isElement() == false) { - qDebug()<<"Can't find element by id"< spl = data->GeometricObject(tool.getId()); SCASSERT(not spl.isNull()) - return spl->NameForHistory(tr("Curve")); + record.description = spl->NameForHistory(tr("Curve")); + return record; } case Tool::CubicBezier: { const QSharedPointer spl = data->GeometricObject(tool.getId()); SCASSERT(not spl.isNull()) - return spl->NameForHistory(tr("Cubic bezier curve")); + record.description = spl->NameForHistory(tr("Cubic bezier curve")); + return record; } case Tool::Arc: { const QSharedPointer arc = data->GeometricObject(tool.getId()); SCASSERT(not arc.isNull()) - return arc->NameForHistory(tr("Arc")); + record.description = arc->NameForHistory(tr("Arc")); + return record; } case Tool::ArcWithLength: { const QSharedPointer arc = data->GeometricObject(tool.getId()); SCASSERT(not arc.isNull()) - return tr("%1 with length %2") - .arg(arc->NameForHistory(tr("Arc"))) - .arg(arc->GetLength()); + record.description = tr("%1 with length %2") + .arg(arc->NameForHistory(tr("Arc"))) + .arg(arc->GetLength()); + return record; } case Tool::SplinePath: { const QSharedPointer splPath = data->GeometricObject(tool.getId()); SCASSERT(not splPath.isNull()) - return splPath->NameForHistory(tr("Spline path")); + record.description = splPath->NameForHistory(tr("Spline path")); + return record; } case Tool::CubicBezierPath: { const QSharedPointer splPath = data->GeometricObject(tool.getId()); SCASSERT(not splPath.isNull()) - return splPath->NameForHistory(tr("Cubic bezier curve path")); + record.description = splPath->NameForHistory(tr("Cubic bezier curve path")); + return record; } case Tool::PointOfContact: - return tr("%4 - point of contact of arc with the center in point %1 and line %2_%3") - .arg(PointName(AttrUInt(domElem, AttrCenter)), - PointName(AttrUInt(domElem, AttrFirstPoint)), - PointName(AttrUInt(domElem, AttrSecondPoint)), - PointName(tool.getId())); + record.description = tr("%4 - point of contact of arc with the center in point %1 and line %2_%3") + .arg(PointName(AttrUInt(domElem, AttrCenter)), + PointName(AttrUInt(domElem, AttrFirstPoint)), + PointName(AttrUInt(domElem, AttrSecondPoint)), + PointName(tool.getId())); + return record; case Tool::Height: - return tr("Point of perpendicular from point %1 to line %2_%3") - .arg(PointName(AttrUInt(domElem, AttrBasePoint)), - PointName(AttrUInt(domElem, AttrP1Line)), - PointName(AttrUInt(domElem, AttrP2Line))); + record.description = tr("Point of perpendicular from point %1 to line %2_%3") + .arg(PointName(AttrUInt(domElem, AttrBasePoint)), + PointName(AttrUInt(domElem, AttrP1Line)), + PointName(AttrUInt(domElem, AttrP2Line))); + return record; case Tool::Triangle: - return tr("Triangle: axis %1_%2, points %3 and %4") - .arg(PointName(AttrUInt(domElem, AttrAxisP1)), - PointName(AttrUInt(domElem, AttrAxisP2)), - PointName(AttrUInt(domElem, AttrFirstPoint)), - PointName(AttrUInt(domElem, AttrSecondPoint))); + record.description = tr("Triangle: axis %1_%2, points %3 and %4") + .arg(PointName(AttrUInt(domElem, AttrAxisP1)), + PointName(AttrUInt(domElem, AttrAxisP2)), + PointName(AttrUInt(domElem, AttrFirstPoint)), + PointName(AttrUInt(domElem, AttrSecondPoint))); + return record; case Tool::PointOfIntersection: - return tr("%1 - point of intersection %2 and %3") - .arg(PointName(tool.getId()), - PointName(AttrUInt(domElem, AttrFirstPoint)), - PointName(AttrUInt(domElem, AttrSecondPoint))); + record.description = tr("%1 - point of intersection %2 and %3") + .arg(PointName(tool.getId()), + PointName(AttrUInt(domElem, AttrFirstPoint)), + PointName(AttrUInt(domElem, AttrSecondPoint))); + return record; case Tool::CutArc: { const QSharedPointer arc = data->GeometricObject(AttrUInt(domElem, AttrArc)); SCASSERT(not arc.isNull()) - return tr("%1 - cut %2") - .arg(PointName(tool.getId()), arc->NameForHistory(tr("arc"))); + record.description = tr("%1 - cut %2") + .arg(PointName(tool.getId()), arc->NameForHistory(tr("arc"))); + return record; } case Tool::CutSpline: { const quint32 splineId = AttrUInt(domElem, VToolCutSpline::AttrSpline); const QSharedPointer spl = data->GeometricObject(splineId); SCASSERT(not spl.isNull()) - return tr("%1 - cut %2") - .arg(PointName(tool.getId()), spl->NameForHistory(tr("curve"))); + record.description = tr("%1 - cut %2") + .arg(PointName(tool.getId()), spl->NameForHistory(tr("curve"))); + return record; } case Tool::CutSplinePath: { @@ -349,56 +379,71 @@ QString DialogHistory::Record(const VToolRecord &tool) const QSharedPointer splPath = data->GeometricObject(splinePathId); SCASSERT(not splPath.isNull()) - return tr("%1 - cut %2") - .arg(PointName(tool.getId()), splPath->NameForHistory(tr("curve path"))); + record.description = tr("%1 - cut %2") + .arg(PointName(tool.getId()), splPath->NameForHistory(tr("curve path"))); + return record; } case Tool::LineIntersectAxis: - return tr("%1 - point of intersection line %2_%3 and axis through point %4") - .arg(PointName(tool.getId()), - PointName(AttrUInt(domElem, AttrP1Line)), - PointName(AttrUInt(domElem, AttrP2Line)), - PointName(AttrUInt(domElem, AttrBasePoint))); + record.description = tr("%1 - point of intersection line %2_%3 and axis through point %4") + .arg(PointName(tool.getId()), + PointName(AttrUInt(domElem, AttrP1Line)), + PointName(AttrUInt(domElem, AttrP2Line)), + PointName(AttrUInt(domElem, AttrBasePoint))); + return record; case Tool::CurveIntersectAxis: - return tr("%1 - point of intersection curve and axis through point %2") - .arg(PointName(tool.getId()), PointName(AttrUInt(domElem, AttrBasePoint))); + record.description = tr("%1 - point of intersection curve and axis through point %2") + .arg(PointName(tool.getId()), PointName(AttrUInt(domElem, AttrBasePoint))); + return record; case Tool::PointOfIntersectionArcs: - return tr("%1 - point of arcs intersection").arg(PointName(tool.getId())); + record.description = tr("%1 - point of arcs intersection").arg(PointName(tool.getId())); + return record; case Tool::PointOfIntersectionCircles: - return tr("%1 - point of circles intersection").arg(PointName(tool.getId())); + record.description = tr("%1 - point of circles intersection").arg(PointName(tool.getId())); + return record; case Tool::PointOfIntersectionCurves: - return tr("%1 - point of curves intersection").arg(PointName(tool.getId())); + record.description = tr("%1 - point of curves intersection").arg(PointName(tool.getId())); + return record; case Tool::PointFromCircleAndTangent: - return tr("%1 - point from circle and tangent").arg(PointName(tool.getId())); + record.description = tr("%1 - point from circle and tangent").arg(PointName(tool.getId())); + return record; case Tool::PointFromArcAndTangent: - return tr("%1 - point from arc and tangent").arg(PointName(tool.getId())); + record.description = tr("%1 - point from arc and tangent").arg(PointName(tool.getId())); + return record; case Tool::TrueDarts: - return tr("Correction the dart %1_%2_%3") - .arg(PointName(AttrUInt(domElem, AttrDartP1)), - PointName(AttrUInt(domElem, AttrDartP2)), - PointName(AttrUInt(domElem, AttrDartP2))); + record.description = tr("Correction the dart %1_%2_%3") + .arg(PointName(AttrUInt(domElem, AttrDartP1)), + PointName(AttrUInt(domElem, AttrDartP2)), + PointName(AttrUInt(domElem, AttrDartP2))); + return record; case Tool::EllipticalArc: { const QSharedPointer elArc = data->GeometricObject(tool.getId()); SCASSERT(not elArc.isNull()) - return tr("%1 with length %2") - .arg(elArc->NameForHistory(tr("Elliptical arc"))) - .arg(elArc->GetLength()); + record.description = tr("%1 with length %2") + .arg(elArc->NameForHistory(tr("Elliptical arc"))) + .arg(elArc->GetLength()); + return record; } case Tool::Rotation: - return tr("Rotate objects around point %1. Suffix '%2'") - .arg(PointName(AttrUInt(domElem, AttrCenter)), - doc->GetParametrString(domElem, AttrSuffix, QString())); + record.description = tr("Rotate objects around point %1. Suffix '%2'") + .arg(PointName(AttrUInt(domElem, AttrCenter)), + doc->GetParametrString(domElem, AttrSuffix, QString())); + return record; case Tool::FlippingByLine: - return tr("Flipping by line %1_%2. Suffix '%3'") - .arg(PointName(AttrUInt(domElem, AttrP1Line)), - PointName(AttrUInt(domElem, AttrP2Line)), - doc->GetParametrString(domElem, AttrSuffix, QString())); + record.description = tr("Flipping by line %1_%2. Suffix '%3'") + .arg(PointName(AttrUInt(domElem, AttrP1Line)), + PointName(AttrUInt(domElem, AttrP2Line)), + doc->GetParametrString(domElem, AttrSuffix, QString())); + return record; case Tool::FlippingByAxis: - return tr("Flipping by axis through %1 point. Suffix '%2'") - .arg(PointName(AttrUInt(domElem, AttrCenter)), - doc->GetParametrString(domElem, AttrSuffix, QString())); + record.description = tr("Flipping by axis through %1 point. Suffix '%2'") + .arg(PointName(AttrUInt(domElem, AttrCenter)), + doc->GetParametrString(domElem, AttrSuffix, QString())); + return record; case Tool::Move: - return tr("Move objects. Suffix '%1'").arg(doc->GetParametrString(domElem, AttrSuffix, QString())); + record.description = tr("Move objects. Suffix '%1'") + .arg(doc->GetParametrString(domElem, AttrSuffix, QString())); + return record; //Because "history" not only show history of pattern, but help restore current data for each pattern's //piece, we need add record about details and nodes, but don't show them. case Tool::Piece: @@ -414,16 +459,16 @@ QString DialogHistory::Record(const VToolRecord &tool) case Tool::PlaceLabel: case Tool::InsertNode: case Tool::DuplicateDetail: - return QString(); + return record; } } catch (const VExceptionBadId &e) { qDebug()<GeometricObject(pointId)->name(); } //--------------------------------------------------------------------------------------------------------------------- -quint32 DialogHistory::AttrUInt(const QDomElement &domElement, const QString &name) +quint32 DialogHistory::AttrUInt(const QDomElement &domElement, const QString &name) const { return doc->GetParametrUInt(domElement, name, QChar('0')); } diff --git a/src/app/valentina/dialogs/dialoghistory.h b/src/app/valentina/dialogs/dialoghistory.h index bf5987b85..85e3e999d 100644 --- a/src/app/valentina/dialogs/dialoghistory.h +++ b/src/app/valentina/dialogs/dialoghistory.h @@ -35,6 +35,12 @@ class VPattern; +struct HistoryRecord +{ + QString description{}; + quint32 id{NULL_ID}; +}; + namespace Ui { class DialogHistory; @@ -86,11 +92,11 @@ private: qint32 cursorToolRecordRow; void FillTable(); - QString Record(const VToolRecord &tool); + HistoryRecord Record(const VToolRecord &tool) const; void InitialTable(); void ShowPoint(); - QString PointName(quint32 pointId); - quint32 AttrUInt(const QDomElement &domElement, const QString &name); + QString PointName(quint32 pointId) const; + quint32 AttrUInt(const QDomElement &domElement, const QString &name) const; void RetranslateUi(); int CursorRow() const; }; diff --git a/src/app/valentina/xml/vpattern.cpp b/src/app/valentina/xml/vpattern.cpp index 53387a7e1..763ae9415 100644 --- a/src/app/valentina/xml/vpattern.cpp +++ b/src/app/valentina/xml/vpattern.cpp @@ -3450,6 +3450,22 @@ void VPattern::GarbageCollector(bool commit) { modElement.removeChild(modNode); cleared = true; + + // Clear history + try + { + vidtype id = GetParametrId(modNode); + auto record = std::find_if(history.begin(), history.end(), + [id](const VToolRecord &record) { return record.getId() == id; }); + if (record != history.end()) + { + history.erase(record); + } + } + catch(const VExceptionWrongId &) + { + // do nothing + } } } } diff --git a/src/libs/ifc/xml/vdomdocument.cpp b/src/libs/ifc/xml/vdomdocument.cpp index 40941545d..df1b0061c 100644 --- a/src/libs/ifc/xml/vdomdocument.cpp +++ b/src/libs/ifc/xml/vdomdocument.cpp @@ -264,7 +264,7 @@ VDomDocument::~VDomDocument() } //--------------------------------------------------------------------------------------------------------------------- -QDomElement VDomDocument::elementById(quint32 id, const QString &tagName) +QDomElement VDomDocument::elementById(quint32 id, const QString &tagName, bool updateCache) { if (id == 0) { @@ -278,11 +278,12 @@ QDomElement VDomDocument::elementById(quint32 id, const QString &tagName) { return e; } - m_elementIdCache.remove(id); } - // Cached missed - RefreshElementIdCache(); + if (updateCache) + { // Cached missed + RefreshElementIdCache(); + } if (tagName.isEmpty()) { @@ -290,10 +291,8 @@ QDomElement VDomDocument::elementById(quint32 id, const QString &tagName) QHash tmpCache; if (VDomDocument::find(tmpCache, this->documentElement(), id)) { - m_elementIdCache = tmpCache; - return m_elementIdCache.value(id); + return tmpCache.value(id); } - m_elementIdCache = tmpCache; } else { @@ -305,7 +304,6 @@ QDomElement VDomDocument::elementById(quint32 id, const QString &tagName) { const quint32 elementId = GetParametrUInt(domElement, AttrId, NULL_ID_STR); - m_elementIdCache.insert(elementId, domElement); if (elementId == id) { return domElement; diff --git a/src/libs/ifc/xml/vdomdocument.h b/src/libs/ifc/xml/vdomdocument.h index ed3b2f108..238851546 100644 --- a/src/libs/ifc/xml/vdomdocument.h +++ b/src/libs/ifc/xml/vdomdocument.h @@ -96,7 +96,7 @@ public: explicit VDomDocument(QObject *parent = nullptr); virtual ~VDomDocument(); - QDomElement elementById(quint32 id, const QString &tagName = QString()); + QDomElement elementById(quint32 id, const QString &tagName = QString(), bool updateCache=true); template void SetAttribute(QDomElement &domElement, const QString &name, const T &value) const;