Skip to content

Commit

Permalink
Fix miscellaneous issues and warnings (#652)
Browse files Browse the repository at this point in the history
- 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 <[email protected]>
  • Loading branch information
azeey authored Jan 30, 2025
1 parent d110e5b commit 14d04ab
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 12 deletions.
26 changes: 19 additions & 7 deletions src/plugins/image_display/ImageDisplay.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,33 @@ class ImageDisplay::Implementation

/// \brief To provide images for QML.
public: ImageProvider *provider{nullptr};

/// \brief Holds the provider name unique to this plugin instance
public: QString providerName;
};

/////////////////////////////////////////////////
ImageDisplay::ImageDisplay()
: dataPtr(gz::utils::MakeUniqueImpl<Implementation>())
{
this->dataPtr->provider = new ImageProvider();
}

/////////////////////////////////////////////////
ImageDisplay::~ImageDisplay()
{
App()->Engine()->removeImageProvider(
this->CardItem()->objectName() + "imagedisplay");
App()->Engine()->removeImageProvider(this->ImageProviderName());
}

void ImageDisplay::RegisterImageProvider(const QString &_uniqueName)
{
this->dataPtr->providerName = _uniqueName;
App()->Engine()->addImageProvider(_uniqueName,
this->dataPtr->provider);
}

QString ImageDisplay::ImageProviderName() {
return this->dataPtr->providerName;
}

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

if (!topic.empty())
this->OnTopic(QString::fromStdString(topic));
{
this->SetTopicList({QString::fromStdString(topic)});
}
else
this->OnRefresh();

this->dataPtr->provider = new ImageProvider();
App()->Engine()->addImageProvider(
this->CardItem()->objectName() + "imagedisplay", this->dataPtr->provider);
}

/////////////////////////////////////////////////
Expand Down
15 changes: 11 additions & 4 deletions src/plugins/image_display/ImageDisplay.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
#ifndef GZ_GUI_PLUGINS_IMAGEDISPLAY_HH_
#define GZ_GUI_PLUGINS_IMAGEDISPLAY_HH_

#include <algorithm>
#include <memory>
#include <QImage>
#include <QString>
#include <QStringList>
#include <QQuickImageProvider>

#include <gz/msgs/image.pb.h>
Expand Down Expand Up @@ -53,8 +54,7 @@ namespace gz::gui::plugins
if (!this->img.isNull())
{
// Must return a copy
QImage copy(this->img);
return copy;
return this->img;
}

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

/// \brief Register the image provider with the given name
/// \param[in] _uniqueName Unique name for the provider to be registered
public: Q_INVOKABLE void RegisterImageProvider(const QString &_uniqueName);

/// \brief Get the provider name unique to this plugin instance
public: Q_INVOKABLE QString ImageProviderName();

/// \brief Notify that topic list has changed
signals: void TopicListChanged();

Expand Down
1 change: 1 addition & 0 deletions src/plugins/image_display/ImageDisplay.qml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Rectangle {
return;

uniqueName = parent.card().objectName + "imagedisplay";
ImageDisplay.RegisterImageProvider(uniqueName);
image.reload();
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/world_stats/WorldStats.qml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Rectangle {
}
PropertyChanges {
target: hideButton
x: worldStats.width - hideToolButton.width - compactLabel.width - 10
x: worldStats.width - hideToolButton.width - compactLabel.implicitWidth - 10
}
PropertyChanges {
target: compactLabel
Expand Down

0 comments on commit 14d04ab

Please sign in to comment.