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

Unnecessary image processing applied #711

Closed
nakiwo opened this issue Jun 16, 2017 · 2 comments
Closed

Unnecessary image processing applied #711

nakiwo opened this issue Jun 16, 2017 · 2 comments

Comments

@nakiwo
Copy link

nakiwo commented Jun 16, 2017

What

I'm testing ImageProcessor behavior with 84b582e .
And found sometimes unnecessary image processing applied.

Reproduce

I modified cell configuration method on
Kingfisher-Demo/ViewController.swift as follows:

let processor = BlurImageProcessor(blurRadius: 4)

_ = (cell as! CollectionViewCell).cellImageView.kf.setImage(with: url,
    placeholder: nil,
    options: [.transition(ImageTransition.fade(0.2)), .processor(processor)],
    progressBlock: { receivedSize, totalSize in
        print("\(indexPath.row + 1): \(receivedSize)/\(totalSize)")
    },
    completionHandler: { image, error, cacheType, imageURL in
        print("\(indexPath.row + 1): Finished")
    })

Then scoroll through from top to bottom.
I think this produces 10 cached images that are applied blur filter.
However there are 14 images in com.onevcat.Kingfisher.ImageCache.default directory.

0dac4f4941be6a51763b3713eca252c7
3db5a4d7015ca13059b44835e0629ad4
5c353c05e21a4a8d5190e420cc026408
331f48cef4038ba59627ff133212c3a0
725dac71dd2ca6764cad85348fa4b73a
881c976ffc3e7c113a69599a785c5249
6276089266b6aacd864aa51e946b38b9
abb0b79ae70b4d707cb4fab0915c02cc
b97dbd68138f90ca5f5f9c817d625a52
b678f087ac033600f6e4da991925bc8d
bf8bb2352ac143346b8ab83d0abe5ab5
c5c43eed9ed685b3d5d535173b5b4c31
caa8558f77eeedb9fce51041e59cc336
e8bf9406e69c9171be257b36044d02e0

Above includes:

  • 10 original images.
  • 4 blurred images.

Demo app shows all images blurred.

I wrote log output code in BlurImageProcessor.process as follows:

// BlurImageProcessor

public func process(item: ImageProcessItem, options: KingfisherOptionsInfo) -> Image? {
    switch item {
    case .image(let image):
        NSLog("🍏1")
        let radius = blurRadius * options.scaleFactor
        return image.kf.blurred(withRadius: radius)
    case .data(_):
        NSLog("🍏2")
        return (DefaultImageProcessor.default >> self).process(item: item, options: options)
    }
}

Then scoroll through from top to bottom.
Console output is like this:

2017-06-16 17:33:25.441 Kingfisher-Demo[6112:1539751] 🍏1
2017-06-16 17:33:25.531 Kingfisher-Demo[6112:1539751] 🍏1
2017-06-16 17:33:25.631 Kingfisher-Demo[6112:1539751] 🍏1
7: Finished
6: Finished
4: Finished
5: Finished
3: Finished
2: Finished
1: Finished
4: Finished
2017-06-16 17:33:27.219 Kingfisher-Demo[6112:1539751] 🍏1
2017-06-16 17:33:27.319 Kingfisher-Demo[6112:1539751] 🍏1
5: Finished
2017-06-16 17:33:27.418 Kingfisher-Demo[6112:1539751] 🍏1
6: Finished
2017-06-16 17:33:27.552 Kingfisher-Demo[6112:1539751] 🍏1
7: Finished
8: Finished
2017-06-16 17:33:27.753 Kingfisher-Demo[6112:1539751] 🍏1
9: Finished
2017-06-16 17:33:28.069 Kingfisher-Demo[6112:1539751] 🍏1
10: Finished
.
.
.

Excepected results in this situation are:

  • ImageProcesser.process runs once per image.
  • disk cache contains only 10 blurred images.
@onevcat
Copy link
Owner

onevcat commented Jun 16, 2017

Hi,

For most part it is as expected, expect that the blurred images are not cached (and I will fix it later)

Here is the reason:

  1. The original images are cached due to the prefetcher. You may not notice that we have a prefetch support in the demo, which will try to get original images (and cache them).
  2. The first four blurred images are downloaded and processed, then cached as normal.
  3. For the later blurred images, there is an optimization in Kingfisher to check whether an original image is already in cache. If so, that image will not be downloaded again but only be processed with input cache.
  4. And I forgot to store the processed images in cache from step 3!

I will give it a fix now. Thank you!

@onevcat
Copy link
Owner

onevcat commented Jun 16, 2017

3.10.2 released and this should be fixed.

@onevcat onevcat closed this as completed Jun 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants