[WIP] add ImageModifier

[ethansinjin]

Resolves #810

This adds an ImageModifier, allowing edits to be performed on a concrete type of Image (ex. UIImage) right before the image is used/displayed.

I've created the KingfisherOptionsInfoItem case and ImageModifier struct, but the actual modifier isn't being run yet. I'd like advice on where exactly to run the modifier. I think it should go in downloadAndCacheImage and tryToRetrieveImageFromCache, but I'm not sure; there might be a better place for it.

TODO:

• add ImageModifierTests
• execute the modifier's modify() method somewhere in KingfisherManager.swift

[onevcat]

@ethansinjin

Thanks for the p-r. I have to say it is a much bigger one than I expected! IMO, it would be sufficient to just provide a single and light wrapper type to meet all the needs, just similar to the existing RequestModifier. Something like this:

import Foundation

/// Request modifier of image downloader.
public protocol ImageModifier {
    func modified(for image: Image) -> Image?
}

struct DefaultImageModifier: ImageModifier {
    static let `default` = DefaultImageModifier()
    private init() {}
    func modified(for image: Image) -> Image? {
        return image
    }
}

public struct AnyImageModifier: ImageModifier {
    
    let block: (Image) -> Image?
    
    public func modified(for image: Image) -> Image? {
        return block(image)
    }
    
    public init(modify: @escaping (Image) -> Image? ) {
        block = modify
    }
}

By passing a modifying block to AnyImageModifier, users could do everything they want to apply change to the image. Currently it seems that you are following the way of ImageProcessor, to have identifier and cascading of modifiers. However, the identifiers there are mainly used for purpose of data persisting and caching. It is not the case here for image modifier.

How do you think about it? Does the identifier take any responsibility here?

[ethansinjin]

That's a fair point; I guess it is a bit over-architected. I'll rework it to be more like RequestModifier.

A few thoughts:

• I still think we should include a RenderingModeImageModifier and FlipsForRightToLeftLayoutDirectionImageModifier because I'm pretty sure those are the most common reasons this class is needed.
• I think the return parameter should be Image instead of Image?, because at the point of execution of the ImageModifier, a valid Image is already required as a parameter. So, if the change isn't possible, couldn't the original image just be returned?

[ethansinjin]

Alright, please let me know what you think of the changes! @onevcat

[onevcat]

@ethansinjin Great work!

I still think we should include a RenderingModeImageModifier and FlipsForRightToLeftLayoutDirectionImageModifier because I'm pretty sure those are the most common reasons this class is needed.

Yes, I totally agree with you.

I think the return parameter should be Image instead of Image?, because at the point of execution of the ImageModifier, a valid Image is already required as a parameter. So, if the change isn't possible, couldn't the original image just be returned?

The original thought is letting users have a chance to return nil if the do not want to set the image at the last minute. But I guess it could rarely happen so current implementation looks fine!

[onevcat]

I think I could do the last part of applying the modifier later. Once it has been done, I will release an update version. Thank you!

[ethansinjin]

Thanks for merging!