Skip to content

Commit 14d04ab

Browse files
authored
Fix miscellaneous issues and warnings (#652)
- Binding loop warning in WorldStats - ImageDisplay warning stating that the image source is invalid - This happened because the provider is being initialized in LoadConfig which runs after the QML has been loaded. The solution was to move it to the constructor. However, it depends on a unique name obtained from the QML, which causes a chicken and egg problem. To fix this, the QML now calls a member function of ImageDisplay to register the image provider with a unique name. --------- Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
1 parent d110e5b commit 14d04ab

File tree

4 files changed

+32
-12
lines changed

4 files changed

+32
-12
lines changed

src/plugins/image_display/ImageDisplay.cc

+19-7
Original file line numberDiff line numberDiff line change
@@ -51,19 +51,33 @@ class ImageDisplay::Implementation
5151

5252
/// \brief To provide images for QML.
5353
public: ImageProvider *provider{nullptr};
54+
55+
/// \brief Holds the provider name unique to this plugin instance
56+
public: QString providerName;
5457
};
5558

5659
/////////////////////////////////////////////////
5760
ImageDisplay::ImageDisplay()
5861
: dataPtr(gz::utils::MakeUniqueImpl<Implementation>())
5962
{
63+
this->dataPtr->provider = new ImageProvider();
6064
}
6165

6266
/////////////////////////////////////////////////
6367
ImageDisplay::~ImageDisplay()
6468
{
65-
App()->Engine()->removeImageProvider(
66-
this->CardItem()->objectName() + "imagedisplay");
69+
App()->Engine()->removeImageProvider(this->ImageProviderName());
70+
}
71+
72+
void ImageDisplay::RegisterImageProvider(const QString &_uniqueName)
73+
{
74+
this->dataPtr->providerName = _uniqueName;
75+
App()->Engine()->addImageProvider(_uniqueName,
76+
this->dataPtr->provider);
77+
}
78+
79+
QString ImageDisplay::ImageProviderName() {
80+
return this->dataPtr->providerName;
6781
}
6882

6983
/////////////////////////////////////////////////
@@ -95,13 +109,11 @@ void ImageDisplay::LoadConfig(const tinyxml2::XMLElement *_pluginElem)
95109
this->PluginItem()->setProperty("showPicker", topicPicker);
96110

97111
if (!topic.empty())
98-
this->OnTopic(QString::fromStdString(topic));
112+
{
113+
this->SetTopicList({QString::fromStdString(topic)});
114+
}
99115
else
100116
this->OnRefresh();
101-
102-
this->dataPtr->provider = new ImageProvider();
103-
App()->Engine()->addImageProvider(
104-
this->CardItem()->objectName() + "imagedisplay", this->dataPtr->provider);
105117
}
106118

107119
/////////////////////////////////////////////////

src/plugins/image_display/ImageDisplay.hh

+11-4
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@
1818
#ifndef GZ_GUI_PLUGINS_IMAGEDISPLAY_HH_
1919
#define GZ_GUI_PLUGINS_IMAGEDISPLAY_HH_
2020

21-
#include <algorithm>
22-
#include <memory>
21+
#include <QImage>
22+
#include <QString>
23+
#include <QStringList>
2324
#include <QQuickImageProvider>
2425

2526
#include <gz/msgs/image.pb.h>
@@ -53,8 +54,7 @@ namespace gz::gui::plugins
5354
if (!this->img.isNull())
5455
{
5556
// Must return a copy
56-
QImage copy(this->img);
57-
return copy;
57+
return this->img;
5858
}
5959

6060
// Placeholder in case we have no image yet
@@ -115,6 +115,13 @@ namespace gz::gui::plugins
115115
/// \param[in] _topicList Message type
116116
public: Q_INVOKABLE void SetTopicList(const QStringList &_topicList);
117117

118+
/// \brief Register the image provider with the given name
119+
/// \param[in] _uniqueName Unique name for the provider to be registered
120+
public: Q_INVOKABLE void RegisterImageProvider(const QString &_uniqueName);
121+
122+
/// \brief Get the provider name unique to this plugin instance
123+
public: Q_INVOKABLE QString ImageProviderName();
124+
118125
/// \brief Notify that topic list has changed
119126
signals: void TopicListChanged();
120127

src/plugins/image_display/ImageDisplay.qml

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ Rectangle {
4444
return;
4545

4646
uniqueName = parent.card().objectName + "imagedisplay";
47+
ImageDisplay.RegisterImageProvider(uniqueName);
4748
image.reload();
4849
}
4950

src/plugins/world_stats/WorldStats.qml

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ Rectangle {
9393
}
9494
PropertyChanges {
9595
target: hideButton
96-
x: worldStats.width - hideToolButton.width - compactLabel.width - 10
96+
x: worldStats.width - hideToolButton.width - compactLabel.implicitWidth - 10
9797
}
9898
PropertyChanges {
9999
target: compactLabel

0 commit comments

Comments
 (0)