[network/ruqola] src/widgets/room/delegate: Fix disable animating when necessary. We can have more attachment => use identifier
Laurent Montel <[email protected]>
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit dad594a3b85caf296d3c35231be867f003f641df by Laurent Montel.
Committed on 13/08/2026 at 12:04.
Pushed by mlaurent into branch 'master'.
Fix disable animating when necessary. We can have more attachment => use identifier
M +20 -10 src/widgets/room/delegate/messageattachmentdelegatehelperimage.cpp
M +5 -2 src/widgets/room/delegate/messageattachmentdelegatehelperimage.h
M +23 -13 src/widgets/room/delegate/messagedelegatehelperreactions.cpp
M +5 -2 src/widgets/room/delegate/messagedelegatehelperreactions.h
M +12 -4 src/widgets/room/delegate/runninganimatedimage.cpp
M +5 -1 src/widgets/room/delegate/runninganimatedimage.h
https://invent.kde.org/network/ruqola/-/commit/dad594a3b85caf296d3c35231be867f003f641df
diff --git a/src/widgets/room/delegate/messageattachmentdelegatehelperimage.cpp b/src/widgets/room/delegate/messageattachmentdelegatehelperimage.cpp
index 0435947a0b..dc9ad39b48 100644
--- a/src/widgets/room/delegate/messageattachmentdelegatehelperimage.cpp
+++ b/src/widgets/room/delegate/messageattachmentdelegatehelperimage.cpp
@@ -45,8 +45,9 @@ void MessageAttachmentDelegateHelperImage::draw(const MessageAttachment &msgAtta
// Only an animated attachment that we actually paint may keep a running QMovie: otherwise it would
// go on emitting frameChanged() -> view->update() forever, repainting something we don't draw.
const bool animateImage = !layout.pixmap.isNull() && layout.isShown && layout.isAnimatedImage && RuqolaGlobalConfig::self()->animateGifImage();
+ const QByteArray attachmentId = msgAttach.attachmentId();
if (!animateImage) {
- removeRunningAnimatedImage(index);
+ removeRunningAnimatedImage(index, attachmentId);
}
// drawTitle(msgAttach, painter, );
painter->drawText(messageRect.x(), messageRect.y() + option.fontMetrics.ascent(), layout.title);
@@ -62,11 +63,11 @@ void MessageAttachmentDelegateHelperImage::draw(const MessageAttachment &msgAtta
if (layout.isShown) {
QPixmap scaledPixmap;
if (animateImage) {
- auto it = findRunningAnimatedImage(index);
+ auto it = findRunningAnimatedImage(index, attachmentId);
if (it != mRunningAnimatedImages.end()) {
scaledPixmap = (*it).movie->currentPixmap();
} else {
- mRunningAnimatedImages.emplace_back(index);
+ mRunningAnimatedImages.emplace_back(index, attachmentId);
auto &rai = mRunningAnimatedImages.back();
rai.movie->setFileName(layout.imagePreviewPath);
rai.movie->setScaledSize(layout.imageSize);
@@ -80,7 +81,8 @@ void MessageAttachmentDelegateHelperImage::draw(const MessageAttachment &msgAtta
if (view->viewport()->rect().intersects(view->visualRect(idx))) {
view->update(idx);
} else {
- removeRunningAnimatedImage(idx);
+ // The whole message is out of sight: stop all its animations, not just this one.
+ removeRunningAnimatedImages(idx);
}
},
Qt::QueuedConnection);
@@ -227,22 +229,30 @@ MessageAttachmentDelegateHelperImage::ImageLayout MessageAttachmentDelegateHelpe
return layout;
}
-std::vector<RunningAnimatedImage>::iterator MessageAttachmentDelegateHelperImage::findRunningAnimatedImage(const QModelIndex &index) const
+std::vector<RunningAnimatedImage>::iterator MessageAttachmentDelegateHelperImage::findRunningAnimatedImage(const QModelIndex &index,
+ const QByteArray &identifier) const
{
- auto matchesIndex = [&](const RunningAnimatedImage &rai) {
- return rai.index == index;
+ auto matchesImage = [&](const RunningAnimatedImage &rai) {
+ return rai.index == index && rai.identifier == identifier;
};
- return std::find_if(mRunningAnimatedImages.begin(), mRunningAnimatedImages.end(), matchesIndex);
+ return std::find_if(mRunningAnimatedImages.begin(), mRunningAnimatedImages.end(), matchesImage);
}
-void MessageAttachmentDelegateHelperImage::removeRunningAnimatedImage(const QModelIndex &index) const
+void MessageAttachmentDelegateHelperImage::removeRunningAnimatedImage(const QModelIndex &index, const QByteArray &identifier) const
{
- auto it = findRunningAnimatedImage(index);
+ auto it = findRunningAnimatedImage(index, identifier);
if (it != mRunningAnimatedImages.end()) {
mRunningAnimatedImages.erase(it);
}
}
+void MessageAttachmentDelegateHelperImage::removeRunningAnimatedImages(const QModelIndex &index) const
+{
+ std::erase_if(mRunningAnimatedImages, [&](const RunningAnimatedImage &rai) {
+ return rai.index == index;
+ });
+}
+
QPoint MessageAttachmentDelegateHelperImage::adaptMousePosition(const QPoint &pos,
const MessageAttachment &msgAttach,
QRect attachmentsRect,
diff --git a/src/widgets/room/delegate/messageattachmentdelegatehelperimage.h b/src/widgets/room/delegate/messageattachmentdelegatehelperimage.h
index d254891c36..1d56f3145a 100644
--- a/src/widgets/room/delegate/messageattachmentdelegatehelperimage.h
+++ b/src/widgets/room/delegate/messageattachmentdelegatehelperimage.h
@@ -48,8 +48,11 @@ private:
[[nodiscard]] ImageLayout
layoutImage(const MessageAttachment &msgAttach, const QStyleOptionViewItem &option, int attachmentsWidth, int attachmentsHeight) const;
- [[nodiscard]] std::vector<RunningAnimatedImage>::iterator findRunningAnimatedImage(const QModelIndex &index) const;
- void removeRunningAnimatedImage(const QModelIndex &index) const;
+ // A message can hold several image attachments, so an animation is identified by (index, attachment id).
+ [[nodiscard]] std::vector<RunningAnimatedImage>::iterator findRunningAnimatedImage(const QModelIndex &index, const QByteArray &identifier) const;
+ void removeRunningAnimatedImage(const QModelIndex &index, const QByteArray &identifier) const;
+ // Removes every animation of this message, whatever the attachment (used when the message is not visible).
+ void removeRunningAnimatedImages(const QModelIndex &index) const;
[[nodiscard]] bool contextMenu(const QPoint &pos,
const QPoint &globalPos,
const MessageAttachment &msgAttach,
diff --git a/src/widgets/room/delegate/messagedelegatehelperreactions.cpp b/src/widgets/room/delegate/messagedelegatehelperreactions.cpp
index 988b990827..7db0f216ad 100644
--- a/src/widgets/room/delegate/messagedelegatehelperreactions.cpp
+++ b/src/widgets/room/delegate/messagedelegatehelperreactions.cpp
@@ -97,9 +97,11 @@ void MessageDelegateHelperReactions::draw(QPainter *painter, QRect reactionsRect
if (auto react = message->reactions()) {
reactions = react->reactions();
} else {
+ removeRunningAnimatedImages(index);
return;
}
if (reactions.isEmpty()) {
+ removeRunningAnimatedImages(index);
return;
}
#if 0
@@ -119,6 +121,7 @@ void MessageDelegateHelperReactions::draw(QPainter *painter, QRect reactionsRect
const qreal smallMargin = 4;
painter->setRenderHint(QPainter::Antialiasing);
+ QList<QByteArray> animatedReactions;
for (const ReactionLayout &reactionLayout : layouts) {
Q_ASSERT(!reactionLayout.emojiString.isEmpty() || !reactionLayout.emojiImagePath.isEmpty());
const QRectF reactionRect = reactionLayout.reactionRect;
@@ -138,15 +141,18 @@ void MessageDelegateHelperReactions::draw(QPainter *painter, QRect reactionsRect
}
painter->drawText(r, reactionLayout.emojiString);
} else {
- if (reactionLayout.reaction.isAnimatedImage() && RuqolaGlobalConfig::self()->animateGifImage()) {
+ const bool animateGif = reactionLayout.reaction.isAnimatedImage() && RuqolaGlobalConfig::self()->animateGifImage();
+ if (animateGif) {
const int maxIconSize = option.widget->style()->pixelMetric(QStyle::PM_ButtonIconSize);
+ const QByteArray identifier = reactionLayout.reaction.reactionName().toUtf8();
+ animatedReactions.append(identifier);
QPixmap scaledPixmap;
- auto it = findRunningAnimatedImage(index);
+ auto it = findRunningAnimatedImage(index, identifier);
if (it != mRunningAnimatedImages.end()) {
scaledPixmap = (*it).movie->currentPixmap();
} else {
- mRunningAnimatedImages.emplace_back(index);
+ mRunningAnimatedImages.emplace_back(index, identifier);
auto &rai = mRunningAnimatedImages.back();
rai.movie->setFileName(reactionLayout.emojiImagePath);
rai.movie->setScaledSize(QSize(maxIconSize, maxIconSize));
@@ -160,7 +166,8 @@ void MessageDelegateHelperReactions::draw(QPainter *painter, QRect reactionsRect
if (view->viewport()->rect().intersects(view->visualRect(idx))) {
view->update(idx);
} else {
- removeRunningAnimatedImage(idx);
+ // The whole message is out of sight: stop all its animations, not just this one.
+ removeRunningAnimatedImages(idx);
}
},
Qt::QueuedConnection);
@@ -180,22 +187,25 @@ void MessageDelegateHelperReactions::draw(QPainter *painter, QRect reactionsRect
painter->setFont(option.font);
painter->drawText(reactionLayout.countRect, reactionLayout.countStr);
}
+ // Any other animation of this message is stale (reaction removed, collapsed, animations turned off...):
+ // its movie would otherwise keep emitting frameChanged() -> view->update() for something we don't paint.
+ removeRunningAnimatedImages(index, animatedReactions);
}
-std::vector<RunningAnimatedImage>::iterator MessageDelegateHelperReactions::findRunningAnimatedImage(const QModelIndex &index) const
+std::vector<RunningAnimatedImage>::iterator MessageDelegateHelperReactions::findRunningAnimatedImage(const QModelIndex &index,
+ const QByteArray &identifier) const
{
- auto matchesIndex = [&](const RunningAnimatedImage &rai) {
- return rai.index == index;
+ auto matchesReaction = [&](const RunningAnimatedImage &rai) {
+ return rai.index == index && rai.identifier == identifier;
};
- return std::find_if(mRunningAnimatedImages.begin(), mRunningAnimatedImages.end(), matchesIndex);
+ return std::find_if(mRunningAnimatedImages.begin(), mRunningAnimatedImages.end(), matchesReaction);
}
-void MessageDelegateHelperReactions::removeRunningAnimatedImage(const QModelIndex &index) const
+void MessageDelegateHelperReactions::removeRunningAnimatedImages(const QModelIndex &index, const QList<QByteArray> &identifiersToKeep) const
{
- auto it = findRunningAnimatedImage(index);
- if (it != mRunningAnimatedImages.end()) {
- mRunningAnimatedImages.erase(it);
- }
+ std::erase_if(mRunningAnimatedImages, [&](const RunningAnimatedImage &rai) {
+ return rai.index == index && !identifiersToKeep.contains(rai.identifier);
+ });
}
QSize MessageDelegateHelperReactions::sizeHint(const QModelIndex &index, int maxWidth, const QStyleOptionViewItem &option) const
diff --git a/src/widgets/room/delegate/messagedelegatehelperreactions.h b/src/widgets/room/delegate/messagedelegatehelperreactions.h
index 8f3e42967a..5d98e16d5a 100644
--- a/src/widgets/room/delegate/messagedelegatehelperreactions.h
+++ b/src/widgets/room/delegate/messagedelegatehelperreactions.h
@@ -50,8 +50,11 @@ private:
bool useEmojiFont;
};
- [[nodiscard]] LIBRUQOLAWIDGETS_NO_EXPORT std::vector<RunningAnimatedImage>::iterator findRunningAnimatedImage(const QModelIndex &index) const;
- LIBRUQOLAWIDGETS_NO_EXPORT void removeRunningAnimatedImage(const QModelIndex &index) const;
+ // A message can show several animated reactions at once, so an animation is identified by (index, reaction name).
+ [[nodiscard]] LIBRUQOLAWIDGETS_NO_EXPORT std::vector<RunningAnimatedImage>::iterator findRunningAnimatedImage(const QModelIndex &index,
+ const QByteArray &identifier) const;
+ // Removes the animations of this message whose reaction is not in identifiersToKeep, i.e. all of them by default.
+ LIBRUQOLAWIDGETS_NO_EXPORT void removeRunningAnimatedImages(const QModelIndex &index, const QList<QByteArray> &identifiersToKeep = {}) const;
[[nodiscard]] LIBRUQOLAWIDGETS_NO_EXPORT QList<ReactionLayout>
layoutReactions(const QList<Reaction> &reactions, QRect reactionsRect, const QStyleOptionViewItem &option) const;
const QFont mEmojiFont;
diff --git a/src/widgets/room/delegate/runninganimatedimage.cpp b/src/widgets/room/delegate/runninganimatedimage.cpp
index bc84c83e74..bd5ecc9881 100644
--- a/src/widgets/room/delegate/runninganimatedimage.cpp
+++ b/src/widgets/room/delegate/runninganimatedimage.cpp
@@ -8,8 +8,9 @@
#include <QMovie>
-RunningAnimatedImage::RunningAnimatedImage(const QModelIndex &idx)
+RunningAnimatedImage::RunningAnimatedImage(const QModelIndex &idx, const QByteArray &identifier)
: index(idx)
+ , identifier(identifier)
, movie(new QMovie)
{
}
@@ -22,6 +23,7 @@ RunningAnimatedImage::~RunningAnimatedImage()
RunningAnimatedImage::RunningAnimatedImage(RunningAnimatedImage &&other) noexcept
: index(other.index)
+ , identifier(std::move(other.identifier))
, movie(other.movie)
{
other.movie = nullptr;
@@ -29,8 +31,14 @@ RunningAnimatedImage::RunningAnimatedImage(RunningAnimatedImage &&other) noexcep
RunningAnimatedImage &RunningAnimatedImage::operator=(RunningAnimatedImage &&other)
{
- index = other.index;
- movie = other.movie;
- other.movie = nullptr;
+ if (this != &other) {
+ index = other.index;
+ identifier = std::move(other.identifier);
+ // Don't leak the movie we own: std::vector::erase() move-assigns the following elements over the
+ // erased one, so this is what destroys the movie of a removed (non-last) entry.
+ delete movie;
+ movie = other.movie;
+ other.movie = nullptr;
+ }
return *this;
}
diff --git a/src/widgets/room/delegate/runninganimatedimage.h b/src/widgets/room/delegate/runninganimatedimage.h
index fab53ebfad..30a312c134 100644
--- a/src/widgets/room/delegate/runninganimatedimage.h
+++ b/src/widgets/room/delegate/runninganimatedimage.h
@@ -8,11 +8,12 @@
#include "libruqolawidgets_private_export.h"
+#include <QByteArray>
#include <QPersistentModelIndex>
class QMovie;
struct LIBRUQOLAWIDGETS_TESTS_EXPORT RunningAnimatedImage {
- explicit RunningAnimatedImage(const QModelIndex &idx);
+ explicit RunningAnimatedImage(const QModelIndex &idx, const QByteArray &identifier);
~RunningAnimatedImage();
RunningAnimatedImage(const RunningAnimatedImage &) = delete;
RunningAnimatedImage(RunningAnimatedImage &&other) noexcept;
@@ -20,5 +21,8 @@ struct LIBRUQOLAWIDGETS_TESTS_EXPORT RunningAnimatedImage {
RunningAnimatedImage &operator=(RunningAnimatedImage &&other);
QPersistentModelIndex index;
+ // A single message (i.e. a single model index) can show several animated images at once, so the index
+ // alone doesn't identify an animation: the identifier tells them apart (attachment id, reaction name...).
+ QByteArray identifier;
QMovie *movie = nullptr;
};