From a043b7fa5e8301f8723f9b371343da24f850d929 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Sat, 24 May 2025 14:06:33 +0200 Subject: [PATCH] QmlDesigner: Remove extra isValid checks for anchors QmlItemNode::isValid() is very expensive. NodeInstance like ModelNode etc. should already handle if it is called in an invalid state and return false. So there is no need for the extra check. Change-Id: Ic60f50c0e6653189bb11904b735358d3b63d1fe2 Reviewed-by: Thomas Hartmann --- .../components/formeditor/movemanipulator.cpp | 3 +- .../propertyeditor/qmlanchorbindingproxy.cpp | 27 +++++----- .../qmldesigner/instances/nodeinstance.cpp | 49 +++++++++++++------ .../qmldesigner/instances/nodeinstance.h | 13 +++-- .../instances/nodeinstanceview.cpp | 13 ++++- .../qmldesigner/instances/nodeinstanceview.h | 3 +- .../qmldesigner/qmltools/qmlanchors.cpp | 3 -- .../qmldesigner/qmltools/qmlobjectnode.cpp | 7 ++- .../qmldesigner/qmltools/qmlobjectnode.h | 2 +- 9 files changed, 75 insertions(+), 45 deletions(-) diff --git a/src/plugins/qmldesigner/components/formeditor/movemanipulator.cpp b/src/plugins/qmldesigner/components/formeditor/movemanipulator.cpp index 516e7a3c609..a6425b7bf49 100644 --- a/src/plugins/qmldesigner/components/formeditor/movemanipulator.cpp +++ b/src/plugins/qmldesigner/components/formeditor/movemanipulator.cpp @@ -104,7 +104,8 @@ void MoveManipulator::setDirectUpdateInNodeInstances(bool directUpdate) const auto allFormEditorItems = m_view->scene()->allFormEditorItems(); for (FormEditorItem *item : std::as_const(m_itemList)) { if (item && allFormEditorItems.contains(item) && item->qmlItemNode().isValid()) - item->qmlItemNode().nodeInstance().setDirectUpdate(directUpdate); + if (auto instance = item->qmlItemNode().nodeInstance()) + instance.setDirectUpdate(directUpdate); } } diff --git a/src/plugins/qmldesigner/components/propertyeditor/qmlanchorbindingproxy.cpp b/src/plugins/qmldesigner/components/propertyeditor/qmlanchorbindingproxy.cpp index 627a3d01b5f..87d0b9864d5 100644 --- a/src/plugins/qmldesigner/components/propertyeditor/qmlanchorbindingproxy.cpp +++ b/src/plugins/qmldesigner/components/propertyeditor/qmlanchorbindingproxy.cpp @@ -274,7 +274,7 @@ bool QmlAnchorBindingProxy::executeInTransaction(const QByteArray &identifier, c bool QmlAnchorBindingProxy::hasParent() const { - return m_qmlItemNode.isValid() && m_qmlItemNode.hasNodeParent(); + return m_qmlItemNode.hasNodeParent(); } bool QmlAnchorBindingProxy::isInLayout() const @@ -284,41 +284,36 @@ bool QmlAnchorBindingProxy::isInLayout() const bool QmlAnchorBindingProxy::isFilled() const { - return m_qmlItemNode.isValid() - && hasAnchors() - && topAnchored() - && bottomAnchored() - && leftAnchored() - && rightAnchored() - && (m_qmlItemNode.instanceValue("anchors.topMargin").toInt() == 0) - && (m_qmlItemNode.instanceValue("anchors.bottomMargin").toInt() == 0) - && (m_qmlItemNode.instanceValue("anchors.leftMargin").toInt() == 0) - && (m_qmlItemNode.instanceValue("anchors.rightMargin").toInt() == 0); + return hasAnchors() && topAnchored() && bottomAnchored() && leftAnchored() && rightAnchored() + && (m_qmlItemNode.instanceValue("anchors.topMargin").toInt() == 0) + && (m_qmlItemNode.instanceValue("anchors.bottomMargin").toInt() == 0) + && (m_qmlItemNode.instanceValue("anchors.leftMargin").toInt() == 0) + && (m_qmlItemNode.instanceValue("anchors.rightMargin").toInt() == 0); } bool QmlAnchorBindingProxy::topAnchored() const { - return m_qmlItemNode.isValid() && m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineTop); + return m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineTop); } bool QmlAnchorBindingProxy::bottomAnchored() const { - return m_qmlItemNode.isValid() && m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineBottom); + return m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineBottom); } bool QmlAnchorBindingProxy::leftAnchored() const { - return m_qmlItemNode.isValid() && m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineLeft); + return m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineLeft); } bool QmlAnchorBindingProxy::rightAnchored() const { - return m_qmlItemNode.isValid() && m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineRight); + return m_qmlItemNode.anchors().instanceHasAnchor(AnchorLineRight); } bool QmlAnchorBindingProxy::hasAnchors() const { - return m_qmlItemNode.isValid() && m_qmlItemNode.anchors().instanceHasAnchors(); + return m_qmlItemNode.anchors().instanceHasAnchors(); } diff --git a/src/plugins/qmldesigner/instances/nodeinstance.cpp b/src/plugins/qmldesigner/instances/nodeinstance.cpp index b136795bde3..9e1ee62086f 100644 --- a/src/plugins/qmldesigner/instances/nodeinstance.cpp +++ b/src/plugins/qmldesigner/instances/nodeinstance.cpp @@ -20,7 +20,10 @@ namespace QmlDesigner { class ProxyNodeInstanceData { public: - ProxyNodeInstanceData() + ProxyNodeInstanceData() = default; + + ProxyNodeInstanceData(const ModelNode &node) + : modelNode{node} {} qint32 parentInstanceId{-1}; @@ -58,20 +61,14 @@ public: QStringList allStates; }; -NodeInstance::NodeInstance() = default; - -NodeInstance::NodeInstance(ProxyNodeInstanceData *dPointer) - : d(dPointer) +NodeInstance::NodeInstance(const ModelNode &node) + : d(std::make_shared(node)) { } NodeInstance NodeInstance::create(const ModelNode &node) { - auto d = new ProxyNodeInstanceData; - - d->modelNode = node; - - return NodeInstance(d); + return NodeInstance{node}; } NodeInstance::~NodeInstance() = default; @@ -141,17 +138,33 @@ bool NodeInstance::hasAnchors() const QString NodeInstance::error() const { - return d->errorMessage; + if (d) + return d->errorMessage; + + return {}; } bool NodeInstance::hasError() const { - return !d->errorMessage.isEmpty(); + if (d) + return !d->errorMessage.isEmpty(); + + return false; } QStringList NodeInstance::allStateNames() const { - return d->allStates; + if (d) + return d->allStates; + + return {}; +} + +static constinit NodeInstance nullInstance; + +NodeInstance &NodeInstance::null() +{ + return nullInstance; } bool NodeInstance::isValid() const @@ -405,7 +418,7 @@ qint32 NodeInstance::parentId() const if (isValid()) return d->parentInstanceId; else - return false; + return -1; } bool NodeInstance::hasAnchor(PropertyNameView name) const @@ -489,11 +502,17 @@ void NodeInstance::setProperty(PropertyNameView name, const QVariant &value) QPixmap NodeInstance::renderPixmap() const { - return d->renderPixmap; + if (isValid()) + return d->renderPixmap; + + return {}; } QPixmap NodeInstance::blurredRenderPixmap() const { + if (!isValid()) + return {}; + if (d->blurredRenderPixmap.isNull()) { d->blurredRenderPixmap = QPixmap(d->renderPixmap.size()); QPainter blurPainter(&d->blurredRenderPixmap); diff --git a/src/plugins/qmldesigner/instances/nodeinstance.h b/src/plugins/qmldesigner/instances/nodeinstance.h index 1cb1fcd233f..ee15dbb9315 100644 --- a/src/plugins/qmldesigner/instances/nodeinstance.h +++ b/src/plugins/qmldesigner/instances/nodeinstance.h @@ -3,7 +3,6 @@ #pragma once -#include #include #include #include @@ -12,6 +11,8 @@ #include "commondefines.h" #include "nodeinstanceglobal.h" +#include + namespace QmlDesigner { class ModelNode; @@ -24,13 +25,15 @@ class NodeInstance public: static NodeInstance create(const ModelNode &node); - NodeInstance(); + constexpr NodeInstance() = default; ~NodeInstance(); NodeInstance(const NodeInstance &other); NodeInstance& operator=(const NodeInstance &other); ModelNode modelNode() const; bool isValid() const; + + explicit operator bool() const { return isValid(); } void makeInvalid(); QRectF boundingRect() const; QRectF contentItemBoundingRect() const; @@ -71,6 +74,8 @@ public: bool hasError() const; QStringList allStateNames() const; + static NodeInstance &null(); + protected: void setProperty(PropertyNameView name, const QVariant &value); InformationName setInformation(InformationName name, @@ -107,10 +112,10 @@ protected: void setParentId(qint32 instanceId); void setRenderPixmap(const QImage &image); bool setError(const QString &errorMessage); - NodeInstance(ProxyNodeInstanceData *d); + NodeInstance(const ModelNode &node); private: - QSharedPointer d; + std::shared_ptr d; }; bool operator ==(const NodeInstance &first, const NodeInstance &second); diff --git a/src/plugins/qmldesigner/instances/nodeinstanceview.cpp b/src/plugins/qmldesigner/instances/nodeinstanceview.cpp index 3f3a29047f2..74350c2c60e 100644 --- a/src/plugins/qmldesigner/instances/nodeinstanceview.cpp +++ b/src/plugins/qmldesigner/instances/nodeinstanceview.cpp @@ -777,12 +777,21 @@ QList NodeInstanceView::instances() const \sa NodeInstance */ -NodeInstance NodeInstanceView::instanceForModelNode(const ModelNode &node) const +const NodeInstance &NodeInstanceView::instanceForModelNode(const ModelNode &node) const +{ + return const_cast(*this).instanceForModelNode(node); +} + +NodeInstance &NodeInstanceView::instanceForModelNode(const ModelNode &node) { Q_ASSERT(node.isValid()); Q_ASSERT(m_nodeInstanceHash.contains(node)); Q_ASSERT(m_nodeInstanceHash.value(node).modelNode() == node); - return m_nodeInstanceHash.value(node); + auto found = m_nodeInstanceHash.find(node); + if (found != m_nodeInstanceHash.end()) + return *found; + + return NodeInstance::null(); } bool NodeInstanceView::hasInstanceForModelNode(const ModelNode &node) const diff --git a/src/plugins/qmldesigner/instances/nodeinstanceview.h b/src/plugins/qmldesigner/instances/nodeinstanceview.h index 7a5ea908d94..fca1c74e9a3 100644 --- a/src/plugins/qmldesigner/instances/nodeinstanceview.h +++ b/src/plugins/qmldesigner/instances/nodeinstanceview.h @@ -102,7 +102,8 @@ public: void sceneCreated(const SceneCreatedCommand &command) override; QList instances() const; - NodeInstance instanceForModelNode(const ModelNode &node) const ; + const NodeInstance &instanceForModelNode(const ModelNode &node) const; + NodeInstance &instanceForModelNode(const ModelNode &node); bool hasInstanceForModelNode(const ModelNode &node) const; NodeInstance instanceForId(qint32 id) const; diff --git a/src/plugins/qmldesigner/qmltools/qmlanchors.cpp b/src/plugins/qmldesigner/qmltools/qmlanchors.cpp index a3a54d85fb1..fc1f6b13c32 100644 --- a/src/plugins/qmldesigner/qmltools/qmlanchors.cpp +++ b/src/plugins/qmldesigner/qmltools/qmlanchors.cpp @@ -458,9 +458,6 @@ bool QmlAnchors::instanceHasAnchor(AnchorLineType sourceAnchorLine, SL sl) const keyValue("model node", *this), keyValue("caller location", sl)}; - if (!qmlItemNode().isValid()) - return false; - const PropertyNameView propertyName = anchorPropertyName(sourceAnchorLine); if (sourceAnchorLine & AnchorLineFill) diff --git a/src/plugins/qmldesigner/qmltools/qmlobjectnode.cpp b/src/plugins/qmldesigner/qmltools/qmlobjectnode.cpp index 5b99776edf7..2a2c529d6c3 100644 --- a/src/plugins/qmldesigner/qmltools/qmlobjectnode.cpp +++ b/src/plugins/qmldesigner/qmltools/qmlobjectnode.cpp @@ -822,9 +822,12 @@ bool QmlObjectNode::instanceHasBinding(PropertyNameView name, SL sl) const return nodeInstance().hasBindingForProperty(name); } -NodeInstance QmlObjectNode::nodeInstance() const +const NodeInstance &QmlObjectNode::nodeInstance() const { - return nodeInstanceView()->instanceForModelNode(modelNode()); + if (auto view = nodeInstanceView()) + return view->instanceForModelNode(modelNode()); + + return NodeInstance::null(); } QmlObjectNode QmlObjectNode::nodeForInstance(const NodeInstance &instance) const diff --git a/src/plugins/qmldesigner/qmltools/qmlobjectnode.h b/src/plugins/qmldesigner/qmltools/qmlobjectnode.h index 1b0364d626a..f8e40b89bb4 100644 --- a/src/plugins/qmldesigner/qmltools/qmlobjectnode.h +++ b/src/plugins/qmldesigner/qmltools/qmlobjectnode.h @@ -127,7 +127,7 @@ public: QList getAllConnections(SL sl = {}) const; protected: - NodeInstance nodeInstance() const; + const NodeInstance &nodeInstance() const; QmlObjectNode nodeForInstance(const NodeInstance &instance) const; QmlItemNode itemForInstance(const NodeInstance &instance) const; };