From 2b2cacfc09d72a4663b69932c156b46d989367d5 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 12 Jan 2021 12:12:56 +0100 Subject: [PATCH] Fix vanishing transitions when zooming to state The main design issue in this plugin is that there is no clear separation between model (which pretent to be ScxmlDocument and his childern of ScxmlTag type) and its views (GraphicsScene and its children of BaseItem subclasses). When the "Zoom to State" action is invoked, the new view is being created, showing just a part of the ScxmlDocument model. However, some validation is done only on the view part, checking that BaseItems belong to the common GraphicsScene. In case the transition was defined from internal state node to somewhere outside, we don't have for it an BaseItem on the gui side, as we just show the part of the scene. That's why the validation vanishes the transition when viewing a zoom. In general, all the validations should be moved from gui to the ScxmlDocument / ScxmlTag part. This in general would require really big refactoring, or even rewrite. This patch moves some checks into a model side and disables direct modifications of the model when it's not desired. Fixes: QTCREATORBUG-21676 Change-Id: Ica0771f637a9802dcc0fb87ad04b0ee77a21cda2 Reviewed-by: Ulf Hermann --- .../scxmleditor/plugin_interface/scxmldocument.cpp | 8 ++++++++ .../scxmleditor/plugin_interface/scxmldocument.h | 1 + .../scxmleditor/plugin_interface/scxmltag.cpp | 14 ++++++++++++++ .../scxmleditor/plugin_interface/scxmltag.h | 2 ++ .../plugin_interface/transitionitem.cpp | 13 +++++++------ .../scxmleditor/plugin_interface/transitionitem.h | 4 ++-- .../scxmleditor/plugin_interface/undocommands.cpp | 8 ++++++-- 7 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/plugins/scxmleditor/plugin_interface/scxmldocument.cpp b/src/plugins/scxmleditor/plugin_interface/scxmldocument.cpp index a0c677ba464..8e85ae8097f 100644 --- a/src/plugins/scxmleditor/plugin_interface/scxmldocument.cpp +++ b/src/plugins/scxmleditor/plugin_interface/scxmldocument.cpp @@ -649,6 +649,14 @@ ScxmlTag *ScxmlDocument::scxmlRootTag() const return tag; } +ScxmlTag *ScxmlDocument::tagForId(const QString &id) const +{ + if (id.isEmpty()) + return nullptr; + ScxmlTag *root = scxmlRootTag(); + return root ? root->tagForId(id) : nullptr; +} + ScxmlTag *ScxmlDocument::rootTag() const { return m_rootTags.isEmpty() ? 0 : m_rootTags.last(); diff --git a/src/plugins/scxmleditor/plugin_interface/scxmldocument.h b/src/plugins/scxmleditor/plugin_interface/scxmldocument.h index 19506731f23..50620a617c0 100644 --- a/src/plugins/scxmleditor/plugin_interface/scxmldocument.h +++ b/src/plugins/scxmleditor/plugin_interface/scxmldocument.h @@ -163,6 +163,7 @@ public: */ ScxmlTag *rootTag() const; ScxmlTag *scxmlRootTag() const; + ScxmlTag *tagForId(const QString &id) const; void pushRootTag(ScxmlTag *tag); ScxmlTag *popRootTag(); diff --git a/src/plugins/scxmleditor/plugin_interface/scxmltag.cpp b/src/plugins/scxmleditor/plugin_interface/scxmltag.cpp index 65c0e689ff6..f28b85dbb96 100644 --- a/src/plugins/scxmleditor/plugin_interface/scxmltag.cpp +++ b/src/plugins/scxmleditor/plugin_interface/scxmltag.cpp @@ -485,6 +485,20 @@ bool ScxmlTag::isRootTag() const return m_document->rootTag() == this; } +ScxmlTag *ScxmlTag::tagForId(const QString &id) const +{ + for (ScxmlTag *child : m_childTags) { + const TagType type = child->tagType(); + const bool typeOK = type == State || type == Parallel || type == History || type == Final; + if (typeOK && child->attribute("id") == id) + return child; + ScxmlTag *foundTag = child->tagForId(id); + if (foundTag) + return foundTag; + } + return nullptr; +} + QVector ScxmlTag::allChildren() const { return m_childTags; diff --git a/src/plugins/scxmleditor/plugin_interface/scxmltag.h b/src/plugins/scxmleditor/plugin_interface/scxmltag.h index 3031c9b4658..96635d0f9d7 100644 --- a/src/plugins/scxmleditor/plugin_interface/scxmltag.h +++ b/src/plugins/scxmleditor/plugin_interface/scxmltag.h @@ -112,6 +112,8 @@ public: int childIndex(const ScxmlTag *child) const; int index() const; bool isRootTag() const; + ScxmlTag *tagForId(const QString &id) const; + /** * @brief writeXml - write tag's content with the QXMLStreamWriter. Call writeXml-function for all children too. diff --git a/src/plugins/scxmleditor/plugin_interface/transitionitem.cpp b/src/plugins/scxmleditor/plugin_interface/transitionitem.cpp index fb0c5743ab2..307af62f6f9 100644 --- a/src/plugins/scxmleditor/plugin_interface/transitionitem.cpp +++ b/src/plugins/scxmleditor/plugin_interface/transitionitem.cpp @@ -708,7 +708,7 @@ void TransitionItem::setEndPos(const QPointF &endPos, bool snap) } } -void TransitionItem::setEndItem(ConnectableItem *item) +void TransitionItem::setEndItem(ConnectableItem *item, bool fixValue) { if (item) { @@ -725,7 +725,7 @@ void TransitionItem::setEndItem(ConnectableItem *item) } updateZValue(); - updateTarget(); + updateTarget(fixValue); } QPointF TransitionItem::loadPoint(const QString &name) @@ -984,9 +984,10 @@ void TransitionItem::updateEditorInfo(bool allChilds) m_pen.setColor(stateColor.isValid() ? stateColor : qRgb(0x12, 0x12, 0x12)); } -void TransitionItem::updateTarget() +void TransitionItem::updateTarget(bool fixValue) { - setTagValue("target", m_endItem ? m_endItem->itemId() : QString()); + if (fixValue) + setTagValue("target", m_endItem ? m_endItem->itemId() : QString()); if (m_endItem) m_endItem->checkInitial(true); } @@ -1002,7 +1003,7 @@ void TransitionItem::updateAttributes() m_endItem = nullptr; findEndItem(); - updateTarget(); + updateTarget(false); updateZValue(); } @@ -1123,7 +1124,7 @@ void TransitionItem::findEndItem() if (items[i]->type() >= FinalStateType) { auto item = qgraphicsitem_cast(items[i]); if (item && item->itemId() == targetId) { - setEndItem(item); + setEndItem(item, false); break; } } diff --git a/src/plugins/scxmleditor/plugin_interface/transitionitem.h b/src/plugins/scxmleditor/plugin_interface/transitionitem.h index b8586fe9165..59ad6a3547e 100644 --- a/src/plugins/scxmleditor/plugin_interface/transitionitem.h +++ b/src/plugins/scxmleditor/plugin_interface/transitionitem.h @@ -79,7 +79,7 @@ public: void disconnectItem(ConnectableItem *item); void setStartItem(ConnectableItem *item); - void setEndItem(ConnectableItem *item); + void setEndItem(ConnectableItem *item, bool fixValue = true); void startTransitionFrom(ConnectableItem *item, const QPointF &mouseScenePos); void setEndPos(const QPointF &endPos, bool snap = true); @@ -95,7 +95,7 @@ public: void init(ScxmlTag *tag, BaseItem *parentItem = nullptr, bool initChildren = true, bool blockUpdates = false) override; void updateEditorInfo(bool allChilds = true) override; void updateAttributes() override; - void updateTarget(); + void updateTarget(bool fixValue = true); void finalizeCreation() override; void checkVisibility(double scaleFactor) override; bool containsScenePoint(const QPointF &p) const override; diff --git a/src/plugins/scxmleditor/plugin_interface/undocommands.cpp b/src/plugins/scxmleditor/plugin_interface/undocommands.cpp index 0a729cb80ca..b4bd0f75f92 100644 --- a/src/plugins/scxmleditor/plugin_interface/undocommands.cpp +++ b/src/plugins/scxmleditor/plugin_interface/undocommands.cpp @@ -143,8 +143,12 @@ SetAttributeCommand::SetAttributeCommand(ScxmlDocument *doc, ScxmlTag *tag, cons void SetAttributeCommand::doAction(const QString &key, const QString &value) { emit m_document->beginTagChange(ScxmlDocument::TagAttributesChanged, m_tag, m_tag->attribute(key)); - m_tag->setAttribute(key, value); - emit m_document->endTagChange(ScxmlDocument::TagAttributesChanged, m_tag, value); + + QString newValue = value; + if (m_tag->tagType() == Transition && key == "target" && !m_document->tagForId(value)) + newValue = QString(); + m_tag->setAttribute(key, newValue); + emit m_document->endTagChange(ScxmlDocument::TagAttributesChanged, m_tag, newValue); } int SetAttributeCommand::id() const