Skip to content

Commit

Permalink
fix: GUI responsiveness when changing tracks
Browse files Browse the repository at this point in the history
Refactor KNotification to use QImage instead of tossing around QPixmaps.
This reduces unnessecary serialization. In addition, resize notification
thumbnail to 512px.

The GUI might still take too long to update for extremely large images,
as resizing is not free. This is still an improvement.

In the future, we could cache these smaller thumbnails, or on Linux,
pass the `image-path` parameter.
  • Loading branch information
nullobsi committed Jul 29, 2024
1 parent 2e93583 commit 719adb5
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 48 deletions.
26 changes: 13 additions & 13 deletions 3rdparty/knotifications/src/knotification.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,15 @@ QString KNotification::iconName() const
return d->iconName;
}

QPixmap KNotification::pixmap() const
const QImage& KNotification::image() const
{
return d->pixmap;
return d->image;
}

void KNotification::setPixmap(const QPixmap& pix)
void KNotification::setImage(const QImage& img)
{
d->needUpdate = true;
d->pixmap = pix;
d->image = img;
if (d->id >= 0) {
d->updateTimer.start();
}
Expand Down Expand Up @@ -416,14 +416,14 @@ static QString defaultComponentName()
KNotification* KNotification::event(const QString& eventid,
const QString& title,
const QString& text,
const QPixmap& pixmap,
const QImage& image,
const NotificationFlags& flags,
const QString& componentName)
{
KNotification* notify = new KNotification(eventid, flags);
notify->setTitle(title);
notify->setText(text);
notify->setPixmap(pixmap);
notify->setImage(image);
notify->setComponentName((flags & DefaultEvent) ? defaultComponentName() : componentName);

QTimer::singleShot(0, notify, &KNotification::sendEvent);
Expand All @@ -432,19 +432,19 @@ KNotification* KNotification::event(const QString& eventid,
}

KNotification*
KNotification::event(const QString& eventid, const QString& text, const QPixmap& pixmap, const NotificationFlags& flags, const QString& componentName)
KNotification::event(const QString& eventid, const QString& text, const QImage& image, const NotificationFlags& flags, const QString& componentName)
{
return event(eventid, QString(), text, pixmap, flags, componentName);
return event(eventid, QString(), text, image, flags, componentName);
}

KNotification* KNotification::event(StandardEvent eventid, const QString& title, const QString& text, const QPixmap& pixmap, const NotificationFlags& flags)
KNotification* KNotification::event(StandardEvent eventid, const QString& title, const QString& text, const QImage& image, const NotificationFlags& flags)
{
return event(standardEventToEventId(eventid), title, text, pixmap, flags | DefaultEvent);
return event(standardEventToEventId(eventid), title, text, image, flags | DefaultEvent);
}

KNotification* KNotification::event(StandardEvent eventid, const QString& text, const QPixmap& pixmap, const NotificationFlags& flags)
KNotification* KNotification::event(StandardEvent eventid, const QString& text, const QImage& image, const NotificationFlags& flags)
{
return event(eventid, QString(), text, pixmap, flags);
return event(eventid, QString(), text, image, flags);
}

KNotification* KNotification::event(const QString& eventid,
Expand Down Expand Up @@ -491,7 +491,7 @@ void KNotification::deref()

void KNotification::beep(const QString& reason)
{
event(QStringLiteral("beep"), reason, QPixmap(), CloseOnTimeout | DefaultEvent);
event(QStringLiteral("beep"), reason, QImage(), CloseOnTimeout | DefaultEvent);
}

void KNotification::sendEvent()
Expand Down
30 changes: 15 additions & 15 deletions 3rdparty/knotifications/src/knotification.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <QList>
#include <QObject>
#include <QPair>
#include <QPixmap>
#include <QImage>
#include <QUrl>
#include <QVariant>
#include <QWindow>
Expand Down Expand Up @@ -302,16 +302,16 @@ class KNotification : public QObject {
void setIconName(const QString& icon);

/**
* \return the pixmap shown in the popup
* \see setPixmap
* \return the image shown in the popup
* \see setImage
*/
QPixmap pixmap() const;
const QImage& image() const;
/**
* Set the pixmap that will be shown in the popup. If you want to use an icon from the icon theme use setIconName instead.
* Set the image that will be shown in the popup. If you want to use an icon from the icon theme use setIconName instead.
*
* @param pix the pixmap
* @param img the image
*/
void setPixmap(const QPixmap& pix);
void setImage(const QImage& img);

/**
* @return the default action, or nullptr if none is set
Expand Down Expand Up @@ -690,15 +690,15 @@ public Q_SLOTS:
* @param eventId is the name of the event
* @param title is title of the notification to show in the popup.
* @param text is the text of the notification to show in the popup.
* @param pixmap is a picture which may be shown in the popup.
* @param image is a picture which may be shown in the popup.
* @param flags is a bitmask of NotificationFlag
* @param componentName used to determine the location of the config file. by default, appname is used
* @since 4.4
*/
static KNotification* event(const QString& eventId,
const QString& title,
const QString& text,
const QPixmap& pixmap = QPixmap(),
const QImage& image = QImage(),
const NotificationFlags& flags = CloseOnTimeout,
const QString& componentName = QString());

Expand All @@ -711,13 +711,13 @@ public Q_SLOTS:
*
* @param eventId is the name of the event
* @param text is the text of the notification to show in the popup.
* @param pixmap is a picture which may be shown in the popup.
* @param image is a picture which may be shown in the popup.
* @param flags is a bitmask of NotificationFlag
* @param componentName used to determine the location of the config file. by default, plasma_workspace is used
*/
static KNotification* event(const QString& eventId,
const QString& text = QString(),
const QPixmap& pixmap = QPixmap(),
const QImage& image = QImage(),
const NotificationFlags& flags = CloseOnTimeout,
const QString& componentName = QString());

Expand All @@ -730,11 +730,11 @@ public Q_SLOTS:
*
* @param eventId is the name of the event
* @param text is the text of the notification to show in the popup
* @param pixmap is a picture which may be shown in the popup
* @param image is a picture which may be shown in the popup
* @param flags is a bitmask of NotificationFlag
*/
static KNotification*
event(StandardEvent eventId, const QString& text = QString(), const QPixmap& pixmap = QPixmap(), const NotificationFlags& flags = CloseOnTimeout);
event(StandardEvent eventId, const QString& text = QString(), const QImage& image = QImage(), const NotificationFlags& flags = CloseOnTimeout);

/**
* @brief emit a standard event
Expand All @@ -746,12 +746,12 @@ public Q_SLOTS:
* @param eventId is the name of the event
* @param title is title of the notification to show in the popup.
* @param text is the text of the notification to show in the popup
* @param pixmap is a picture which may be shown in the popup
* @param image is a picture which may be shown in the popup
* @param flags is a bitmask of NotificationFlag
* @since 4.4
*/
static KNotification*
event(StandardEvent eventId, const QString& title, const QString& text, const QPixmap& pixmap, const NotificationFlags& flags = CloseOnTimeout);
event(StandardEvent eventId, const QString& title, const QString& text, const QImage& image, const NotificationFlags& flags = CloseOnTimeout);

/**
* @brief emit a standard event with the possibility of setting an icon by icon name
Expand Down
2 changes: 1 addition & 1 deletion 3rdparty/knotifications/src/knotification_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct Q_DECL_HIDDEN KNotification::Private {
bool ownsActions = true;
QString xdgActivationToken;
std::unique_ptr<KNotificationReplyAction> replyAction;
QPixmap pixmap;
QImage image;
NotificationFlags flags = KNotification::CloseOnTimeout;
QString componentName;
KNotification::Urgency urgency = KNotification::DefaultUrgency;
Expand Down
3 changes: 1 addition & 2 deletions 3rdparty/knotifications/src/knotificationmanager_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <memory>

class KNotification;
class QPixmap;
class KNotificationPlugin;

/**
Expand Down Expand Up @@ -43,7 +42,7 @@ class KNotificationManager : public QObject {
void close(int id);

/**
* update one notification text and pixmap and actions
* update one notification text and image and actions
*/
void update(KNotification* n);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ - (void)userNotificationCenter:(NSUserNotificationCenter *)center didDeliverNoti
internalNotificationId, @"internalId", nil];
osxNotification.informativeText = text;

if (notification->pixmap().isNull()) {
if (notification->image().isNull()) {
QIcon notificationIcon = QIcon::fromTheme(notification->iconName());
if (!notificationIcon.isNull()) {
osxNotification.contentImage = [[NSImage alloc]
initWithCGImage: notificationIcon.pixmap(QSize(64, 64)).toImage().toCGImage() size: NSMakeSize(64, 64)];
}
} else {
osxNotification.contentImage = [[NSImage alloc]
initWithCGImage: notification->pixmap().toImage().toCGImage() size: NSMakeSize(64, 64)];
initWithCGImage: notification->image().toCGImage() size: NSMakeSize(64, 64)];
}

if (notification->actions().isEmpty()) {
Expand Down
9 changes: 2 additions & 7 deletions 3rdparty/knotifications/src/notifybypopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,8 @@ bool NotifyByPopup::sendNotificationToServer(KNotification* notification, const

// FIXME - re-enable/fix
// let's see if we've got an image, and store the image in the hints map
if (!notification->pixmap().isNull()) {
QByteArray pixmapData;
QBuffer buffer(&pixmapData);
buffer.open(QIODevice::WriteOnly);
notification->pixmap().save(&buffer, "PNG");
buffer.close();
hintsMap[QStringLiteral("image_data")] = ImageConverter::variantForImage(QImage::fromData(pixmapData));
if (!notification->image().isNull()) {
hintsMap[QStringLiteral("image-data")] = ImageConverter::variantForImage(notification->image());
}

// Persistent => 0 == infinite timeout
Expand Down
10 changes: 5 additions & 5 deletions 3rdparty/knotifications/src/notifybyportal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,16 +296,16 @@ bool NotifyByPortalPrivate::sendNotificationToPortal(KNotification* notification
qDBusRegisterMetaType<QList<QVariantMap>>();
qDBusRegisterMetaType<PortalIcon>();

if (!notification->pixmap().isNull()) {
QByteArray pixmapData;
QBuffer buffer(&pixmapData);
if (!notification->image().isNull()) {
QByteArray imageData;
QBuffer buffer(&imageData);
buffer.open(QIODevice::WriteOnly);
notification->pixmap().save(&buffer, "PNG");
notification->image().save(&buffer, "PNG");
buffer.close();

PortalIcon icon;
icon.str = QStringLiteral("bytes");
icon.data.setVariant(pixmapData);
icon.data.setVariant(imageData);
portalArgs.insert(QStringLiteral("icon"), QVariant::fromValue<PortalIcon>(icon));
}
else {
Expand Down
4 changes: 2 additions & 2 deletions 3rdparty/knotifications/src/notifybysnore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ void NotifyBySnore::notifyDeferred(KNotification* notification)

// handle the icon for toast notification
const QString iconPath = m_iconDir.path() + QLatin1Char('/') + QString::number(notification->id());
const bool hasIcon = (notification->pixmap().isNull()) ? qApp->windowIcon().pixmap(1024, 1024).save(iconPath, "PNG")//
: notification->pixmap().save(iconPath, "PNG");
const bool hasIcon = (notification->image().isNull()) ? qApp->windowIcon().pixmap(1024, 1024).save(iconPath, "PNG")//
: notification->image().save(iconPath, "PNG");
if (hasIcon) {
snoretoastArgsList << QStringLiteral("-p") << iconPath;
}
Expand Down
3 changes: 2 additions & 1 deletion gui/trayitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,11 @@ void TrayItem::songChanged(const Song& song, bool isPlaying)
if (useable && isPlaying) {
if (songNotif == nullptr) {
songNotif = new KNotification("newSong");
songNotif->setAutoDelete(false);
}
songNotif->setTitle(song.mainText());
songNotif->setText(song.subText());
songNotif->setPixmap(QPixmap::fromImage(CurrentCover::self()->image()));
songNotif->setImage(CurrentCover::self()->image().scaledToHeight(512));
songNotif->setUrgency(KNotification::LowUrgency);
songNotif->sendEvent();
}
Expand Down

0 comments on commit 719adb5

Please sign in to comment.