From db84bc43a9086e1ebc2662f7f04a03ab1aed82ca Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Thu, 11 Apr 2024 18:13:10 +0200 Subject: [PATCH] QmlDesigner: Improve node creation with project storage The file url was missing, so no valid source id could be created. That lead to invalid imports. The qualified path for the node instances is now created in the node instance view. Change-Id: I2685ab2fdbdb0fa8a5f89793be52c8fae2b8db8c Reviewed-by: Reviewed-by: Thomas Hartmann Reviewed-by: Qt CI Patch Build Bot --- .../components/integration/designdocument.cpp | 5 +- .../components/integration/designdocument.h | 3 +- .../qmldesigner/designercore/include/model.h | 1 + .../designercore/include/nodemetainfo.h | 2 +- .../instances/nodeinstanceview.cpp | 59 +++++++++++++------ .../designercore/metainfo/nodemetainfo.cpp | 2 +- .../qmldesigner/designercore/model/model.cpp | 10 ++++ .../designercore/model/texttomodelmerger.cpp | 2 +- .../projectstorage/commontypecache.h | 3 + .../projectstorage/projectstorage.h | 9 ++- .../projectstorage/projectstorageinterface.h | 1 + src/plugins/qmldesigner/documentmanager.cpp | 4 +- tests/unit/tests/mocks/projectstoragemock.cpp | 13 ++++ tests/unit/tests/mocks/projectstoragemock.h | 8 +++ .../tests/unittests/model/modelutils-test.cpp | 2 +- 15 files changed, 95 insertions(+), 29 deletions(-) diff --git a/src/plugins/qmldesigner/components/integration/designdocument.cpp b/src/plugins/qmldesigner/components/integration/designdocument.cpp index c0bebbb82bd..804ac076e62 100644 --- a/src/plugins/qmldesigner/components/integration/designdocument.cpp +++ b/src/plugins/qmldesigner/components/integration/designdocument.cpp @@ -64,13 +64,14 @@ namespace QmlDesigner { DesignDocument acts as a facade to a model representing a qml document, and the different views/widgets accessing it. */ -DesignDocument::DesignDocument(ProjectStorageDependencies projectStorageDependencies, +DesignDocument::DesignDocument([[maybe_unused]] const QUrl &filePath, + ProjectStorageDependencies projectStorageDependencies, ExternalDependenciesInterface &externalDependencies) #ifdef QDS_USE_PROJECTSTORAGE : m_documentModel(Model::create(projectStorageDependencies, "Item", {Import::createLibraryImport("QtQuick")}, - {}, + filePath, std::make_unique())) #else : m_documentModel( diff --git a/src/plugins/qmldesigner/components/integration/designdocument.h b/src/plugins/qmldesigner/components/integration/designdocument.h index 0d75141205e..52089d67c1b 100644 --- a/src/plugins/qmldesigner/components/integration/designdocument.h +++ b/src/plugins/qmldesigner/components/integration/designdocument.h @@ -41,7 +41,8 @@ class QMLDESIGNERCOMPONENTS_EXPORT DesignDocument : public QObject Q_OBJECT public: - DesignDocument(ProjectStorageDependencies projectStorageDependencies, + DesignDocument(const QUrl &filePath, + ProjectStorageDependencies projectStorageDependencies, ExternalDependenciesInterface &externalDependencies); ~DesignDocument() override; diff --git a/src/plugins/qmldesigner/designercore/include/model.h b/src/plugins/qmldesigner/designercore/include/model.h index 8dd87baa5bf..85f129cdbce 100644 --- a/src/plugins/qmldesigner/designercore/include/model.h +++ b/src/plugins/qmldesigner/designercore/include/model.h @@ -166,6 +166,7 @@ public: NodeMetaInfo qtQmlConnectionsMetaInfo() const; NodeMetaInfo qtQmlModelsListModelMetaInfo() const; NodeMetaInfo qtQmlModelsListElementMetaInfo() const; + NodeMetaInfo qtQmlXmlListModelXmlListModelRoleMetaInfo() const; NodeMetaInfo qtQuick3DBakedLightmapMetaInfo() const; NodeMetaInfo qtQuick3DDefaultMaterialMetaInfo() const; NodeMetaInfo qtQuick3DDirectionalLightMetaInfo() const; diff --git a/src/plugins/qmldesigner/designercore/include/nodemetainfo.h b/src/plugins/qmldesigner/designercore/include/nodemetainfo.h index 53c755ddc88..23110175c1b 100644 --- a/src/plugins/qmldesigner/designercore/include/nodemetainfo.h +++ b/src/plugins/qmldesigner/designercore/include/nodemetainfo.h @@ -167,6 +167,7 @@ public: bool isQtMultimediaSoundEffect() const; bool isQtObject() const; bool isQtQmlConnections() const; + bool isQtQmlModelsListElement() const; bool isQtQuick3DBakedLightmap() const; bool isQtQuick3DBuffer() const; bool isQtQuick3DCamera() const; @@ -176,7 +177,6 @@ public: bool isQtQuick3DInstanceList() const; bool isQtQuick3DInstanceListEntry() const; bool isQtQuick3DLight() const; - bool isQtQuickListElement() const; bool isQtQuickListModel() const; bool isQtQuickListView() const; bool isQtQuick3DMaterial() const; diff --git a/src/plugins/qmldesigner/designercore/instances/nodeinstanceview.cpp b/src/plugins/qmldesigner/designercore/instances/nodeinstanceview.cpp index 33a100f7b20..26f2687c233 100644 --- a/src/plugins/qmldesigner/designercore/instances/nodeinstanceview.cpp +++ b/src/plugins/qmldesigner/designercore/instances/nodeinstanceview.cpp @@ -64,6 +64,8 @@ #include #include +#include + #include #include @@ -205,22 +207,17 @@ NodeInstanceView::~NodeInstanceView() static bool isSkippedRootNode(const ModelNode &node) { - static const PropertyNameList skipList({"Qt.ListModel", "QtQuick.ListModel", "Qt.ListModel", "QtQuick.ListModel"}); - - if (skipList.contains(node.type())) - return true; - - return false; + return node.metaInfo().isQtQuickListModel(); } static bool isSkippedNode(const ModelNode &node) { - static const PropertyNameList skipList({"QtQuick.XmlRole", "Qt.XmlRole", "QtQuick.ListElement", "Qt.ListElement"}); + auto model = node.model(); - if (skipList.contains(node.type())) - return true; + auto listElement = model->qtQmlModelsListElementMetaInfo(); + auto xmlRole = model->qtQmlXmlListModelXmlListModelRoleMetaInfo(); - return false; + return node.metaInfo().isBasedOn(listElement, xmlRole); } static bool parentTakesOverRendering(const ModelNode &modelNode) @@ -644,7 +641,7 @@ void NodeInstanceView::auxiliaryDataChanged(const ModelNode &node, TypeName(), key.type}; m_nodeInstanceServer->changeAuxiliaryValues({{container}}); - }; + } break; case AuxiliaryDataType::NodeInstanceAuxiliary: @@ -656,7 +653,7 @@ void NodeInstanceView::auxiliaryDataChanged(const ModelNode &node, TypeName(), key.type}; m_nodeInstanceServer->changeAuxiliaryValues({{container}}); - }; + } break; case AuxiliaryDataType::NodeInstancePropertyOverwrite: @@ -991,6 +988,8 @@ QRectF NodeInstanceView::sceneRect() const return {}; } +namespace { + QList filterNodesForSkipItems(const QList &nodeList) { QList filteredNodeList; @@ -1003,14 +1002,12 @@ QList filterNodesForSkipItems(const QList &nodeList) return filteredNodeList; } -namespace { bool shouldSendAuxiliary(const AuxiliaryDataKey &key) { return key.type == AuxiliaryDataType::NodeInstancePropertyOverwrite || key.type == AuxiliaryDataType::NodeInstanceAuxiliary || key == invisibleProperty || key == lockedProperty; } -} // namespace bool parentIsBehavior(ModelNode node) { @@ -1024,6 +1021,31 @@ bool parentIsBehavior(ModelNode node) return false; } +TypeName createQualifiedTypeName(const ModelNode &node) +{ + if (!node) + return {}; + +#ifdef QDS_USE_PROJECTSTORAGE + auto model = node.model(); + auto exportedTypes = node.metaInfo().exportedTypeNamesForSourceId(model->fileUrlSourceId()); + if (exportedTypes.size()) { + const auto &exportedType = exportedTypes.front(); + Utils::PathString typeName = model->projectStorage()->moduleName(exportedType.moduleId); + typeName += '/'; + typeName += exportedType.name; + + return typeName.toQByteArray(); + } + + return {}; +#else + return node.type(); +#endif +} + +} // namespace + CreateSceneCommand NodeInstanceView::createCreateSceneCommand() { QList nodeList = allModelNodes(); @@ -1079,8 +1101,9 @@ CreateSceneCommand NodeInstanceView::createCreateSceneCommand() nodeFlags |= InstanceContainer::ParentTakesOverRendering; const auto modelNode = instance.modelNode(); + InstanceContainer container(instance.instanceId(), - modelNode.type(), + createQualifiedTypeName(modelNode), modelNode.majorVersion(), modelNode.minorVersion(), ModelUtils::componentFilePath(modelNode), @@ -1243,7 +1266,7 @@ CreateInstancesCommand NodeInstanceView::createCreateInstancesCommand(const QLis const auto modelNode = instance.modelNode(); InstanceContainer container(instance.instanceId(), - modelNode.type(), + createQualifiedTypeName(modelNode), modelNode.majorVersion(), modelNode.minorVersion(), ModelUtils::componentFilePath(modelNode), @@ -1850,7 +1873,7 @@ QVariant NodeInstanceView::previewImageDataForImageNode(const ModelNode &modelNo ModelNodePreviewImageData imageData; imageData.id = modelNode.id(); - imageData.type = QString::fromLatin1(modelNode.type()); + imageData.type = QString::fromUtf8(createQualifiedTypeName(modelNode)); const double ratio = m_externalDependencies.formEditorDevicePixelRatio(); if (imageSource.isEmpty() && modelNode.metaInfo().isQtQuick3DTexture()) { @@ -1958,7 +1981,7 @@ QVariant NodeInstanceView::previewImageDataForGenericNode(const ModelNode &model if (m_imageDataMap.contains(id)) { imageData = m_imageDataMap[id]; } else { - imageData.type = QString::fromLatin1(modelNode.type()); + imageData.type = QString::fromLatin1(createQualifiedTypeName(modelNode)); imageData.id = id; m_imageDataMap.insert(id, imageData); } diff --git a/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp b/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp index 45da52254d0..845a1db1e52 100644 --- a/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp +++ b/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp @@ -2766,7 +2766,7 @@ bool NodeMetaInfo::isQtQuick3DLight() const } } -bool NodeMetaInfo::isQtQuickListElement() const +bool NodeMetaInfo::isQtQmlModelsListElement() const { if constexpr (useProjectStorage()) { using namespace Storage::Info; diff --git a/src/plugins/qmldesigner/designercore/model/model.cpp b/src/plugins/qmldesigner/designercore/model/model.cpp index 714480991d0..4f0bfba1ced 100644 --- a/src/plugins/qmldesigner/designercore/model/model.cpp +++ b/src/plugins/qmldesigner/designercore/model/model.cpp @@ -2210,6 +2210,16 @@ NodeMetaInfo Model::qtQmlModelsListElementMetaInfo() const } } +NodeMetaInfo Model::qtQmlXmlListModelXmlListModelRoleMetaInfo() const +{ + if constexpr (useProjectStorage()) { + using namespace Storage::Info; + return createNodeMetaInfo(); + } else { + return metaInfo("QtQml.XmlListModel.XmlListModelRole"); + } +} + NodeMetaInfo Model::qmlQtObjectMetaInfo() const { if constexpr (useProjectStorage()) { diff --git a/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp b/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp index 885868ddd00..1c1aba5feb9 100644 --- a/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp +++ b/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp @@ -501,7 +501,7 @@ public: if (!propertyMetaInfo.isValid()) { const bool isAttached = !propertyName.isEmpty() && propertyName[0].isUpper(); // Only list elements might have unknown properties. - if (!node.metaInfo().isQtQuickListElement() && !isAttached) { + if (!node.metaInfo().isQtQmlModelsListElement() && !isAttached) { qCInfo(texttomodelMergerLog) << Q_FUNC_INFO << "\nUnknown property" << propertyPrefix + QLatin1Char('.') + toString(propertyId) << "on line" diff --git a/src/plugins/qmldesigner/designercore/projectstorage/commontypecache.h b/src/plugins/qmldesigner/designercore/projectstorage/commontypecache.h index 03c25dfac79..35658c005f1 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/commontypecache.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/commontypecache.h @@ -91,6 +91,7 @@ inline constexpr char QtMultimedia[] = "QtMultimedia"; inline constexpr char QtObject[] = "QtObject"; inline constexpr char QtQml[] = "QtQml"; inline constexpr char QtQml_Models[] = "QtQml.Models"; +inline constexpr char QtQml_XmlListModel[] = "QtQml.XmlListModel"; inline constexpr char QtQuick3D[] = "QtQuick3D"; inline constexpr char QtQuick3D_Particles3D[] = "QtQuick3D.Particles3D"; inline constexpr char QtQuick3D_Particles3D_cppnative[] = "QtQuick3D.Particles3D-cppnative"; @@ -131,6 +132,7 @@ inline constexpr char Transition[] = "Transition"; inline constexpr char UIntType[] = "uint"; inline constexpr char View3D[] = "View3D"; inline constexpr char Window[] = "Window"; +inline constexpr char XmlListModelRole[] = "XmlListModelRole"; inline constexpr char color[] = "color"; inline constexpr char date[] = "date"; inline constexpr char font[] = "font"; @@ -176,6 +178,7 @@ class CommonTypeCache CacheType, CacheType, CacheType, + CacheType, CacheType, CacheType, CacheType, diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h index 7188d0e814e..83e11f952bc 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h @@ -168,7 +168,7 @@ public: return moduleId; } - Utils::SmallString moduleName(ModuleId moduleId) const + Utils::SmallString moduleName(ModuleId moduleId) const override { using NanotraceHR::keyValue; NanotraceHR::Tracer tracer{"get module name"_t, @@ -3278,8 +3278,9 @@ private: auto update = [&](const TypeWithDefaultPropertyView &view, const Storage::Synchronization::Type &value) { using NanotraceHR::keyValue; - NanotraceHR::Tracer tracer{"reset default properties by update"_t, + NanotraceHR::Tracer tracer{"synchronize default properties by update"_t, projectStorageCategory(), + keyValue("type id", value.typeId), keyValue("value", value), keyValue("view", view)}; @@ -3294,7 +3295,8 @@ private: updateDefaultPropertyIdStatement.write(value.typeId, valueDefaultPropertyId); - tracer.end(keyValue("updated", "yes")); + tracer.end(keyValue("updated", "yes"), + keyValue("default property id", valueDefaultPropertyId)); return Sqlite::UpdateChange::Update; }; @@ -3324,6 +3326,7 @@ private: using NanotraceHR::keyValue; NanotraceHR::Tracer tracer{"reset changed default properties by update"_t, projectStorageCategory(), + keyValue("type id", value.typeId), keyValue("value", value), keyValue("view", view)}; diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h index edf35f745cb..13a40695bf9 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h @@ -32,6 +32,7 @@ public: virtual void removeObserver(ProjectStorageObserver *observer) = 0; virtual ModuleId moduleId(::Utils::SmallStringView name) const = 0; + virtual Utils::SmallString moduleName(ModuleId moduleId) const = 0; virtual std::optional propertyDeclaration(PropertyDeclarationId propertyDeclarationId) const = 0; virtual TypeId typeId(ModuleId moduleId, diff --git a/src/plugins/qmldesigner/documentmanager.cpp b/src/plugins/qmldesigner/documentmanager.cpp index 75830d19b3d..b94de22c289 100644 --- a/src/plugins/qmldesigner/documentmanager.cpp +++ b/src/plugins/qmldesigner/documentmanager.cpp @@ -230,7 +230,9 @@ void DocumentManager::setCurrentDesignDocument(Core::IEditor *editor) auto found = m_designDocuments.find(editor); if (found == m_designDocuments.end()) { auto &inserted = m_designDocuments[editor] = std::make_unique( - m_projectManager.projectStorageDependencies(), m_externalDependencies); + editor->document()->filePath().toString(), + m_projectManager.projectStorageDependencies(), + m_externalDependencies); m_currentDesignDocument = inserted.get(); m_currentDesignDocument->setEditor(editor); } else { diff --git a/tests/unit/tests/mocks/projectstoragemock.cpp b/tests/unit/tests/mocks/projectstoragemock.cpp index 27e5c152d21..48357ff3e63 100644 --- a/tests/unit/tests/mocks/projectstoragemock.cpp +++ b/tests/unit/tests/mocks/projectstoragemock.cpp @@ -51,6 +51,7 @@ ModuleId ProjectStorageMock::createModule(Utils::SmallStringView moduleName) incrementBasicId(moduleId); ON_CALL(*this, moduleId(Eq(moduleName))).WillByDefault(Return(moduleId)); + ON_CALL(*this, moduleName(Eq(moduleId))).WillByDefault(Return(moduleName)); ON_CALL(*this, fetchModuleIdUnguarded(Eq(moduleName))).WillByDefault(Return(moduleId)); return moduleId; @@ -122,6 +123,14 @@ void ProjectStorageMock::addExportedTypeName(QmlDesigner::TypeId typeId, exportedTypeName[typeId].emplace_back(moduleId, typeName); } +void ProjectStorageMock::addExportedTypeNameBySourceId(QmlDesigner::TypeId typeId, + QmlDesigner::ModuleId moduleId, + Utils::SmallStringView typeName, + QmlDesigner::SourceId sourceId) +{ + exportedTypeNameBySourceId[{typeId, sourceId}].emplace_back(moduleId, typeName); +} + void ProjectStorageMock::removeExportedTypeName(QmlDesigner::TypeId typeId, QmlDesigner::ModuleId moduleId, Utils::SmallStringView typeName) @@ -364,6 +373,10 @@ ProjectStorageMock::ProjectStorageMock() ON_CALL(*this, exportedTypeNames(_)).WillByDefault([&](TypeId id) { return exportedTypeName[id]; }); + + ON_CALL(*this, exportedTypeNames(_, _)).WillByDefault([&](TypeId typeId, SourceId sourceId) { + return exportedTypeNameBySourceId[{typeId, sourceId}]; + }); } void ProjectStorageMock::setupQtQuick() diff --git a/tests/unit/tests/mocks/projectstoragemock.h b/tests/unit/tests/mocks/projectstoragemock.h index ed370bde96f..a68e37c2a15 100644 --- a/tests/unit/tests/mocks/projectstoragemock.h +++ b/tests/unit/tests/mocks/projectstoragemock.h @@ -46,6 +46,11 @@ public: QmlDesigner::ModuleId moduleId, Utils::SmallStringView typeName); + void addExportedTypeNameBySourceId(QmlDesigner::TypeId typeId, + QmlDesigner::ModuleId moduleId, + Utils::SmallStringView typeName, + QmlDesigner::SourceId sourceId); + void removeExportedTypeName(QmlDesigner::TypeId typeId, QmlDesigner::ModuleId moduleId, Utils::SmallStringView typeName); @@ -122,6 +127,7 @@ public: MOCK_METHOD(void, removeObserver, (QmlDesigner::ProjectStorageObserver *), (override)); MOCK_METHOD(QmlDesigner::ModuleId, moduleId, (::Utils::SmallStringView), (const, override)); + MOCK_METHOD(Utils::SmallString, moduleName, (QmlDesigner::ModuleId), (const, override)); MOCK_METHOD(std::optional, propertyDeclaration, @@ -331,6 +337,8 @@ public: QmlDesigner::Storage::Info::CommonTypeCache typeCache{*this}; std::map exportedTypeName; + std::map, QmlDesigner::Storage::Info::ExportedTypeNames> + exportedTypeNameBySourceId; }; class ProjectStorageMockWithQtQtuick : public ProjectStorageMock diff --git a/tests/unit/tests/unittests/model/modelutils-test.cpp b/tests/unit/tests/unittests/model/modelutils-test.cpp index c7a70cfbdb3..5a9e63b60da 100644 --- a/tests/unit/tests/unittests/model/modelutils-test.cpp +++ b/tests/unit/tests/unittests/model/modelutils-test.cpp @@ -141,7 +141,7 @@ TEST_F(ModelUtils, find_lowest_common_ancestor_when_one_of_the_nodes_is_parent) ASSERT_THAT(commonAncestor, parentNode); } -TEST_F(ModelUtils, lowest_common_ancestor_for_uncle_and_nephew_should_return_the_grandFather) +TEST_F(ModelUtils, lowest_common_ancestor_for_uncle_and_nephew_should_return_the_grandfather) { auto grandFatherNode = model.createModelNode("Item"); auto fatherNode = model.createModelNode("Item");