From 2a125148efd13fc611ba243b2c8717cd7042d084 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 2 May 2023 23:14:32 -0500 Subject: [PATCH 1/6] Speed up Resource Spawner load time by fetching model list asynchronously (#1962) The main reason that the Resource Spawner took a long time to load is that it tried to fetch the list of all available models on Fuel instead of just the selected owner. And it did that while blocking the Qt thread, so the user was unable to interact with the GUI while the model list was being fetched. The approach I've taken is to only fetch the list of models for the default owner ("openrobotics") and allow users to add/remove any owner they want. The fetching is also done in a separate thread so as to not block the GUI. --------- Signed-off-by: Addisu Z. Taddese --- src/CMakeLists.txt | 6 +- .../resource_spawner/ResourceSpawner.cc | 294 ++++++++++++------ .../resource_spawner/ResourceSpawner.hh | 53 +++- .../resource_spawner/ResourceSpawner.qml | 216 ++++++++----- src/gz.cc | 5 + 5 files changed, 398 insertions(+), 176 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6c52f7d832..d6c94cccfe 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -37,13 +37,17 @@ ign_add_component(ign cmd/ModelCommandAPI.cc GET_TARGET_NAME ign_lib_target) target_link_libraries(${ign_lib_target} - PRIVATE + PUBLIC ${PROJECT_LIBRARY_TARGET_NAME} ignition-common${IGN_COMMON_VER}::ignition-common${IGN_COMMON_VER} ignition-gazebo${PROJECT_VERSION_MAJOR} ignition-gazebo${PROJECT_VERSION_MAJOR}-gui ) +# Executable target that runs the GUI without ruby for debugging purposes. +add_executable(runGui gz.cc) +target_link_libraries(runGui PRIVATE ${ign_lib_target}) + set (sources Barrier.cc Conversions.cc diff --git a/src/gui/plugins/resource_spawner/ResourceSpawner.cc b/src/gui/plugins/resource_spawner/ResourceSpawner.cc index 1db342258a..8d89e3aff2 100644 --- a/src/gui/plugins/resource_spawner/ResourceSpawner.cc +++ b/src/gui/plugins/resource_spawner/ResourceSpawner.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -40,6 +41,9 @@ #include "gz/sim/gui/GuiEvents.hh" + +Q_DECLARE_METATYPE(ignition::gazebo::Resource) + namespace ignition::gazebo { class ResourceSpawnerPrivate @@ -70,9 +74,34 @@ namespace ignition::gazebo /// \brief Holds all of the relevant data used by `DisplayData()` in order /// to filter and sort the displayed resources as desired by the user. public: Display displayData; + + /// \brief The list of Fuel servers to download from. + public: std::vector servers; + + /// \brief Data structure to hold relevant bits for a worker thread that + /// fetches the list of recources available for an owner on Fuel. + struct FetchResourceListWorker + { + /// \brief Thread that runs the worker + std::thread thread; + /// \brief Flag to notify the worker that it needs to stop. This could be + /// when an owner is removed or when the program exits. + std::atomic stopDownloading{false}; + /// \brief The workers own Fuel client to avoid synchronization. + fuel_tools::FuelClient fuelClient; + }; + + /// \brief Holds a map from owner to the associated resource list worker. + public: std::unordered_map fetchResourceListWorkers; }; } +namespace { + +// Default owner to be fetched from Fuel. This owner cannot be removed. +constexpr const char *kDefaultOwner = "openrobotics"; +} using namespace ignition; using namespace ignition::gazebo; @@ -86,15 +115,27 @@ void PathModel::AddPath(const std::string &_path) { IGN_PROFILE_THREAD_NAME("Qt thread"); IGN_PROFILE("PathModel::AddPath"); - QStandardItem *parentItem{nullptr}; - - parentItem = this->invisibleRootItem(); - auto localModel = new QStandardItem(QString::fromStdString(_path)); localModel->setData(QString::fromStdString(_path), this->roleNames().key("path")); - parentItem->appendRow(localModel); + this->appendRow(localModel); +} + +///////////////////////////////////////////////// +void PathModel::RemovePath(const std::string &_path) +{ + IGN_PROFILE_THREAD_NAME("Qt thread"); + IGN_PROFILE("PathModel::RemovePath"); + QString qPath = QString::fromStdString(_path); + for (int i = 0; i < this->rowCount(); ++i) + { + if (this->data(this->index(i, 0)) == qPath) + { + this->removeRow(i); + break; + } + } } ///////////////////////////////////////////////// @@ -114,14 +155,9 @@ ResourceModel::ResourceModel() : QStandardItemModel() ///////////////////////////////////////////////// void ResourceModel::Clear() { - QStandardItem *parentItem{nullptr}; - parentItem = this->invisibleRootItem(); - - while (parentItem->rowCount() > 0) - { - parentItem->removeRow(0); - } + this->clear(); this->gridIndex = 0; + emit sizeChanged(); } ///////////////////////////////////////////////// @@ -132,13 +168,10 @@ void ResourceModel::AddResources(std::vector &_resources) } ///////////////////////////////////////////////// -void ResourceModel::AddResource(Resource &_resource) +void ResourceModel::AddResource(const Resource &_resource) { IGN_PROFILE_THREAD_NAME("Qt thread"); IGN_PROFILE("GridModel::AddResource"); - QStandardItem *parentItem{nullptr}; - - parentItem = this->invisibleRootItem(); auto resource = new QStandardItem(QString::fromStdString(_resource.name)); resource->setData(_resource.isFuel, @@ -166,8 +199,9 @@ void ResourceModel::AddResource(Resource &_resource) this->roleNames().key("index")); this->gridIndex++; } + emit sizeChanged(); - parentItem->appendRow(resource); + this->appendRow(resource); } ///////////////////////////////////////////////// @@ -209,6 +243,7 @@ ResourceSpawner::ResourceSpawner() : gz::gui::Plugin(), dataPtr(std::make_unique()) { + qRegisterMetaType(); gz::gui::App()->Engine()->rootContext()->setContextProperty( "ResourceList", &this->dataPtr->resourceModel); gz::gui::App()->Engine()->rootContext()->setContextProperty( @@ -217,10 +252,45 @@ ResourceSpawner::ResourceSpawner() "OwnerList", &this->dataPtr->ownerModel); this->dataPtr->fuelClient = std::make_unique(); + + auto servers = this->dataPtr->fuelClient->Config().Servers(); + // Since the ign->gz rename, `servers` here returns two items for the + // canonical Fuel server: fuel.ignitionrobotics.org and fuel.gazebosim.org. + // For the purposes of the ResourceSpawner, these will be treated as the same + // and we will remove the ignitionrobotics server here. + auto urlIs = [](const std::string &_url) + { + return [_url](const fuel_tools::ServerConfig &_server) + { return _server.Url().Str() == _url; }; + }; + + auto ignIt = std::find_if(servers.begin(), servers.end(), + urlIs("https://fuel.ignitionrobotics.org")); + if (ignIt != servers.end()) + { + auto gzsimIt = std::find_if(servers.begin(), servers.end(), + urlIs("https://fuel.gazebosim.org")); + if (gzsimIt != servers.end()) + { + servers.erase(ignIt); + } + } + + this->dataPtr->servers = servers; } ///////////////////////////////////////////////// -ResourceSpawner::~ResourceSpawner() = default; +ResourceSpawner::~ResourceSpawner() +{ + for (auto &workers : this->dataPtr->fetchResourceListWorkers) + { + workers.second.stopDownloading = true; + if (workers.second.thread.joinable()) + { + workers.second.thread.join(); + } + } +} ///////////////////////////////////////////////// void ResourceSpawner::SetThumbnail(const std::string &_thumbnailPath, @@ -330,7 +400,7 @@ std::vector ResourceSpawner::FuelResources(const std::string &_owner) if (this->dataPtr->ownerModelMap.find(_owner) != this->dataPtr->ownerModelMap.end()) { - for (Resource resource : this->dataPtr->ownerModelMap[_owner]) + for (const Resource &resource : this->dataPtr->ownerModelMap[_owner]) { fuelResources.push_back(resource); } @@ -549,85 +619,9 @@ void ResourceSpawner::LoadConfig(const tinyxml2::XMLElement *) this->AddPath(path); } - auto servers = this->dataPtr->fuelClient->Config().Servers(); - // Since the ign->gz rename, `servers` here returns two items for the - // canonical Fuel server: fuel.ignitionrobotics.org and fuel.gazebosim.org. - // For the purposes of the ResourceSpawner, these will be treated as the same - // and we will remove the ignitionrobotics server here. - auto urlIs = [](const std::string &_url) - { - return [_url](const fuel_tools::ServerConfig &_server) - { return _server.Url().Str() == _url; }; - }; - - auto ignIt = std::find_if(servers.begin(), servers.end(), - urlIs("https://fuel.ignitionrobotics.org")); - if (ignIt != servers.end()) - { - auto gzsimIt = std::find_if(servers.begin(), servers.end(), - urlIs("https://fuel.gazebosim.org")); - if (gzsimIt != servers.end()) - { - servers.erase(ignIt); - } - } - ignmsg << "Please wait... Loading models from Fuel.\n"; - - // Add notice for the user that fuel resources are being loaded - this->dataPtr->ownerModel.AddPath("Please wait... Loading models from Fuel."); - - // Pull in fuel models asynchronously - std::thread t([this, servers] - { - // A set isn't necessary to keep track of the owners, but it - // maintains alphabetical order - std::set ownerSet; - for (auto const &server : servers) - { - std::vector models; - for (auto iter = this->dataPtr->fuelClient->Models(server); iter; ++iter) - { - models.push_back(iter->Identification()); - } - - // Create each fuel resource and add them to the ownerModelMap - for (const auto &id : models) - { - Resource resource; - resource.name = id.Name(); - resource.isFuel = true; - resource.isDownloaded = false; - resource.owner = id.Owner(); - resource.sdfPath = id.UniqueName(); - std::string path; - - // If the resource is cached, we can go ahead and populate the - // respective information - if (this->dataPtr->fuelClient->CachedModel( - common::URI(id.UniqueName()), path)) - { - resource.isDownloaded = true; - resource.sdfPath = common::joinPaths(path, "model.sdf"); - std::string thumbnailPath = common::joinPaths(path, "thumbnails"); - this->SetThumbnail(thumbnailPath, resource); - } - ownerSet.insert(id.Owner()); - this->dataPtr->ownerModelMap[id.Owner()].push_back(resource); - } - } - - // Clear the loading message - this->dataPtr->ownerModel.clear(); - - // Add all unique owners to the owner model - for (const auto &resource : ownerSet) - { - this->dataPtr->ownerModel.AddPath(resource); - } - ignmsg << "Fuel resources loaded.\n"; - }); - t.detach(); + this->dataPtr->ownerModel.AddPath(kDefaultOwner); + RunFetchResourceListThread(kDefaultOwner); } ///////////////////////////////////////////////// @@ -653,6 +647,112 @@ void ResourceSpawner::OnResourceSpawn(const QString &_sdfPath) &event); } +///////////////////////////////////////////////// +void ResourceSpawner::UpdateOwnerListModel(Resource _resource) +{ + // If the resource is cached, we can go ahead and populate the + // respective information + std::string path; + if (this->dataPtr->fuelClient->CachedModel( + common::URI(_resource.sdfPath), path)) + { + _resource.isDownloaded = true; + _resource.sdfPath = common::joinPaths(path, "model.sdf"); + std::string thumbnailPath = common::joinPaths(path, "thumbnails"); + this->SetThumbnail(thumbnailPath, _resource); + } + + this->dataPtr->ownerModelMap[_resource.owner].push_back(_resource); + if (this->dataPtr->displayData.ownerPath == _resource.owner) + { + this->dataPtr->resourceModel.AddResource(_resource); + } +} + +///////////////////////////////////////////////// +bool ResourceSpawner::AddOwner(const QString &_owner) +{ + const std::string ownerString = _owner.toStdString(); + if (this->dataPtr->ownerModelMap.find(ownerString) != + this->dataPtr->ownerModelMap.end()) + { + QString errorMsg = QString("Owner %1 already added").arg(_owner); + emit resourceSpawnerError(errorMsg); + return false; + } + this->dataPtr->ownerModel.AddPath(ownerString); + RunFetchResourceListThread(ownerString); + return true; +} + +///////////////////////////////////////////////// +void ResourceSpawner::RemoveOwner(const QString &_owner) +{ + const std::string ownerString = _owner.toStdString(); + this->dataPtr->ownerModelMap.erase(ownerString); + this->dataPtr->ownerModel.RemovePath(ownerString); + this->dataPtr->fetchResourceListWorkers[ownerString].stopDownloading = true; +} + +///////////////////////////////////////////////// +bool ResourceSpawner::IsDefaultOwner(const QString &_owner) const +{ + return _owner.toStdString() == kDefaultOwner; +} + +///////////////////////////////////////////////// +void ResourceSpawner::RunFetchResourceListThread(const std::string &_owner) +{ + auto &worker = this->dataPtr->fetchResourceListWorkers[_owner]; + // If the owner had been deleted, we need to clean the previous thread and + // restart. + if (worker.thread.joinable()) + { + worker.stopDownloading = true; + worker.thread.join(); + } + + worker.stopDownloading = false; + + // Pull in fuel models asynchronously + this->dataPtr->fetchResourceListWorkers[_owner].thread = std::thread( + [this, owner = _owner, &worker] + { + int counter = 0; + for (auto const &server : this->dataPtr->servers) + { + fuel_tools::ModelIdentifier modelId; + modelId.SetServer(server); + modelId.SetOwner(owner); + for (auto iter = worker.fuelClient.Models(modelId, false); + iter; ++iter, ++counter) + { + if (worker.stopDownloading) + { + return; + } + auto id = iter->Identification(); + Resource resource; + resource.name = id.Name(); + resource.isFuel = true; + resource.isDownloaded = false; + resource.owner = id.Owner(); + resource.sdfPath = id.UniqueName(); + + QMetaObject::invokeMethod( + this, "UpdateOwnerListModel", Qt::QueuedConnection, + Q_ARG(ignition::gazebo::Resource, resource)); + } + } + if (counter == 0) + { + QString errorMsg = QString("No resources found for %1") + .arg(QString::fromStdString(owner)); + emit resourceSpawnerError(errorMsg); + } + }); +} + // Register this plugin IGNITION_ADD_PLUGIN(ResourceSpawner, gz::gui::Plugin) diff --git a/src/gui/plugins/resource_spawner/ResourceSpawner.hh b/src/gui/plugins/resource_spawner/ResourceSpawner.hh index 6a378cbe81..0cf31016f8 100644 --- a/src/gui/plugins/resource_spawner/ResourceSpawner.hh +++ b/src/gui/plugins/resource_spawner/ResourceSpawner.hh @@ -18,9 +18,12 @@ #ifndef GZ_GAZEBO_GUI_RESOURCE_SPAWNER_HH_ #define GZ_GAZEBO_GUI_RESOURCE_SPAWNER_HH_ +#include #include #include #include +#include +#include #include @@ -92,9 +95,13 @@ namespace gazebo /// \brief Destructor public: ~PathModel() override = default; - /// \brief Add a local model to the grid view. - /// param[in] _model The local model to be added - public slots: void AddPath(const std::string &_path); + /// \brief Add a path. + /// param[in] _path The path to be added. + public: void AddPath(const std::string &_path); + + /// \brief Remove a path. + /// param[in] _path The path to be removed. + public: void RemovePath(const std::string &_path); // Documentation inherited public: QHash roleNames() const override; @@ -106,6 +113,10 @@ namespace gazebo { Q_OBJECT + /// \brief Property used to display the total number of resources associated + /// with an owner. + Q_PROPERTY(int totalCount MEMBER gridIndex NOTIFY sizeChanged) + /// \brief Constructor public: explicit ResourceModel(); @@ -114,7 +125,7 @@ namespace gazebo /// \brief Add a resource to the grid view. /// param[in] _resource The local resource to be added - public: void AddResource(Resource &_resource); + public: void AddResource(const Resource &_resource); /// \brief Add a vector of resources to the grid view. /// param[in] _resource The vector of local resources to be added @@ -133,6 +144,9 @@ namespace gazebo // Documentation inherited public: QHash roleNames() const override; + /// \brief Signal used with the totalCount property + public: signals: void sizeChanged(); + // \brief Index to keep track of the position of each resource in the qml // grid, used primarily to access currently loaded resources for updates. public: int gridIndex = 0; @@ -237,6 +251,37 @@ namespace gazebo public: void SetThumbnail(const std::string &_thumbnailPath, Resource &_resource); + /// \brief Called form a download thread to update the GUI's list of + /// resources. + /// \param[in] _resource The resource fetched from Fuel. Note that it is + /// passed by value as a copy is necessary to update the resource if it's + /// cached. + public: Q_INVOKABLE void UpdateOwnerListModel( + ignition::gazebo::Resource _resource); + + /// \brief Add owner to the list of owners whose resources would be fetched + /// from Fuel. + /// \param[in] _owner Name of owner. + /// \return True if the owner was successfully added. + public: Q_INVOKABLE bool AddOwner(const QString &_owner); + + /// \brief Remove owner from the list of owners whose resources would be + /// fetched from Fuel. + /// \param[in] _owner Name of owner. + public: Q_INVOKABLE void RemoveOwner(const QString &_owner); + + /// \brief Determine if owner is the default owner + /// \param[in] _owner Name of owner. + public: Q_INVOKABLE bool IsDefaultOwner(const QString &_owner) const; + + /// \brief Signal emitted when an error is encountered regarding an owner + /// \param[in] _errorMsg Error message to be displayed. + signals: void resourceSpawnerError(const QString &_errorMsg); + + /// \brief Starts a thread that fetches the resources list for a given owner + /// \param[in] _owner Name of owner. + private: void RunFetchResourceListThread(const std::string &_owner); + /// \internal /// \brief Pointer to private data. private: std::unique_ptr dataPtr; diff --git a/src/gui/plugins/resource_spawner/ResourceSpawner.qml b/src/gui/plugins/resource_spawner/ResourceSpawner.qml index 51ac1a37e2..3ca3921ab5 100644 --- a/src/gui/plugins/resource_spawner/ResourceSpawner.qml +++ b/src/gui/plugins/resource_spawner/ResourceSpawner.qml @@ -98,14 +98,15 @@ Rectangle { border.color: "gray" border.width: 1 Layout.alignment: Qt.AlignLeft - Layout.preferredHeight: 25 + Layout.preferredHeight: 35 Layout.fillWidth: true + Layout.leftMargin: -border.width + Layout.rightMargin: -border.width Label { - topPadding: 2 - leftPadding: 5 + padding: 5 text: "Local resources" anchors.fill: parent - font.pointSize: 12 + font.pointSize: 14 } } TreeView { @@ -121,6 +122,7 @@ Rectangle { verticalScrollBarPolicy: Qt.ScrollBarAsNeeded headerVisible: false backgroundVisible: false + frameVisible: false headerDelegate: Rectangle { visible: false @@ -143,7 +145,7 @@ Rectangle { height: treeItemHeight } itemDelegate: Rectangle { - id: item + id: localItem color: styleData.selected ? Material.accent : (styleData.row % 2 == 0) ? evenColor : oddColor height: treeItemHeight @@ -188,7 +190,7 @@ Rectangle { ToolTip { visible: ma.containsMouse delay: 500 - y: item.z - 30 + y: localItem.z - 30 text: model === null ? "?" : model.path enter: null @@ -207,100 +209,136 @@ Rectangle { color: evenColor border.color: "gray" Layout.alignment: Qt.AlignLeft - Layout.preferredHeight: 25 + Layout.preferredHeight: 35 Layout.fillWidth: true + border.width: 1 + Layout.leftMargin: -border.width + Layout.rightMargin: -border.width + Layout.topMargin: -border.width Label { text: "Fuel resources" - topPadding: 2 - leftPadding: 5 + padding: 5 anchors.fill: parent - font.pointSize: 12 + font.pointSize: 14 } } - TreeView { - id: treeView2 + + ListView { + id: listView model: OwnerList - // For some reason, SingleSelection is not working - selectionMode: SelectionMode.MultiSelection - verticalScrollBarPolicy: Qt.ScrollBarAsNeeded - headerVisible: false - backgroundVisible: false - Layout.minimumWidth: 300 - Layout.alignment: Qt.AlignCenter Layout.fillWidth: true Layout.fillHeight: true + Layout.minimumWidth: 300 + clip: true - headerDelegate: Rectangle { - visible: false + ScrollBar.vertical: ScrollBar { + active: true; } - TableViewColumn - { - role: "name" - } + delegate: Rectangle { + id: fuelItem2 + color: ListView.view.currentIndex == index ? Material.accent : (index % 2 == 0) ? evenColor : oddColor + height: treeItemHeight + width: ListView.view.width + ListView.onAdd : { + ListView.view.currentIndex = index + } - selection: ItemSelectionModel { - model: OwnerList - } + ListView.onCurrentItemChanged: { + if (index >= 0) { + currentPath = model.path + ResourceSpawner.OnOwnerClicked(model.path) + ResourceSpawner.DisplayResources(); + treeView.selection.clearSelection() + gridView.currentIndex = -1 + } + } - style: TreeViewStyle { - indentation: 0 - rowDelegate: Rectangle { - id: row2 - color: Material.background - height: treeItemHeight + MouseArea { + anchors.fill: parent + onClicked: { + listView.currentIndex = index + } } - itemDelegate: Rectangle { - id: item - color: styleData.selected ? Material.accent : (styleData.row % 2 == 0) ? evenColor : oddColor - height: treeItemHeight - anchors.top: parent.top - anchors.right: parent.right + RowLayout { + anchors.fill: parent + anchors.leftMargin: 10 + anchors.rightMargin: 10 + clip: true Image { - id: dirIcon - source: styleData.selected ? "folder_open.png" : "folder_closed.png" - height: treeItemHeight * 0.6 - width: treeItemHeight * 0.6 - anchors.verticalCenter: parent.verticalCenter - anchors.left: parent.left + id: dirIcon2 + source: listView.currentIndex == index ? "folder_open.png" : "folder_closed.png" + Layout.preferredHeight: treeItemHeight * 0.6 + Layout.preferredWidth: treeItemHeight * 0.6 } Label { - text: (model === null) ? "" : model.path + text: model.path + Layout.fillWidth: true elide: Text.ElideMiddle font.pointSize: 12 - anchors.leftMargin: 1 - anchors.left: dirIcon.right - anchors.verticalCenter: parent.verticalCenter leftPadding: 2 } - MouseArea { - id: ma - anchors.fill: parent - propagateComposedEvents: true - hoverEnabled: true + Button { + // unicode for emdash (—) + text: "\u2014" + flat: true + Layout.fillHeight : true + Layout.preferredWidth: 30 + visible: !ResourceSpawner.IsDefaultOwner(model.path) + onClicked: { - ResourceSpawner.OnOwnerClicked(model.path) - ResourceSpawner.DisplayResources(); - treeView2.selection.select(styleData.index, ItemSelectionModel.ClearAndSelect) - treeView.selection.clearSelection() - currentPath = model.path - gridView.currentIndex = -1 - mouse.accepted = false + ResourceSpawner.RemoveOwner(model.path) } } + } + } + } - ToolTip { - visible: ma.containsMouse - delay: 500 - y: item.z - 30 - text: model === null ? - "?" : model.path - enter: null - exit: null + // Add owner button + Rectangle { + id: addOwnerBar + color: evenColor + Layout.minimumHeight: 50 + Layout.fillWidth: true + clip:true + RowLayout { + anchors.fill: parent + anchors.leftMargin: 10 + anchors.rightMargin: 10 + spacing: 10 + + TextField { + Layout.fillWidth: true + id: ownerInput + selectByMouse: true + color: Material.theme == Material.Light ? "black" : "white" + placeholderText: "Add owner" + function processInput() { + if (text != "" && ResourceSpawner.AddOwner(text)) { + text = "" + } + } + onAccepted: { + processInput(); + } + } + + RoundButton { + Material.background: Material.Green + contentItem: Label { + text: "+" + color: "white" + font.pointSize: 30 + horizontalAlignment: Text.AlignHCenter + verticalAlignment: Text.AlignVCenter + } + padding: 0 + onClicked: { + ownerInput.processInput() } } } @@ -322,6 +360,7 @@ Rectangle { RowLayout { id: rowLayout spacing: 7 + anchors.fill: parent Rectangle { color: "transparent" height: 25 @@ -354,10 +393,11 @@ Rectangle { } Rectangle { color: "transparent" - height: 50 + implicitHeight: sortComboBox.implicitHeight Layout.minimumWidth: 140 Layout.preferredWidth: (searchSortBar.width - 80) / 2 ComboBox { + id: sortComboBox anchors.fill: parent model: ListModel { id: cbItems @@ -379,9 +419,9 @@ Rectangle { Layout.fillWidth: true Layout.minimumWidth: 300 height: 40 - color: "transparent" + color: Material.accent Label { - text: currentPath + text: currentPath ? "Owner: " + currentPath + " (" + gridView.model.totalCount + ")" : "" font.pointSize: 12 elide: Text.ElideMiddle anchors.margins: 5 @@ -420,6 +460,8 @@ Rectangle { layer.effect: ElevationEffect { elevation: 6 } + border.width: 1 + border.color: "lightgray" } ColumnLayout { @@ -438,7 +480,7 @@ Rectangle { Layout.margins: 1 source: (model.isFuel && !model.isDownloaded) ? "DownloadToUse.png" : - (model.thumbnail == "" ? + (model.thumbnail === "" ? "NoThumbnail.png" : "file:" + model.thumbnail) fillMode: Image.PreserveAspectFit } @@ -470,6 +512,7 @@ Rectangle { modal: true focus: true title: "Note" + standardButtons: Dialog.Ok Rectangle { color: "transparent" anchors.fill: parent @@ -518,4 +561,29 @@ Rectangle { } } } + + // Dialog for error messages + Dialog { + id: messageDialog + width: 360 + height: 150 + parent: resourceSpawner.Window.window ? resourceSpawner.Window.window.contentItem : resourceSpawner + x: Math.round((parent.width - width) / 2) + y: Math.round((parent.height - height) / 2) + modal: true + focus: true + title: "Error" + standardButtons: Dialog.Ok + contentItem: Text { + text: "" + } + } + + Connections { + target: ResourceSpawner + onResourceSpawnerError : { + messageDialog.contentItem.text = _errorMsg + messageDialog.visible = true + } + } } diff --git a/src/gz.cc b/src/gz.cc index 91124c1ec4..5110d197b1 100644 --- a/src/gz.cc +++ b/src/gz.cc @@ -429,3 +429,8 @@ extern "C" IGNITION_GAZEBO_VISIBLE int runGui( return gz::sim::gui::runGui( argc, &argv, _guiConfig, _file, _waitGui); } + +int main(int argc, char* argv[]) +{ + return gazebo::gui::runGui(argc, argv, nullptr); +} From a6be7302e43f197ce731f81bc685e24923345c0e Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Tue, 9 May 2023 09:14:25 -0700 Subject: [PATCH 2/6] Small fixes to gz headers (#1985) * Include gz/msgs.hh instead of ignition * Install components.hh to gz/sim Signed-off-by: Steve Peters --- include/gz/sim/components/CMakeLists.txt | 2 +- include/gz/sim/gui/TmpIface.hh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/gz/sim/components/CMakeLists.txt b/include/gz/sim/components/CMakeLists.txt index 83f18ce69d..100786be0e 100644 --- a/include/gz/sim/components/CMakeLists.txt +++ b/include/gz/sim/components/CMakeLists.txt @@ -13,5 +13,5 @@ configure_file( install( FILES ${CMAKE_CURRENT_BINARY_DIR}/components.hh - DESTINATION ${IGN_INCLUDE_INSTALL_DIR_FULL}/ignition/${IGN_DESIGNATION} + DESTINATION ${IGN_INCLUDE_INSTALL_DIR_FULL}/gz/sim ) diff --git a/include/gz/sim/gui/TmpIface.hh b/include/gz/sim/gui/TmpIface.hh index 8709d3036d..ed54a7597b 100644 --- a/include/gz/sim/gui/TmpIface.hh +++ b/include/gz/sim/gui/TmpIface.hh @@ -21,7 +21,7 @@ #include #endif -#include +#include #include #include "gz/sim/Export.hh" From ea3415862819c0c7f447d4dba653ce8f10b1c66c Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Wed, 10 May 2023 08:30:53 -0500 Subject: [PATCH 3/6] Prepare for 3.15.0 Release (#1984) Signed-off-by: Addisu Z. Taddese Signed-off-by: Steve Peters Co-authored-by: Steve Peters --- CMakeLists.txt | 2 +- Changelog.md | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c5abddeba..ae902446a6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.10.2 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(ignition-gazebo3 VERSION 3.14.0) +project(ignition-gazebo3 VERSION 3.15.0) set (GZ_DISTRIBUTION "Citadel") #============================================================================ diff --git a/Changelog.md b/Changelog.md index 4873dea3d3..a994ba13b2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,69 @@ ## Gazebo Sim 3.x +### Gazebo Sim 3.15.0 (2023-05-08) + +1. Speed up Resource Spawner load time by fetching model list asynchronously + * [Pull request #1962](https://github.com/gazebosim/gz-sim/pull/1962) + +1. ign -> gz Migrate Ignition Headers : gz-sim + * [Pull request #1646](https://github.com/gazebosim/gz-sim/pull/1646) + * [Pull request #1967](https://github.com/gazebosim/gz-sim/pull/1967) + * [Pull request #1978](https://github.com/gazebosim/gz-sim/pull/1978) + * [Pull request #1983](https://github.com/gazebosim/gz-sim/pull/1983) + * [Pull request #1985](https://github.com/gazebosim/gz-sim/pull/1985) + +1. Infrastructure + * [Pull request #1940](https://github.com/gazebosim/gz-sim/pull/1940) + * [Pull request #1937](https://github.com/gazebosim/gz-sim/pull/1937) + +1. Backport portion of #1771 to fix command-line test + * [Pull request #1771](https://github.com/gazebosim/gz-sim/pull/1771) + +1. cmdsim.rb: fix ruby syntax + * [Pull request #1884](https://github.com/gazebosim/gz-sim/pull/1884) + +1. Fix loading wold with record topic + * [Pull request #1855](https://github.com/gazebosim/gz-sim/pull/1855) + +1. Remove duplicate Fuel server used by ResourceSpawner + * [Pull request #1830](https://github.com/gazebosim/gz-sim/pull/1830) + +1. Re-add namespace for GUI render event + * [Pull request #1826](https://github.com/gazebosim/gz-sim/pull/1826) + +1. Fix QML warnings regarding binding loops + * [Pull request #1829](https://github.com/gazebosim/gz-sim/pull/1829) + +1. Update documentation on `UpdateInfo::realTime` + * [Pull request #1817](https://github.com/gazebosim/gz-sim/pull/1817) + +1. Add jennuine as GUI codeowner + * [Pull request #1800](https://github.com/gazebosim/gz-sim/pull/1800) + +1. Remove plotIcon in Physics.qml for Component Inspector + * [Pull request #1658](https://github.com/gazebosim/gz-sim/pull/1658) + +1. Convert ignitionrobotics to gazebosim in tutorials + * [Pull request #1757](https://github.com/gazebosim/gz-sim/pull/1757) + * [Pull request #1758](https://github.com/gazebosim/gz-sim/pull/1758) + * [Pull request #1759](https://github.com/gazebosim/gz-sim/pull/1759) + * [Pull request #1760](https://github.com/gazebosim/gz-sim/pull/1760) + +1. Added collection name to About Dialog + * [Pull request #1756](https://github.com/gazebosim/gz-sim/pull/1756) + +1. Remove compiler warnings + * [Pull request #1753](https://github.com/gazebosim/gz-sim/pull/1753) + +1. Update examples to use gazebosim.org + * [Pull request #1749](https://github.com/gazebosim/gz-sim/pull/1749) + +1. Remove actors from screen when they are supposed to + * [Pull request #1699](https://github.com/gazebosim/gz-sim/pull/1699) + +1. Readd namespaces for Q_ARGS + * [Pull request #1670](https://github.com/gazebosim/gz-sim/pull/1670) + ### Gazebo Sim 3.14.0 (2022-08-17) 1. Change `CODEOWNERS` and maintainer to Michael From f0ef915bfa505b82cabce5af868b968825dc8c4c Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Wed, 10 May 2023 08:41:23 -0500 Subject: [PATCH 4/6] Update workflow triggers to avoid duplicates (#1988) Signed-off-by: Addisu Z. Taddese --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 744bfcff3b..438c601c92 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,10 @@ name: Ubuntu CI -on: [push, pull_request] +on: + pull_request: + push: + branches: + - 'ign-gazebo3' jobs: bionic-ci: From 3dd77a2ae3eccd40d5c32f3ec7a56c3664165049 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Tue, 23 May 2023 09:03:49 -0700 Subject: [PATCH 5/6] Enable GzWeb visualization of markers by republishing service requests on a topic (#1994) * Working on marker topic Signed-off-by: Nate Koenig * Fixed build Signed-off-by: Nate Koenig * Fixed test Signed-off-by: Nate Koenig * Cleanup Signed-off-by: Nate Koenig * codecheck Signed-off-by: Nate Koenig --------- Signed-off-by: Nate Koenig --- include/gz/sim/rendering/MarkerManager.hh | 1 + src/rendering/MarkerManager.cc | 7 ++ test/integration/CMakeLists.txt | 2 + test/integration/markers.cc | 114 ++++++++++++++++++++++ 4 files changed, 124 insertions(+) create mode 100644 test/integration/markers.cc diff --git a/include/gz/sim/rendering/MarkerManager.hh b/include/gz/sim/rendering/MarkerManager.hh index 5139505772..26c4b0d716 100644 --- a/include/gz/sim/rendering/MarkerManager.hh +++ b/include/gz/sim/rendering/MarkerManager.hh @@ -20,6 +20,7 @@ #include #include +#include #include #include "gz/rendering/RenderTypes.hh" diff --git a/src/rendering/MarkerManager.cc b/src/rendering/MarkerManager.cc index 3e06e7c935..7b61b267f2 100644 --- a/src/rendering/MarkerManager.cc +++ b/src/rendering/MarkerManager.cc @@ -119,6 +119,9 @@ class ignition::gazebo::MarkerManagerPrivate /// \brief Topic name for the marker service public: std::string topicName = "/marker"; + + /// \brief Topic that publishes marker updates. + public: gz::transport::Node::Publisher markerPub; }; ///////////////////////////////////////////////// @@ -189,6 +192,9 @@ bool MarkerManager::Init(const rendering::ScenePtr &_scene) << "_array service.\n"; } + this->dataPtr->markerPub = + this->dataPtr->node.Advertise(this->dataPtr->topicName); + return true; } @@ -215,6 +221,7 @@ void MarkerManagerPrivate::Update() markerIter != this->markerMsgs.end();) { this->ProcessMarkerMsg(*markerIter); + this->markerPub.Publish(*markerIter); this->markerMsgs.erase(markerIter++); } diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index f8335cfe0c..57ed3f3723 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -61,6 +61,7 @@ set(tests_needing_display camera_video_record_system.cc depth_camera.cc gpu_lidar.cc + markers.cc rgbd_camera.cc sensors_system.cc ) @@ -86,6 +87,7 @@ ign_build_tests(TYPE INTEGRATION ${tests} LIB_DEPS ${EXTRA_TEST_LIB_DEPS} + ${PROJECT_LIBRARY_TARGET_NAME}-rendering ) # For INTEGRATION_physics_system, we need to check what version of DART is diff --git a/test/integration/markers.cc b/test/integration/markers.cc new file mode 100644 index 0000000000..c4682d0b3b --- /dev/null +++ b/test/integration/markers.cc @@ -0,0 +1,114 @@ +/* + * Copyright (C) 2023 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include + +#include +#include +#include +#include +#include "gz/rendering/Scene.hh" +#include +#include + +#include "gz/sim/rendering/MarkerManager.hh" +#include "gz/sim/Server.hh" +#include "gz/sim/test_config.hh" + +#include "plugins/MockSystem.hh" +#include "../helpers/EnvTestFixture.hh" + +using namespace gz; +using namespace gz::sim; + +/// \brief Test MarkersTest system +class MarkersTest : public InternalFixture<::testing::Test> +{ +}; + +std::mutex mutex; +std::vector markerMsgs; + +///////////////////////////////////////////////// +void markerCb(const msgs::Marker &_msg) +{ + mutex.lock(); + markerMsgs.push_back(_msg); + mutex.unlock(); +} + +///////////////////////////////////////////////// +TEST_F(MarkersTest, MarkerPublisher) +{ + std::map params; + auto engine = ignition::rendering::engine("ogre2", params); + auto scene = engine->CreateScene("testscene"); + + gz::msgs::Marker markerMsg; + + // Function that Waits for a message to be received + auto wait = [&](std::size_t _size, gz::msgs::Marker &_markerMsg) { + for (int sleep = 0; sleep < 30; ++sleep) + { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + mutex.lock(); + bool received = markerMsgs.size() == _size; + mutex.unlock(); + + if (received) + break; + } + + mutex.lock(); + EXPECT_EQ(markerMsgs.size(), _size); + auto lastMsg = markerMsgs.back(); + EXPECT_EQ(_markerMsg.DebugString(), lastMsg.DebugString()); + mutex.unlock(); + }; + + + MarkerManager markerManager; + markerManager.Init(scene); + + // subscribe to marker topic + transport::Node node; + node.Subscribe("/marker", &markerCb); + + // Send a request, wait for a message + markerMsg.set_ns("default"); + markerMsg.set_id(0); + markerMsg.set_action(gz::msgs::Marker::ADD_MODIFY); + markerMsg.set_type(gz::msgs::Marker::SPHERE); + markerMsg.set_visibility(gz::msgs::Marker::GUI); + node.Request("/marker", markerMsg); + markerManager.Update(); + wait(1, markerMsg); + + // Update without a new message + markerManager.Update(); + + // Send another request, and check that there are two messages + markerMsg.set_ns("default2"); + markerMsg.set_id(1); + markerMsg.set_action(gz::msgs::Marker::ADD_MODIFY); + markerMsg.set_type(gz::msgs::Marker::BOX); + markerMsg.set_visibility(gz::msgs::Marker::GUI); + node.Request("/marker", markerMsg); + markerManager.Update(); + wait(2, markerMsg); +} From 1038a355866eb1003c8b370b01a673d431c51052 Mon Sep 17 00:00:00 2001 From: Henrique Barros Oliveira Date: Mon, 12 Jun 2023 22:11:57 +0200 Subject: [PATCH 6/6] Print an error message when trying to load SDF files that don't contain a `` (#1998) Signed-off-by: Henrique-BO --- src/Server.cc | 8 ++++++++ src/Server_TEST.cc | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/Server.cc b/src/Server.cc index deb622cf5d..51ad4b2e0c 100644 --- a/src/Server.cc +++ b/src/Server.cc @@ -182,6 +182,14 @@ Server::Server(const ServerConfig &_config) return; } + if (this->dataPtr->sdfRoot.WorldCount() == 0) + { + ignerr << "SDF file doesn't contain a world. " << + "If you wish to spawn a model, use the ResourceSpawner GUI plugin " << + "or the 'world//create' service.\n"; + return; + } + // Add record plugin if (_config.UseLogRecord()) { diff --git a/src/Server_TEST.cc b/src/Server_TEST.cc index a2ff63ae55..4a16d723db 100644 --- a/src/Server_TEST.cc +++ b/src/Server_TEST.cc @@ -1088,6 +1088,45 @@ TEST_P(ServerFixture, AddResourcePaths) } } +///////////////////////////////////////////////// +TEST_P(ServerFixture, SdfWithoutWorld) +{ + // Initialize logging + std::string logBasePath = common::joinPaths(PROJECT_BINARY_PATH, "tmp"); + common::setenv(IGN_HOMEDIR, logBasePath); + std::string path = common::uuid(); + ignLogInit(path, "test.log"); + + // Start server with model SDF file + ServerConfig serverConfig; + serverConfig.SetSdfFile(common::joinPaths(PROJECT_SOURCE_PATH, + "test", "worlds", "models", "sphere", "model.sdf")); + + gz::sim::Server server(serverConfig); + EXPECT_FALSE(server.Running()); + EXPECT_FALSE(*server.Running(0)); + + // Check error message in log file + std::string logFullPath = common::joinPaths(logBasePath, path, "test.log"); + std::ifstream ifs(logFullPath.c_str(), std::ios::in); + bool errFound = false; + while ((!errFound) && (!ifs.eof())) + { + std::string line; + std::getline(ifs, line); + std::string errString = "SDF file doesn't contain a world. "; + errString += "If you wish to spawn a model, "; + errString += "use the ResourceSpawner GUI plugin "; + errString += "or the 'world//create' service."; + errFound = (line.find(errString) != std::string::npos); + } + EXPECT_TRUE(errFound); + + // Stop logging + ignLogClose(); + common::removeAll(logBasePath); +} + // Run multiple times. We want to make sure that static globals don't cause // problems. INSTANTIATE_TEST_SUITE_P(ServerRepeat, ServerFixture, ::testing::Range(1, 2));