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

Images for emotes, badges, etc. are not lazily loaded as intended #3929

Open
4 tasks done
dnsge opened this issue Aug 17, 2022 · 0 comments
Open
4 tasks done

Images for emotes, badges, etc. are not lazily loaded as intended #3929

dnsge opened this issue Aug 17, 2022 · 0 comments
Labels
enhancement Feature: Emotes Issues related to Emotes

Comments

@dnsge
Copy link
Contributor

dnsge commented Aug 17, 2022

Checklist

  • I'm reporting a problem with Chatterino
  • I've verified that I'm running the most recent nightly build or stable release
  • I've looked for my problem on the wiki
  • I've searched the issues and pull requests for similar looking reports

Describe your issue

As @pajlada hinted at in #3915 (comment), lazy loading for images that aren't visible (e.g. in a different split tab) is broken.

Currently, images are supposed to be accessed by the pixmapOrLoad method in Image:

boost::optional<QPixmap> Image::pixmapOrLoad() const
{
assertInGuiThread();
this->load();
return this->frames_->current();
}

This lazy loading works as long as no other code calls Image::load. However, in ImageSet, we call load manually:

const ImagePtr &ImageSet::getImageOrLoaded(float scale) const
{
auto &&result = getImagePriv(*this, scale);
// get best image based on scale
result->load();

The intent behind calling load is that we will attempt to load the highest resolution of the image but fall back to any already loaded resolutions if they exist (see rest of method body).

This wouldn't be a problem if getImageOrLoaded was called in the rendering logic which only runs for the visible splits. However, this method is actually called in the message layout logic, which runs for all splits as messages are added:

void EmoteElement::addToContainer(MessageLayoutContainer &container,
MessageElementFlags flags)
{
if (flags.hasAny(this->getFlags()))
{
if (flags.has(MessageElementFlag::EmoteImages))
{
auto image =
this->emote_->images.getImageOrLoaded(container.getScale());
if (image->isEmpty())
return;

I wanted to detail this problem and my investigation into it here so we can find a good solution for properly doing lazy loading. I couldn't immediately think of a solution that uses already-loaded images while queueing up higher resolution images.

This problem reduces the effectiveness of #3915 too, and fixing this problem should also lead to a reduction in memory usage by images.

OS and Chatterino Version

Chatterino 7.3.5 DEBUG (commit 2234670) built with Qt 5.15.2 Running on macOS 12.3, kernel: 21.4.0

@dnsge dnsge added the issue-report An issue reported by a user. label Aug 17, 2022
@Felanbird Felanbird added enhancement Feature: Emotes Issues related to Emotes and removed issue-report An issue reported by a user. labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature: Emotes Issues related to Emotes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants