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 all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Minor: The account switcher is now styled to match your theme. (#4817)
- Minor: Add an invisible resize handle to the bottom of frameless user info popups and reply thread popups. (#4795)
- Minor: The installer now checks for the VC Runtime version and shows more info when it's outdated. (#4847)
- Minor: Reduced image memory usage when running Chatterino for a long time. (#3950)
- Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840)
- Bugfix: Fixed capitalized channel names in log inclusion list not being logged. (#4848)
- Bugfix: Trimmed custom streamlink paths on all platforms making sure you don't accidentally add spaces at the beginning or end of its path. (#4834)
Expand Down
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,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
18 changes: 16 additions & 2 deletions src/messages/Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,12 +432,12 @@ std::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 @@ -495,6 +495,20 @@ 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_;
}

// TODO: Some images contain size as part of their API, we could use that instead of
// this hard-coded size.
return {16, 16};
}

void Image::actuallyLoad()
{
auto weak = weakOf(this);
Expand Down
6 changes: 5 additions & 1 deletion src/messages/Image.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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>
{
Expand All @@ -81,11 +83,11 @@ class Image : public std::enable_shared_from_this<Image>
bool loaded() const;
// either returns the current pixmap, or triggers loading it (lazy loading)
std::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) = delete;
Expand All @@ -97,6 +99,7 @@ class Image : public std::enable_shared_from_this<Image>
Image(qreal scale);

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

Expand All @@ -112,6 +115,7 @@ class Image : public std::enable_shared_from_this<Image>
std::unique_ptr<detail::Frames> frames_{};

friend class ImageExpirationPool;
friend class ImagePriorityOrder;
};

// forward-declarable function that calls Image::getEmpty() under the hood.
Expand Down
93 changes: 93 additions & 0 deletions src/messages/ImagePriorityOrder.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#include "messages/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 (const 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::isStatic() const
{
if (this->animated_ == AnimationFlag::Unknown)
{
this->loadAnimatedFlag();
}

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

// We aren't sure if the image is animated or not because it isn't loaded.
return false;
}

void ImagePriorityOrder::loadAnimatedFlag() const
{
// This function should only be called when the animation flag is unknown
assert(this->animated_ == AnimationFlag::Unknown);

const auto &firstLoaded = this->firstLoadedImage();
if (firstLoaded->loaded())
{
if (firstLoaded->animated())
{
this->animated_ = AnimationFlag::Yes;
}
else
{
this->animated_ = AnimationFlag::No;
}
}
}

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

#include "messages/Image.hpp"

#include <QSize>

#include <vector>

namespace chatterino {

class ImagePriorityOrder final
{
public:
ImagePriorityOrder() = delete;
~ImagePriorityOrder() = default;

explicit ImagePriorityOrder(std::vector<ImagePtr> &&order);

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

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

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

/**
* Returns true if the image is known to be static (i.e. not animated)
* If the image has not been loaded, meaning we don't know if it's static or not, this will return false
*/
bool isStatic() const;

private:
/**
* Attempt to figure out whether this image is animated or not, and store its state in `animated_`
*/
void loadAnimatedFlag() const;

enum AnimationFlag {
Unknown,
No,
Yes,
};

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

} // namespace chatterino
57 changes: 36 additions & 21 deletions src/messages/ImageSet.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
#include "messages/ImageSet.hpp"

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

namespace {

using namespace chatterino;

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

} // namespace

namespace chatterino {

ImageSet::ImageSet()
Expand Down Expand Up @@ -83,31 +95,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
std::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 std::nullopt;
}

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

bool ImageSet::operator==(const ImageSet &other) const
Expand Down
7 changes: 4 additions & 3 deletions src/messages/ImageSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
#include "common/Aliases.hpp"

#include <memory>
#include <optional>
#include <vector>

namespace chatterino {

class ImagePriorityOrder;
class Image;
using ImagePtr = std::shared_ptr<Image>;
ImagePtr getEmptyImagePtr();
Expand All @@ -26,10 +29,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;
std::optional<ImagePriorityOrder> getPriority(float scale) const;

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