search

SwiftcordApp / Swiftcord

A fully native Discord client for macOS built 100% in Swift!
https://swiftcordapp.github.io/Swiftcord/
GNU General Public License v3.0
1.85k stars 84 forks source link

[Bug]: Gifs are laggy #118

Open Im-Fran opened 1 year ago

Im-Fran commented 1 year ago

Describe the Bug

Bug Description The gifs that are shown are laggy, and even more when viewing a folder.

Reproducing the Bug

  1. Hover a gif
  2. See the lag

Version

0.5.1 (Build 15)

Category

Media (images, audio, video etc.)

Relevant Log Output

No response

Screenshots

This is from Official Discord:

This is from SwiftCord

Additional Info

No response

cryptoAlgorithm commented 1 year ago

This appears to be yet another issue related to SwiftyGif not being so swifty after all. I might have to implement a more performant implementation using CoreGraphics directly.

ErrorErrorError commented 1 year ago

@cryptoAlgorithm you could consider using Nuke which does simplify a lot of the image loading if that's something you don't want to reimplement from scratch.

cryptoAlgorithm commented 1 year ago

@ErrorErrorError That's interesting, it like a good alternative to replace both CachedAsyncImage and SwiftyGif 🤔. And it even has first-class SwiftUI support. I will consider refactoring with Nuke in a PR to evaluate its advantages soon.

cryptoAlgorithm commented 1 year ago

@ErrorErrorError I dug deeper and it appears that we are back with the same issues as before. Nuke is a performant image loading solution, but out of the box it does not support rendering animated GIFs (or any other format not natively supported by the system). I'll have to integrate it with another library which actually renders GIFs, and the bottleneck we're facing resides in this rendering library.

It's a bit of a bummer, but I might still consider migrating to Nuke from CachedAsyncImage as it appears to have better caching and loading heuristics.

cryptoAlgorithm commented 1 year ago

After throwing together some quick code to use Nuke instead of CachedAsyncImage to evaluate its suitability, I have come to the conclusion that migration isn't worth it as it actually leads to (very noticeably) worse scrolling performance, albeit with slightly better caching (and so lower network activity). I feel that scrolling performance impacts virtually all users, so saving a few kBs of network activity is not worth it for the significant scroll jank.