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

Synchronize animated image playback inside of LazyStack #300

Open
LosFarmosCTL opened this issue Mar 3, 2024 · 14 comments
Open

Synchronize animated image playback inside of LazyStack #300

LosFarmosCTL opened this issue Mar 3, 2024 · 14 comments
Labels

Comments

@LosFarmosCTL
Copy link

The application I am working on needs to display a lot of animated "emotes" within text messages, often multiple occurrences of the same image, which get out of sync when the individual messages are being scrolled inside of a lazy stack. Since the playback is paused when they move out of view and resumed at the old position when they reappear, the other images have moved on to different frames in the meantime.

Now, I feel like the synchronization aspect should probably be on me (the actual application) to handle, but I can't figure out any way to control the playback of animated images. The image player is not exposed, neither are there methods to control it indirectly.

Would be great to have an option for that, since otherwise the only way I see for me to move forward is building my own custom wrapper for SDWebImage that enables me to do so, which would be a shame compared to just using this more matured library.

import SwiftUI
import SDWebImageSwiftUI

struct ContentView: View {
  let imageURL = URL(string: "https://cdn.betterttv.net/emote/5a8314b61686393232d31027/3x.gif")!

  var body: some View {
    ScrollView {
      LazyVStack {
        ForEach(1...200, id: \.self) { _ in
          AnimatedImage(url: imageURL)
            .resizable()
            .frame(width: 25, height: 25)
        }
      }
    }
  }
}
RPReplay_Final1709503396.MP4
@LosFarmosCTL
Copy link
Author

PS: Things I have tested so far:

  • use WebImage, but even if using it would work, the performance is absolutely unfeasible for my use case, there might be 100+ animated images displayed at once, which is fine using the UIKit implementation but just doesn't scale at all for SwiftUI
  • create the AnimatedImage instance only once as a constant and use that inside of body, doesn't change anything and is definitely not how SwiftUI should work
  • look at the available constructor options and methods of AnimatedImage

@dreampiggy
Copy link
Collaborator

dreampiggy commented Mar 18, 2024

This is a feature which need the upstream SDWebImage support

In SD6, we will support all of playing status on each image view in sync

Currently seems hard to do, until we use a shared SDAnimatedImagePlayer, which need break changes. (Currently, each AnimatedImage hold a different SDAnimatedImagePlayer internally, sync the status between them is hard)

@dreampiggy dreampiggy added the good first issue Good for newcomers label Mar 18, 2024
@LosFarmosCTL
Copy link
Author

In SD6, we will support all of playing status on each image view in sync

Great to hear that this is planned! Is there any rough timeframe for when we could expect SD6?

Depending on that I'll have to decide if I'd just wait for the update or if implementing synchronization in my App using SDAnimatedImagePlayer directly myself is worth it.

@dreampiggy
Copy link
Collaborator

dreampiggy commented May 6, 2024

Seems long time pass.

Do you still have this feature request ?

I'd like to introdce a API, which allows you to sync different image views (For UIKit, it's SDAnimatedImageView, for SwiftUI, it's AnimatedImage).

Just like this (example API usage)

  @State isPlaying: Bool = true
  var player: AnimatedImagePlayer(url: imageURL, isPlaying: $isPlaying)
  var body: some View {
    ScrollView {
      LazyVStack {
        ForEach(1...200, id: \.self) { _ in
          AnimatedImage(player: player)
            .resizable()
            .frame(width: 25, height: 25)
        }
      }
    }

@LosFarmosCTL
Copy link
Author

Do you still have this feature request ?

I'd like to introdce a API, which allows you to sync different image views (For UIKit, it's SDAnimatedImageView, for SwiftUI, it's AnimatedImage).

Just like this (example API usage)

Yes, that would be exactly what I am looking for!

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented May 13, 2024

Seems long time pass.

Do you still have this feature request ?

I'd like to introdce a API, which allows you to sync different image views (For UIKit, it's SDAnimatedImageView, for SwiftUI, it's AnimatedImage).

Just like this (example API usage)

  @State isPlaying: Bool = true
  var player: AnimatedImagePlayer(url: imageURL, isPlaying: $isPlaying)
  var body: some View {
    ScrollView {
      LazyVStack {
        ForEach(1...200, id: \.self) { _ in
          AnimatedImage(player: player)
            .resizable()
            .frame(width: 25, height: 25)
        }
      }
    }

DM reply and post the code here cc @dreampiggy

We can use something like the tag and coordinateSpace API

Public API

public struct AnimatedImageCoordinateTag: EnvironmentKey {
    public static var defaultValue: String? { nil }
}

extension EnvironmentValues {
    public var animatedImageCoordinateTag: String? {
        get { self[AnimatedImageCoordinateTag.self] }
        set { self[AnimatedImageCoordinateTag.self] = newValue }
    }
}

Downstream Usage

struct TestContentView: View {
    let imageURL = URL(string: "https://cdn.betterttv.net/emote/5a8314b61686393232d31027/3x.gif")!

    var body: some View {
        ScrollView {
            LazyVStack {
                ForEach(1 ... 200, id: \.self) { _ in
                    AnimatedImage(url: imageURL)
                        .resizable()
                        .frame(width: 25, height: 25)
                        .environment(\.animatedImageCoordinateTag, "A")
                }
                ForEach(1 ... 200, id: \.self) { _ in
                    AnimatedImage(url: imageURL)
                        .resizable()
                        .frame(width: 25, height: 25)
                        .environment(\.animatedImageCoordinateTag, "B")
                }
            }
        }
    }
}

Internal implementation

public struct AnimatedImage : PlatformViewRepresentable {
    ...
    public func makeUIView(context: Context) -> AnimatedImageViewWrapper {
        let tag = context.environment.animatedImageCoordinateTag
        // Framework internal logic
        ...
    }
    
    public func updateUIView(_ uiView: AnimatedImageViewWrapper, context: Context) {
        let tag = context.environment.animatedImageCoordinateTag
        // Framework internal logic
        ...
    }
}

If you have more question about EnvironmentKey or EnvironmentValues
You can give my implementation of EnvironmentValues and EnvironmentKey a look

@dreampiggy
Copy link
Collaborator

Is this behavior what you want ?

Demo.mov.zip

Note for the Synchronization, we have some hack behavior like pauseable modifier, which may cause other view status mass. Currently I disable that support

@LosFarmosCTL
Copy link
Author

Yes, this looks exactly like what I'm looking for!

@arnauddorgans
Copy link
Contributor

@dreampiggy how did you achieve what you sent in your video?

@dreampiggy
Copy link
Collaborator

dreampiggy commented Oct 19, 2024

@dreampiggy how did you achieve what you sent in your video?

It's a WIP implementation on my branch, which need SDWebImage repo changes at all:

SDWebImageSwiftUI: https://github.com/SDWebImage/SDWebImageSwiftUI/tree/feature/animation_gruop

SDWebImage: https://github.com/dreampiggy/SDWebImage/tree/feature/animated_image_sync_coordinator


But finally I decided not to ship this, because this cause massive API break changes... and have performance regression on normal animated image rendering because of new extra locking

In 5.x API design, each imagView hosts a different player, and status is stored inside the player

This is the main difficult reason to sync status across imageViews. I added another abstract layer to solve this in WIP branch, but it's a hack, honestly

Main logic: dreampiggy/SDWebImage@226c1a6#diff-6ea43bc98bf3d67db6f66b60de02bea0997b22a9586e42ebf85c5e9d25aab58dR143-R151

The actual solution it's to allow different imageViews to host the same player

This will fix the issue more easily, you can control which imageViews group to use player1, which to use player2, and it's alsopossible to achieve the same behavior as current version.

@dreampiggy
Copy link
Collaborator

@arnauddorgans If you really want this, I can have a fresh API design in SDWebImage 6.0 to fix this issue from the root case, instead of fixing and hacking on current 5.x exists API

@arnauddorgans
Copy link
Contributor

@dreampiggy thanks for your answer, I managed to do it on my own by replicating a minimal version of what has been done in SDAnimatedImage

@MainActor
    final class EmotePlayer {
        private static var players: [URL: EmotePlayer] = [:]
        typealias ViewIdentifier = ObjectIdentifier

        let emote: Emote
        private var views: OrderedDictionary<ViewIdentifier, WeakBox<TextAttachmentView>> = [:]
        private var player: SDAnimatedImagePlayer?
        private(set) var image: UIImage?

        init(emote: Emote) {
            self.emote = emote
        }

        private func setAnimatedImage(_ image: SDAnimatedImage) {
            guard player == nil else { return }
            player = .init(provider: image)
            player?.animationFrameHandler = { [weak self] index, image in
                guard let self else { return }
                self.image = image
                for view in views.values {
                    view.value?.layer.setNeedsDisplay()
                }
            }
            playPause()
        }

        private func registerView(_ view: TextWithEmotesView.TextAttachmentView) {
            let identifier = Self.viewIdentifier(view)
            guard views[identifier] == nil else { return }
            views[identifier] = .init(value: view)
            playPause()
        }

        private func unregisterView(identifier: ViewIdentifier) {
            guard views[identifier] != nil else { return }
            views[identifier] = nil
            playPause()
        }

        private func playPause() {
            guard let player else { return }
            if player.isPlaying, views.isEmpty {
                image = nil
                player.stopPlaying()
            } else if !player.isPlaying, !views.isEmpty {
                player.startPlaying()
            }
        }

        static func setAnimatedImage(_ image: SDAnimatedImage, for emote: Emote) {
            player(for: emote).setAnimatedImage(image)
        }

        static func registerView(_ view: TextWithEmotesView.TextAttachmentView, for emote: Emote) {
            player(for: emote).registerView(view)
        }

        static func unregisterView(_ view: TextWithEmotesView.TextAttachmentView, for emote: Emote) {
            unregisterView(identifier: viewIdentifier(view), for: emote)
        }

        static func unregisterView(identifier: ViewIdentifier, for emote: Emote) {
            player(for: emote).unregisterView(identifier: identifier)
        }

        static func player(for emote: Emote, view: TextWithEmotesView.TextAttachmentView) -> EmotePlayer? {
            guard let player = Self.players[emote.url] else { return nil }
            guard player.views[viewIdentifier(view)] != nil else { return nil }
            return player
        }

        private static func player(for emote: Emote) -> EmotePlayer {
            if let player = Self.players[emote.url] {
                return player
            }
            let player = EmotePlayer(emote: emote)
            Self.players[emote.url] = player
            return player
        }

        nonisolated static func viewIdentifier(_ view: TextWithEmotesView.TextAttachmentView) -> ViewIdentifier {
            .init(view)
        }
    }

    final class TextAttachmentView: UIImageView {
        var emote: Emote? {
            didSet {
                guard emote != oldValue else { return }
                if let oldValue {
                    EmotePlayer.unregisterView(self, for: oldValue)
                }
                self.image = nil
                updatePlayerIfNeeded()
                guard let emote else { return }
                SDWebImageManager.shared.loadImage(
                    with: emote.url,
                    context: [.animatedImageClass: SDAnimatedImage.self],
                    progress: nil,
                    completed: { [weak self] image, _, _, _, _, _ in
                        guard let self, emote == self.emote else {
                            return // Emote has changed
                        }
                        if let image = image as? SDAnimatedImage {
                            EmotePlayer.setAnimatedImage(image, for: emote)
                        } else {
                            self.image = image
                        }
                    }
                )
            }
        }

        override func didMoveToWindow() {
            super.didMoveToWindow()
            updatePlayerIfNeeded()
        }

        override func didMoveToSuperview() {
            super.didMoveToSuperview()
            updatePlayerIfNeeded()
        }

        deinit {
            guard let emote else { return }
            let identifier = EmotePlayer.viewIdentifier(self)
            Task { @MainActor in
                EmotePlayer.unregisterView(identifier: identifier, for: emote)
            }
        }

        init() {
            super.init(frame: .zero)
            contentMode = .scaleAspectFit
        }

        @available(*, unavailable)
        required init?(coder: NSCoder) {
            fatalError("init(coder:) has not been implemented")
        }

        private func updatePlayerIfNeeded() {
            guard let emote else { return }
            guard window != nil, superview != nil else {
                return EmotePlayer.unregisterView(self, for: emote)
            }
            EmotePlayer.registerView(self, for: emote)
        }

        override func display(_ layer: CALayer) {
            guard let emote, let player = EmotePlayer.player(for: emote, view: self), let image = player.image else {
                return super.display(layer)
            }
            layer.contentsScale = image.scale
            layer.contents = image.cgImage
        }
    }

I didn't override startAnimating and stopAnimating methods, do you know if it is better for performances?
Thanks

@dreampiggy
Copy link
Collaborator

dreampiggy commented Oct 28, 2024

Your impl looks OK because it just do the similar thing as the changes in my branch, in concept.

I didn't override startAnimating and stopAnimating methods, do you know if it is better for performances?
Thanks

Since you don't need a UIImageView subclass, you don't need to consider this. All the extra complexity of SDAniamtedImageView comes from UIKit, its super class, has some API which may be applied for both UIImage and SDAniamtedImage, which need some hacky code

@arnauddorgans
Copy link
Contributor

@dreampiggy Thanks a lot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants