Skip to content

Fix the issue when using WebImage with some transition like scaleEffect, each time the new state update will cause unused image fetching #92

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

Merged
merged 5 commits into from
Apr 1, 2020

Conversation

dreampiggy
Copy link
Collaborator

@dreampiggy dreampiggy commented Mar 31, 2020

See our demo:

WebImage(url: url, options:[.delayPlaceholder])
    .placeholder(.wifiExclamationmark)
    .scaleEffect(self.scale)
    .gesture(MagnificationGesture(minimumScaleDelta: 0.1).onChanged { value in
        let delta = value / self.lastScale
        self.lastScale = value
        let newScale = self.scale * delta
        self.scale = min(max(newScale, 0.5), 2)
    }.onEnded { value in
        self.lastScale = 1.0
    })

Before this fix, when network load failed, each time you zoom your finder, the new SDWebImage loadImage function call will trigger, this cause huge performance cost, worstly, the UI part will show a placeholder with flashback.

To workaround with this, we roll back to load the image in WebImage's first Appear. However, we have to solve the issue #20 #19

So, this time, we use a trick (smart ?) solution: We query the memory cache firstlly during WebImage init method. This is fast because it just a NSCache objectForKey call.

If memory cache hit, the image does not show placeholder or empty, so fix the editMode issue.

… during transition state, like scaleEffect. In this time, we does not trigger a actualy image loading, only query the memory cache for quickly placeholder
@dreampiggy dreampiggy added fix webimage WebImage struct labels Mar 31, 2020
@dreampiggy dreampiggy force-pushed the fix_query_image_extra_times branch from bc28d08 to 78d9bfb Compare March 31, 2020 12:23
@dreampiggy dreampiggy changed the title Fix the issue when using WebImage with some transition like scaleEffect, each time the new state update will cause unused image loading Fix the issue when using WebImage with some transition like scaleEffect, each time the new state update will cause unused image fetching Mar 31, 2020
@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #92 into master will decrease coverage by 0.82%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   74.28%   73.45%   -0.83%     
==========================================
  Files           9        9              
  Lines         696      727      +31     
==========================================
+ Hits          517      534      +17     
- Misses        179      193      +14     
Flag Coverage Δ
#ios 73.45% <75.00%> (-0.83%) ⬇️
Impacted Files Coverage Δ
SDWebImageSwiftUI/Classes/ImageManager.swift 70.73% <67.85%> (-3.35%) ⬇️
SDWebImageSwiftUI/Classes/WebImage.swift 80.00% <100.00%> (-2.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b11cce...ffeea1a. Read the comment docs.

if imageManager.isFirstLoad {
imageManager.load()
// this prefetch the memory cache of image, to immediately render it on screen
// this solve the case when `onAppear` not been called, for example, some transaction indeterminate state, SwiftUI :)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why SwiftUI Suck here ? The layout engine create a new View struct, touch the body value, but not trigger the onAppear, what does this used for ?

…same time, we need to write correct code to query memory cache only
@dreampiggy dreampiggy merged commit 0dcefe5 into master Apr 1, 2020
@dreampiggy dreampiggy deleted the fix_query_image_extra_times branch April 1, 2020 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix webimage WebImage struct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant