Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazily load images on paint, not layout #3950

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
- Minor: Added highlights for `Elevated Messages`. (#4016)
- Minor: Removed total views from the usercard, as Twitch no longer updates the number. (#3792)
- Minor: Load missing messages from Recent Messages API upon reconnecting (#3878, #3932)
- Minor: Reduced image memory usage when running Chatterino for a long time. (#3915)
- Minor: Reduced image memory usage when running Chatterino for a long time. (#3915, #3950)
- Minor: Added the ability to execute commands on chat messages using the message context menu. (#3738, #3765)
- Minor: Added settings to toggle BTTV/FFZ global/channel emotes (#3935, #3990)
- Minor: Added an option to display tabs on the right and bottom. (#3847)
Expand Down
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ set(SOURCE_FILES
messages/Emote.hpp
messages/Image.cpp
messages/Image.hpp
messages/ImagePriorityOrder.cpp
messages/ImagePriorityOrder.hpp
messages/ImageSet.cpp
messages/ImageSet.hpp
messages/Link.cpp
Expand Down
14 changes: 12 additions & 2 deletions src/messages/Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,12 @@ boost::optional<QPixmap> Image::pixmapOrLoad() const
// See src/messages/layouts/MessageLayoutElement.cpp ImageLayoutElement::paint, for example.
this->lastUsed_ = std::chrono::steady_clock::now();

this->load();
this->loadIfUnloaded();

return this->frames_->current();
}

void Image::load() const
void Image::loadIfUnloaded() const
{
assertInGuiThread();

Expand Down Expand Up @@ -455,6 +455,16 @@ int Image::height() const
return 16;
}

QSize Image::size() const
pajlada marked this conversation as resolved.
Show resolved Hide resolved
{
assertInGuiThread();

if (auto pixmap = this->frames_->first())
return pixmap->size() * this->scale_;
else
return {16, 16};
pajlada marked this conversation as resolved.
Show resolved Hide resolved
}

void Image::actuallyLoad()
{
NetworkRequest(this->url().string)
Expand Down
6 changes: 5 additions & 1 deletion src/messages/Image.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ namespace detail {
class Image;
using ImagePtr = std::shared_ptr<Image>;

class ImagePriorityOrder;

/// This class is thread safe.
class Image : public std::enable_shared_from_this<Image>, boost::noncopyable
{
Expand All @@ -68,11 +70,11 @@ class Image : public std::enable_shared_from_this<Image>, boost::noncopyable
bool loaded() const;
// either returns the current pixmap, or triggers loading it (lazy loading)
boost::optional<QPixmap> pixmapOrLoad() const;
void load() const;
qreal scale() const;
bool isEmpty() const;
int width() const;
int height() const;
QSize size() const;
bool animated() const;

bool operator==(const Image &image) const;
Expand All @@ -84,6 +86,7 @@ class Image : public std::enable_shared_from_this<Image>, boost::noncopyable
Image(qreal scale);

void setPixmap(const QPixmap &pixmap);
void loadIfUnloaded() const;
void actuallyLoad();
void expireFrames();

Expand All @@ -99,6 +102,7 @@ class Image : public std::enable_shared_from_this<Image>, boost::noncopyable
std::unique_ptr<detail::Frames> frames_{};

friend class ImageExpirationPool;
friend class ImagePriorityOrder;
};

class ImageExpirationPool
Expand Down
85 changes: 85 additions & 0 deletions src/messages/ImagePriorityOrder.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
#include "ImagePriorityOrder.hpp"

#include "common/QLogging.hpp"
#include "messages/Image.hpp"

namespace chatterino {

ImagePriorityOrder::ImagePriorityOrder(std::vector<ImagePtr> &&order)
pajlada marked this conversation as resolved.
Show resolved Hide resolved
: order_(std::move(order))
{
assert(!this->order_.empty());
}

ImagePriorityOrder::ImagePriorityOrder(ImagePriorityOrder &&other)
: order_(std::move(other.order_))
{
}

const ImagePtr &ImagePriorityOrder::firstLoadedImage() const
{
for (auto &img : this->order_)
{
if (img != nullptr && !img->isEmpty() && img->loaded())
{
return img;
}
}

// fallback to first-priority image if nothing else is loaded
return this->order_.front();
}

const ImagePtr &ImagePriorityOrder::getLoadedAndQueue() const
{
const auto &firstLoaded = this->firstLoadedImage();
if (firstLoaded != this->order_.front())
{
// The image we have already loaded isn't the desired image.
// Queue up loading the desired image, but use the already loaded one
// for painting this time around.
this->order_.front()->loadIfUnloaded();
}
else if (!firstLoaded->loaded())
{
// firstLoaded is the first-priority image, but it isn't loaded.
firstLoaded->loadIfUnloaded();
}

return firstLoaded;
}

QSize ImagePriorityOrder::firstLoadedImageSize() const
{
return this->firstLoadedImage()->size();
}

bool ImagePriorityOrder::notAnimated() const
{
if (this->animated_ != AnimationFlag::Unknown)
{
// We have already checked the loaded image at some point
return this->animated_ == AnimationFlag::No;
}

const auto &firstLoaded = this->firstLoadedImage();
if (firstLoaded->loaded())
{
if (firstLoaded->animated())
{
this->animated_ = AnimationFlag::Yes;
return false;
}
else
{
this->animated_ = AnimationFlag::No;
return true;
}
pajlada marked this conversation as resolved.
Show resolved Hide resolved
}

// We aren't sure if the image is animated or not because it isn't loaded.
this->animated_ = AnimationFlag::Unknown;
return false;
}

} // namespace chatterino
32 changes: 32 additions & 0 deletions src/messages/ImagePriorityOrder.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#pragma once

#include "messages/Image.hpp"

#include <QSize>
#include <vector>

namespace chatterino {

class ImagePriorityOrder
pajlada marked this conversation as resolved.
Show resolved Hide resolved
{
public:
ImagePriorityOrder(std::vector<ImagePtr> &&order);
ImagePriorityOrder(ImagePriorityOrder &&other);

ImagePriorityOrder(const ImagePriorityOrder &other) = delete;
ImagePriorityOrder &operator=(const ImagePriorityOrder &other) = delete;
ImagePriorityOrder &operator=(ImagePriorityOrder &&other) = delete;

const ImagePtr &firstLoadedImage() const;
const ImagePtr &getLoadedAndQueue() const;
QSize firstLoadedImageSize() const;
bool notAnimated() const;

private:
enum AnimationFlag { Unknown, No, Yes };

std::vector<ImagePtr> order_;
mutable AnimationFlag animated_ = AnimationFlag::Unknown;
};

} // namespace chatterino
55 changes: 33 additions & 22 deletions src/messages/ImageSet.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
#include "messages/ImageSet.hpp"
#include "ImageSet.hpp"

#include "messages/ImagePriorityOrder.hpp"
#include "singletons/Settings.hpp"

namespace chatterino {

namespace {
bool isValidImagePtr(const ImagePtr &img)
{
return img && !img->isEmpty();
}
} // namespace

ImageSet::ImageSet()
: imageX1_(Image::getEmpty())
, imageX2_(Image::getEmpty())
Expand Down Expand Up @@ -84,31 +92,34 @@ const std::shared_ptr<Image> &getImagePriv(const ImageSet &set, float scale)
return set.getImage1();
}

const ImagePtr &ImageSet::getImageOrLoaded(float scale) const
const ImagePtr &ImageSet::getImage(float scale) const
{
auto &&result = getImagePriv(*this, scale);

// get best image based on scale
result->load();

// prefer other image if selected image is not loaded yet
if (result->loaded())
return result;
else if (this->imageX3_ && !this->imageX3_->isEmpty() &&
this->imageX3_->loaded())
return this->imageX3_;
else if (this->imageX2_ && !this->imageX2_->isEmpty() &&
this->imageX2_->loaded())
return this->imageX2_;
else if (this->imageX1_->loaded())
return this->imageX1_;
else
return result;
return getImagePriv(*this, scale);
}

const ImagePtr &ImageSet::getImage(float scale) const
boost::optional<ImagePriorityOrder> ImageSet::getPriority(float scale) const
{
return getImagePriv(*this, scale);
std::vector<ImagePtr> result;
pajlada marked this conversation as resolved.
Show resolved Hide resolved
result.reserve(4);

auto pushIfNotEmpty = [&result](const ImagePtr &img) {
if (isValidImagePtr(img))
{
result.push_back(img);
}
};

pushIfNotEmpty(this->getImage(scale));
pushIfNotEmpty(this->imageX3_);
pushIfNotEmpty(this->imageX2_);
pushIfNotEmpty(this->imageX1_);

if (result.empty())
{
return boost::none;
}

return ImagePriorityOrder(std::move(result));
}

bool ImageSet::operator==(const ImageSet &other) const
Expand Down
9 changes: 6 additions & 3 deletions src/messages/ImageSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@

#include "messages/Image.hpp"

#include <boost/optional.hpp>
#include <vector>

namespace chatterino {

class ImagePriorityOrder;

class ImageSet
{
public:
Expand All @@ -19,10 +24,8 @@ class ImageSet
const ImagePtr &getImage2() const;
const ImagePtr &getImage3() const;

/// Preferes getting an already loaded image, even if it is smaller/bigger.
/// However, it starts loading the proper image.
const ImagePtr &getImageOrLoaded(float scale) const;
const ImagePtr &getImage(float scale) const;
boost::optional<ImagePriorityOrder> getPriority(float scale) const;

bool operator==(const ImageSet &other) const;
bool operator!=(const ImageSet &other) const;
Expand Down