service/tray: account for more edge cases and add placeholder img

src/core/iconimageprovider.cpp

@@ -1,6 +1,9 @@
 #include "iconimageprovider.hpp"
 
+#include <qcolor.h>
 #include <qicon.h>
+#include <qlogging.h>
+#include <qpainter.h>
 #include <qpixmap.h>
 #include <qsize.h>
 #include <qstring.h>
@@ -10,8 +13,32 @@ IconImageProvider::requestPixmap(const QString& id, QSize* size, const QSize& re
 	auto icon = QIcon::fromTheme(id);
 
 	auto targetSize = requestedSize.isValid() ? requestedSize : QSize(100, 100);
+	if (targetSize.width() == 0 || targetSize.height() == 0) targetSize = QSize(2, 2);
 	auto pixmap = icon.pixmap(targetSize.width(), targetSize.height());
 
+	if (pixmap.isNull()) {
+		qWarning() << "Could not load icon" << id << "at size" << targetSize << "from request";
+		pixmap = IconImageProvider::missingPixmap(targetSize);
+	}
+
 	if (size != nullptr) *size = pixmap.size();
 	return pixmap;
 }
+
+QPixmap IconImageProvider::missingPixmap(const QSize& size) {
+	auto width = size.width() % 2 == 0 ? size.width() : size.width() + 1;
+	auto height = size.height() % 2 == 0 ? size.height() : size.height() + 1;
+	if (width < 2) width = 2;
+	if (height < 2) height = 2;
+
+	auto pixmap = QPixmap(width, height);
+	pixmap.fill(QColorConstants::Black);
+	auto painter = QPainter(&pixmap);
+
+	auto halfWidth = width / 2;
+	auto halfHeight = height / 2;
+	auto purple = QColor(0xd900d8);
+	painter.fillRect(halfWidth, 0, halfWidth, halfHeight, purple);
+	painter.fillRect(0, halfHeight, halfWidth, halfHeight, purple);
+	return pixmap;
+}

src/core/iconimageprovider.hpp

@@ -8,4 +8,6 @@ public:
 	explicit IconImageProvider(): QQuickImageProvider(QQuickImageProvider::Pixmap) {}
 
 	QPixmap requestPixmap(const QString& id, QSize* size, const QSize& requestedSize) override;
+
+	static QPixmap missingPixmap(const QSize& size);
 };

src/services/status_notifier/item.cpp

@@ -25,11 +25,15 @@ Q_LOGGING_CATEGORY(logStatusNotifierItem, "quickshell.service.sni.item", QtWarni
 
 namespace qs::service::sni {
 
-StatusNotifierItem::StatusNotifierItem(const QString& address, QObject* parent): QObject(parent) {
-	// spec is unclear about what exactly an item address is, so split off anything but the connection path
-	auto conn = address.split("/").value(0);
-	this->item =
-	    new DBusStatusNotifierItem(conn, "/StatusNotifierItem", QDBusConnection::sessionBus(), this);
+StatusNotifierItem::StatusNotifierItem(const QString& address, QObject* parent)
+    : QObject(parent)
+    , watcherId(address) {
+	// spec is unclear about what exactly an item address is, so account for both combinations
+	auto splitIdx = address.indexOf('/');
+	auto conn = splitIdx == -1 ? address : address.sliced(0, splitIdx);
+	auto path = splitIdx == -1 ? "/StatusNotifierItem" : address.sliced(splitIdx);
+
+	this->item = new DBusStatusNotifierItem(conn, path, QDBusConnection::sessionBus(), this);
 
 	if (!this->item->isValid()) {
 		qCWarning(logStatusNotifierHost).noquote() << "Cannot create StatusNotifierItem for" << conn;
@@ -85,8 +89,7 @@ QString StatusNotifierItem::iconId() const {
 		if (!name.isEmpty() && overlayName.isEmpty()) return QString("image://icon/") + name;
 	}
 
-	return QString("image://service.sni/") + this->item->service() + "/"
-	     + QString::number(this->iconIndex);
+	return QString("image://service.sni/") + this->watcherId + "/" + QString::number(this->iconIndex);
 }
 
 QPixmap StatusNotifierItem::createPixmap(const QSize& size) const {

src/services/status_notifier/item.hpp

@@ -61,6 +61,7 @@ private:
 
 	// bumped to inhibit caching
 	quint32 iconIndex = 0;
+	QString watcherId;
 };
 
 } // namespace qs::service::sni

src/services/status_notifier/trayimageprovider.cpp

@@ -6,26 +6,37 @@
 #include <qsize.h>
 #include <qstring.h>
 
+#include "../../core/iconimageprovider.hpp"
 #include "host.hpp"
 
 namespace qs::service::sni {
 
 QPixmap
 TrayImageProvider::requestPixmap(const QString& id, QSize* size, const QSize& requestedSize) {
-	auto split = id.split('/');
-	if (split.size() != 2) {
+	QPixmap pixmap;
+
+	auto targetSize = requestedSize.isValid() ? requestedSize : QSize(100, 100);
+	if (targetSize.width() == 0 || targetSize.height() == 0) targetSize = QSize(2, 2);
+
+	auto lastSplit = id.lastIndexOf('/');
+	if (lastSplit == -1) {
 		qCWarning(logStatusNotifierHost) << "Invalid image request:" << id;
-		return QPixmap();
+	} else {
+		auto path = id.sliced(0, lastSplit);
+
+		auto* item = StatusNotifierHost::instance()->itemByService(path);
+
+		if (item == nullptr) {
+			qCWarning(logStatusNotifierHost) << "Image requested for nonexistant service" << path;
+		} else {
+			pixmap = item->createPixmap(targetSize);
+		}
 	}
 
-	auto* item = StatusNotifierHost::instance()->itemByService(split[0]);
-
-	if (item == nullptr) {
-		qCWarning(logStatusNotifierHost) << "Image requested for nonexistant service" << split[0];
-		return QPixmap();
+	if (pixmap.isNull()) {
+		pixmap = IconImageProvider::missingPixmap(targetSize);
 	}
 
-	auto pixmap = item->createPixmap(requestedSize);
 	if (size != nullptr) *size = pixmap.size();
 	return pixmap;
 }

src/services/status_notifier/watcher.cpp

@@ -68,17 +68,29 @@ void StatusNotifierWatcher::onServiceUnregistered(const QString& service) {
 		    << "Active StatusNotifierWatcher unregistered, attempting registration";
 		this->tryRegister();
 		return;
-	} else if (this->items.removeAll(service) != 0) {
-		qCDebug(logStatusNotifierWatcher).noquote()
-		    << "Unregistered StatusNotifierItem" << service << "from watcher";
-		emit this->StatusNotifierItemUnregistered(service);
-	} else if (this->hosts.removeAll(service) != 0) {
-		qCDebug(logStatusNotifierWatcher).noquote()
-		    << "Unregistered StatusNotifierHost" << service << "from watcher";
-		emit this->StatusNotifierHostUnregistered();
+		;
 	} else {
-		qCWarning(logStatusNotifierWatcher).noquote()
-		    << "Got service unregister event for untracked service" << service;
+		QString qualifiedItem;
+		this->items.removeIf([&](const QString& item) {
+			if (item.startsWith(service)) {
+				qualifiedItem = item;
+				return true;
+			} else return false;
+		});
+
+		if (!qualifiedItem.isEmpty()) {
+			qCDebug(logStatusNotifierWatcher).noquote()
+			    << "Unregistered StatusNotifierItem" << qualifiedItem << "from watcher";
+
+			emit this->StatusNotifierItemUnregistered(qualifiedItem);
+		} else if (this->hosts.removeAll(service) != 0) {
+			qCDebug(logStatusNotifierWatcher).noquote()
+			    << "Unregistered StatusNotifierHost" << service << "from watcher";
+			emit this->StatusNotifierHostUnregistered();
+		} else {
+			qCWarning(logStatusNotifierWatcher).noquote()
+			    << "Got service unregister event for untracked service" << service;
+		}
 	}
 
 	this->serviceWatcher.removeWatchedService(service);
@@ -112,23 +124,44 @@ void StatusNotifierWatcher::RegisterStatusNotifierHost(const QString& host) {
 }
 
 void StatusNotifierWatcher::RegisterStatusNotifierItem(const QString& item) {
-	if (this->items.contains(item)) {
+	auto qualifiedItem = this->qualifiedItem(item);
+	auto service = qualifiedItem.split("/").at(0);
+
+	if (this->items.contains(qualifiedItem)) {
 		qCDebug(logStatusNotifierWatcher).noquote()
-		    << "Skipping duplicate registration of StatusNotifierItem" << item << "to watcher";
+		    << "Skipping duplicate registration of StatusNotifierItem" << qualifiedItem << "to watcher";
 		return;
 	}
 
-	if (!QDBusConnection::sessionBus().interface()->serviceOwner(item).isValid()) {
+	if (!QDBusConnection::sessionBus().interface()->serviceOwner(service).isValid()) {
 		qCWarning(logStatusNotifierWatcher).noquote()
-		    << "Ignoring invalid StatusNotifierItem registration of" << item << "to watcher";
+		    << "Ignoring invalid StatusNotifierItem registration of" << qualifiedItem << "to watcher";
 		return;
 	}
 
-	this->serviceWatcher.addWatchedService(item);
-	this->items.push_back(item);
+	this->serviceWatcher.addWatchedService(service);
+	this->items.push_back(qualifiedItem);
+
 	qCDebug(logStatusNotifierWatcher).noquote()
-	    << "Registered StatusNotifierItem" << item << "to watcher";
-	emit this->StatusNotifierItemRegistered(item);
+	    << "Registered StatusNotifierItem" << qualifiedItem << "to watcher";
+
+	emit this->StatusNotifierItemRegistered(qualifiedItem);
+}
+
+QString StatusNotifierWatcher::qualifiedItem(const QString& item) {
+	// Registered items are often missing either the service id or the path.
+	QString qualifiedItem;
+	if (item.startsWith("/")) {
+		qualifiedItem = this->message().service() + item;
+	} else {
+		qualifiedItem = item;
+	}
+
+	if (!qualifiedItem.contains("/")) {
+		qualifiedItem += "/StatusNotifierItem";
+	}
+
+	return qualifiedItem;
 }
 
 StatusNotifierWatcher* StatusNotifierWatcher::instance() {

src/services/status_notifier/watcher.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <qdbuscontext.h>
 #include <qdbusinterface.h>
 #include <qdbusservicewatcher.h>
 #include <qlist.h>
@@ -12,7 +13,9 @@ Q_DECLARE_LOGGING_CATEGORY(logStatusNotifierWatcher);
 
 namespace qs::service::sni {
 
-class StatusNotifierWatcher: public QObject {
+class StatusNotifierWatcher
+    : public QObject
+    , protected QDBusContext {
 	Q_OBJECT;
 	Q_PROPERTY(qint32 ProtocolVersion READ protocolVersion);
 	Q_PROPERTY(bool IsStatusNotifierHostRegistered READ isHostRegistered);
@@ -46,6 +49,8 @@ private slots:
 	void onServiceUnregistered(const QString& service);
 
 private:
+	QString qualifiedItem(const QString& item);
+
 	QDBusServiceWatcher serviceWatcher;
 	QList<QString> hosts;
 	QList<QString> items;