chatterino2
chatterino2 copied to clipboard
Lazily load images on paint, not layout
Pull request checklist:
- [x]
CHANGELOG.md
was updated, if applicable
Description
Instead of loading image pixmap data on layout, we try to do it when the image is first painted. This PR will increase the efficacy of #3915.
To demonstrate the behavior, you can do the following:
- Open debug popup
- Open a channel
- Switch to a different channel (or an empty tab)
- Send a message in the first channel (from webchat or another instance) that contains an emote
- Observe that the "Loaded images" value in the debug popup doesn't increase until you swap back to the channel in which the message was sent
Example Screenshots
1a810b4d on left, this PR on right.Before sending message:
After sending
PogChamp
message:
Note that "Loaded images" increases by 2 on the left as the broadcaster badge was also then loaded.
After refocusing channel in which message was sent:
Todo:
- [x] Fix/verify emote scaling selection
Fixes #3929
I'm going to run this PR as my daily driver for a while, and I'd urge others to do the same
To test a PR build, you can browse to the linked PR, click the "Checks" tab, then select the "Build" job on the left side, then scroll down to find the build that fits your platform. Note that you need to be signed into GitHub to download from that page.
FFZ badges are missing their background on Windows
Interestingly does NOT apply to custom VIP badges, they continue to work fine.
I've run this for a week now, only thing I've noticed is what Felanbird mentioned, some special badges don't render their background properly
emote scale passed to addImageSetToContainer
was ignored
diff --git a/src/messages/MessageElement.cpp b/src/messages/MessageElement.cpp
index 49d49aacb..ec9bd5c5a 100644
--- a/src/messages/MessageElement.cpp
+++ b/src/messages/MessageElement.cpp
@@ -27,7 +27,7 @@ namespace {
return;
}
- auto size = priority->firstLoadedImageSize() * container.getScale();
+ auto size = priority->firstLoadedImageSize() * scale;
container.addElement((new PriorityImageLayoutElement(
element, std::move(*priority), size))
background is missing for FFZ badges. because makeImageLayoutElement()
methods are not called anymore for elements, where ImageWithBackgroundLayoutElement
happened. Diff dropping now dead code and passing background color down to PriorityImageLayoutElement
diff --git a/src/messages/MessageElement.cpp b/src/messages/MessageElement.cpp
index 49d49aacb..f3cbb6950 100644
--- a/src/messages/MessageElement.cpp
+++ b/src/messages/MessageElement.cpp
@@ -19,7 +19,8 @@ namespace chatterino {
namespace {
void addImageSetToContainer(MessageLayoutContainer &container,
MessageElement &element,
- const ImageSet &imageSet, float scale)
+ const ImageSet &imageSet, float scale,
+ std::optional<QColor> bgColor = std::nullopt)
{
auto priority = imageSet.getPriority(scale);
if (!priority)
@@ -30,7 +31,7 @@ namespace {
auto size = priority->firstLoadedImageSize() * container.getScale();
container.addElement((new PriorityImageLayoutElement(
- element, std::move(*priority), size))
+ element, std::move(*priority), size, bgColor))
->setLink(element.getLink()));
}
@@ -241,12 +242,6 @@ void EmoteElement::addToContainer(MessageLayoutContainer &container,
}
}
-MessageLayoutElement *EmoteElement::makeImageLayoutElement(
- const ImagePtr &image, const QSize &size)
-{
- return new ImageLayoutElement(*this, image, size);
-}
-
LayeredEmoteElement::LayeredEmoteElement(
std::vector<LayeredEmoteElement::Emote> &&emotes, MessageElementFlags flags,
const MessageColor &textElementColor)
@@ -287,10 +282,9 @@ void LayeredEmoteElement::addToContainer(MessageLayoutContainer &container,
individualSizes.push_back(QSize(img->width(), img->height()) *
overallScale);
}
-
- container.addElement(this->makeImageLayoutElement(
- images, individualSizes, largestSize)
- ->setLink(this->getLink()));
+ auto *newElement = new LayeredImageLayoutElement(
+ *this, images, individualSizes, largestSize);
+ container.addElement(newElement->setLink(this->getLink()));
}
else
{
@@ -320,13 +314,6 @@ std::vector<ImagePtr> LayeredEmoteElement::getLoadedImages(float scale)
return res;
}
-MessageLayoutElement *LayeredEmoteElement::makeImageLayoutElement(
- const std::vector<ImagePtr> &images, const std::vector<QSize> &sizes,
- QSize largestSize)
-{
- return new LayeredImageLayoutElement(*this, images, sizes, largestSize);
-}
-
void LayeredEmoteElement::updateTooltips()
{
if (!this->emotes_.empty())
@@ -412,9 +399,11 @@ std::vector<LayeredEmoteElement::Emote> LayeredEmoteElement::getUniqueEmotes()
}
// BADGE
-BadgeElement::BadgeElement(const EmotePtr &emote, MessageElementFlags flags)
+BadgeElement::BadgeElement(const EmotePtr &emote, MessageElementFlags flags,
+ std::optional<QColor> bgColor)
: MessageElement(flags)
, emote_(emote)
+ , bgColor_(bgColor)
{
this->setTooltip(emote->tooltip.string);
}
@@ -425,7 +414,7 @@ void BadgeElement::addToContainer(MessageLayoutContainer &container,
if (flags.hasAny(this->getFlags()))
{
addImageSetToContainer(container, *this, this->emote_->images,
- container.getScale());
+ container.getScale(), this->bgColor_);
}
}
@@ -434,34 +423,13 @@ EmotePtr BadgeElement::getEmote() const
return this->emote_;
}
-MessageLayoutElement *BadgeElement::makeImageLayoutElement(
- const ImagePtr &image, const QSize &size)
-{
- auto element =
- (new ImageLayoutElement(*this, image, size))->setLink(this->getLink());
-
- return element;
-}
-
// MOD BADGE
ModBadgeElement::ModBadgeElement(const EmotePtr &data,
MessageElementFlags flags_)
- : BadgeElement(data, flags_)
+ : BadgeElement(data, flags_, {"#34AE0A"})
{
}
-MessageLayoutElement *ModBadgeElement::makeImageLayoutElement(
- const ImagePtr &image, const QSize &size)
-{
- static const QColor modBadgeBackgroundColor("#34AE0A");
-
- auto element = (new ImageWithBackgroundLayoutElement(
- *this, image, size, modBadgeBackgroundColor))
- ->setLink(this->getLink());
-
- return element;
-}
-
// VIP BADGE
VipBadgeElement::VipBadgeElement(const EmotePtr &data,
MessageElementFlags flags_)
@@ -469,31 +437,11 @@ VipBadgeElement::VipBadgeElement(const EmotePtr &data,
{
}
-MessageLayoutElement *VipBadgeElement::makeImageLayoutElement(
- const ImagePtr &image, const QSize &size)
-{
- auto element =
- (new ImageLayoutElement(*this, image, size))->setLink(this->getLink());
-
- return element;
-}
-
// FFZ Badge
FfzBadgeElement::FfzBadgeElement(const EmotePtr &data,
MessageElementFlags flags_, QColor color_)
- : BadgeElement(data, flags_)
- , color(std::move(color_))
-{
-}
-
-MessageLayoutElement *FfzBadgeElement::makeImageLayoutElement(
- const ImagePtr &image, const QSize &size)
+ : BadgeElement(data, flags_, color_)
{
- auto element =
- (new ImageWithBackgroundLayoutElement(*this, image, size, this->color))
- ->setLink(this->getLink());
-
- return element;
}
// TEXT
diff --git a/src/messages/MessageElement.hpp b/src/messages/MessageElement.hpp
index c35f9616e..3cbb965e9 100644
--- a/src/messages/MessageElement.hpp
+++ b/src/messages/MessageElement.hpp
@@ -316,10 +316,6 @@ public:
MessageElementFlags flags_) override;
EmotePtr getEmote() const;
-protected:
- virtual MessageLayoutElement *makeImageLayoutElement(const ImagePtr &image,
- const QSize &size);
-
private:
std::unique_ptr<TextElement> textElement_;
EmotePtr emote_;
@@ -352,10 +348,6 @@ public:
const std::vector<QString> &getEmoteTooltips() const;
private:
- MessageLayoutElement *makeImageLayoutElement(
- const std::vector<ImagePtr> &image, const std::vector<QSize> &sizes,
- QSize largestSize);
-
QString getCopyString() const;
void updateTooltips();
std::vector<ImagePtr> getLoadedImages(float scale);
@@ -370,39 +362,29 @@ private:
class BadgeElement : public MessageElement
{
public:
- BadgeElement(const EmotePtr &data, MessageElementFlags flags_);
+ BadgeElement(const EmotePtr &emote, MessageElementFlags flags,
+ std::optional<QColor> bgColor = std::nullopt);
void addToContainer(MessageLayoutContainer &container,
MessageElementFlags flags_) override;
EmotePtr getEmote() const;
-protected:
- virtual MessageLayoutElement *makeImageLayoutElement(const ImagePtr &image,
- const QSize &size);
-
private:
EmotePtr emote_;
+ std::optional<QColor> bgColor_;
};
class ModBadgeElement : public BadgeElement
{
public:
ModBadgeElement(const EmotePtr &data, MessageElementFlags flags_);
-
-protected:
- MessageLayoutElement *makeImageLayoutElement(const ImagePtr &image,
- const QSize &size) override;
};
class VipBadgeElement : public BadgeElement
{
public:
VipBadgeElement(const EmotePtr &data, MessageElementFlags flags_);
-
-protected:
- MessageLayoutElement *makeImageLayoutElement(const ImagePtr &image,
- const QSize &size) override;
};
class FfzBadgeElement : public BadgeElement
@@ -410,11 +392,6 @@ class FfzBadgeElement : public BadgeElement
public:
FfzBadgeElement(const EmotePtr &data, MessageElementFlags flags_,
QColor color_);
-
-protected:
- MessageLayoutElement *makeImageLayoutElement(const ImagePtr &image,
- const QSize &size) override;
- const QColor color;
};
// contains a text, formated depending on the preferences
diff --git a/src/messages/layouts/MessageLayoutElement.cpp b/src/messages/layouts/MessageLayoutElement.cpp
index bf8aedf2c..e2619282e 100644
--- a/src/messages/layouts/MessageLayoutElement.cpp
+++ b/src/messages/layouts/MessageLayoutElement.cpp
@@ -327,9 +327,11 @@ int LayeredImageLayoutElement::getXFromIndex(size_t index)
}
PriorityImageLayoutElement::PriorityImageLayoutElement(
- MessageElement &creator, ImagePriorityOrder &&order, const QSize &size)
+ MessageElement &creator, ImagePriorityOrder &&order, const QSize &size,
+ std::optional<QColor> bgColor)
: MessageLayoutElement(creator, size)
, order_(std::move(order))
+ , bgColor_(bgColor)
{
this->trailingSpace = creator.hasTrailingSpace();
}
@@ -366,6 +368,11 @@ void PriorityImageLayoutElement::paint(QPainter &painter,
{
if (auto pixmap = imageToUse->pixmapOrLoad())
{
+ if (this->bgColor_ && this->bgColor_.value().isValid())
+ {
+ painter.fillRect(QRectF(this->getRect()),
+ this->bgColor_.value());
+ }
painter.drawPixmap(QRectF(this->getRect()), *pixmap, QRectF());
}
}
@@ -415,34 +422,6 @@ int PriorityImageLayoutElement::getXFromIndex(size_t index)
}
}
-//
-// IMAGE WITH BACKGROUND
-//
-ImageWithBackgroundLayoutElement::ImageWithBackgroundLayoutElement(
- MessageElement &creator, ImagePtr image, const QSize &size, QColor color)
- : ImageLayoutElement(creator, image, size)
- , color_(color)
-{
-}
-
-void ImageWithBackgroundLayoutElement::paint(
- QPainter &painter, const MessageColors & /*messageColors*/)
-{
- if (this->image_ == nullptr)
- {
- return;
- }
-
- auto pixmap = this->image_->pixmapOrLoad();
- if (pixmap && !this->image_->animated())
- {
- painter.fillRect(QRectF(this->getRect()), this->color_);
-
- // fourtf: make it use qreal values
- painter.drawPixmap(QRectF(this->getRect()), *pixmap, QRectF());
- }
-}
-
//
// IMAGE WITH CIRCLE BACKGROUND
//
diff --git a/src/messages/layouts/MessageLayoutElement.hpp b/src/messages/layouts/MessageLayoutElement.hpp
index 606161646..11b933a3e 100644
--- a/src/messages/layouts/MessageLayoutElement.hpp
+++ b/src/messages/layouts/MessageLayoutElement.hpp
@@ -104,7 +104,8 @@ class PriorityImageLayoutElement : public MessageLayoutElement
{
public:
PriorityImageLayoutElement(MessageElement &creator,
- ImagePriorityOrder &&order, const QSize &size);
+ ImagePriorityOrder &&order, const QSize &size,
+ std::optional<QColor> bgColor = std::nullopt);
protected:
void addCopyTextToString(QString &str, uint32_t from = 0,
@@ -116,6 +117,7 @@ protected:
int getXFromIndex(size_t index) override;
const ImagePriorityOrder order_;
+ std::optional<QColor> bgColor_;
};
class LayeredImageLayoutElement : public MessageLayoutElement
@@ -138,19 +140,6 @@ protected:
std::vector<QSize> sizes_;
};
-class ImageWithBackgroundLayoutElement : public ImageLayoutElement
-{
-public:
- ImageWithBackgroundLayoutElement(MessageElement &creator, ImagePtr image,
- const QSize &size, QColor color);
-
-protected:
- void paint(QPainter &painter, const MessageColors &messageColors) override;
-
-private:
- QColor color_;
-};
-
class ImageWithCircleBackgroundLayoutElement : public ImageLayoutElement
{
public: