Bug: State/Bindings not working in AnimatedImage view type initializers

[TysonParks]

First of all, thanks so much for making SDWebImage and quickly porting it to SwiftUI! I can't tell you how empowering this framework is for the current app I am building.

I've found that binding to the initializers for AnimatedImage view types are not updating the view. I've tested the (url:) and (name:) initializers. I assume that this has to do with AnimatedImage being based on UIKit/AppKit UIView representable instead of SwiftUI's native Image view type because this issue does not occur with the WebImage view type.

Here is some test code to quickly demonstrate the issue:
https://gist.github.com/TysonParks/e6798fe0e5705c44a0258f77315beac5

This is my first time reporting an Issue on Github so my apologies for any noob mistakes.

Thanks.

[dreampiggy]

Hi. Our SwiftUI demo (mentioned in readme), already have a Animated Button, which let you to start/stop the animation, using the Binding<Bool> API. Could you have a check of that ?

I'll test your demo quickly as well. I guess this may be something about SDK issue, but usage issue.

[dreampiggy]

I misunderstand your issue.

You means: I can not change the URL of AnimatedImage, if it already get initialized ?

This maybe logic...Because the URL is readonly after first setup. If you need this, I can provide a fix for that. But this may a little impact the performance.

Previouslly we have this feature, which means, each time this ImageView appears, it trigger a new sd_setImageWithURL: method call. But after v0.4.2, I change it from using a @OversedObject model contains URL, into a @State usage, which does not trigger extra sd_setImageWithURL: call after first loaded. So that even you update the URL, it does not change.

Unlike native SwiftUI View, UIVIewRepresentable struct, does not get a new created one when state changes. Because SwiftUI team think: UIVIewRepresentable which is backed by a UIView class, it's not a trival struct to construct everytime. Instead, it call the method on exist View: updateUIView(:).

Previouslly, I don't get this behavior during test, assume it always create new AnimatedImage struct, so nothing wrong. But now, we should reveret back the code.

[TysonParks]

Yes, that's right: I can not change the image being shown/played within the AnimatedImage view after it is initialized. I've tested the AnimatedImage being set/initialized both by init(url:) or init(name:).

Are you saying that there is a more efficient/better method to change the image assignment of an AnimatedImage via binding? If so I'm happy to adjust my usage.

Yes, I agree it would make sense to revert back to give us the ability to bind the image assignment in the same way that it works for the standard Image and your WebImage view types.

[dreampiggy]

@TysonParks Does this the designed behavior ?

video.mov.zip

I've already fixed it, PR is here: #62

This will cause extra query to sd_setImage(with:), because now, we can not assume anything of previous state. Once the updateUIView(:) is called, we need to refresh anything to the Source of Truth, re-query the image URL (for Data and Name initializer, re-generate the image, because user may also want to change the Name and Data using @State, right ?).

This may cause a little performance cost, because when using AnimatedImage in a List, during the list scroll, you'll receive a new updateUIView as well. When you enter the Edit mode of List, receive a new call. All of them means we call extra sd_setImage(with:). But however, SDWebImage itself, have a logic to avoid the duplicated query (it use the cache, and for downloaded URL, it re-use the same URL request with token). Seems less impact.

For Data and Name initializer, we have no cache yet, so everythingupdateUIView cause a re-generated of SDAnimatedImage (implementation detail), which need to decode the first frame in main queue. Sounds a little frustrating, but this is the only way to make your @State refresh works.

[dreampiggy]

Although this change may have perfoamance cost. But a framework is based on the function correctness. So whatever, we should do this changes.

[TysonParks]

Yes, from your video this looks perfect. Testing the branch on my app now. Thanks for the quick fix!

I'll definitely keep an eye on performance and give you any feedback that I can. I'll admit, I'm new to SwiftUI and your wonderful SDWebImage framework and I haven't yet learned the ins and outs of the ViewRepresentable implementations.

That makes sense that usage in List will have a performance hit. I'll take that into consideration while building my app and if I come across any possible enhancements I'll be sure to share with you.

I agree with you that despite the performance cost, it seems ideal to maintain the functional pattern of SwiftUI and it's implementation of state/binding.

[TysonParks]

I've tested the new branch and the image changing works great! However I found a new bug.

For an AnimatedImage initialized with init(url:), changing the speed on the animation causes the current frame to reset to 0, causing a stuttering effect.

The behavior remains normal for an AnimatedImage initialized with init(name:) and I have not tested the init(data:) initializers.

If I understand what you've written above this is likely a side effect of having to "re-query the image URL" as you mentioned? If the url init case forces a full reload then perhaps the fix can be to capture the current frame and then call seekToFrameAtIndex() to get back to playing current frame upon reload? But you probably have a better idea of how to approach this...

Here is a test App to demonstrate the issue:
https://github.com/TysonParks/SDWebImageSwiftUITests

(It's probably not necessary but you can change the pod target and run 'pod update' in the podfile to test with the SDWebImageSwiftUI master branch. I'm not a git master so I don't know how to set up separate framework branch testing in a more efficient way.)

Thanks again for your help!

[dreampiggy]

changing the speed on the animation causes the current frame to reset to 0, causing a stuttering effect.

You means, the new API playbackRate(:) to change the speed ?

Maybe I need to re-write the part of code. Make all the View Modifier using the @ObservedObject to allows the dynamic changes. The WebImage and AnimatedImage should have different process about the configuration state (like placeholder, indicator, customLoopCount).

[dreampiggy]

I found a way to avoid SwiftUI's this smart behavior and comply with the idea single source of truth.

I bind the String Name, Data, URL to current image or View object, if these value does not changed, do not trigger a new refresh. Only these value changes, we re-generate everthing from the currernt source of truth.

This may help to avoid some issue. Wait me for MR.

[dreampiggy]

video2.mp4.zip

Fix all the Data, Name, URL initializer behavior. All of them can now supports dynamic changes, and do not cause extra re-generation

Re-generation may break your usage like those playbackRate changes, because the re-generated image may not always the same as before (UIImage compare is not always equal). When trigger a new setImage(_:), we stop everything and restart, this is behavior of SDWebImage. The name initializer, hold a global cache as UIKit, so the image instance is always the same and earily return without stop. See details code here

See the video. I'd update the PR.

[TysonParks]

I just looked at the video and it's looking great!

I'm headed to bed now but will test this new patch in the morning.

Perhaps it is all fixed by your patch here, but I just realized that the animation is reset to the first frame also when applying view modifiers and modulating their properties via Slider bindings, same as I was using Slider to modulate playBackRate() in my example app. So far I've found this is true with .offset(), .position(), scaleEffect(), and .rotationEffect().

Again, perhaps this is fixed with your patch already. I'll test tomorrow and let you know what I. find.

Thanks again for the quick feedback and fixes!

[dreampiggy]

but I just realized that the animation is reset to the first frame also when applying view modifiers and modulating their properties via Slider bindings,

Now, everything public API of ViewModifer supports dynamic changes. I've already update them all, not limited to playbackRate(_:)

[TysonParks]

You're fast! Great, I'm super excited to test in the morning!