A humble suggestion on round corner processor background color

[EdgarDegas]

First of all, yes, I've read your suggestion of using .cacheSerializer(FormatIndicatedCacheSerializer.png).

But what do you say if we change RoundCornerImageProcessor.init into:

public init(
    cornerRadius: CGFloat,
    targetSize: CGSize? = nil,
    roundingCorners corners: RectCorner = .all,
    backgroundColor: KFCrossPlatformColor = .clear  // used to be: ? = nil
)

while extending KFCrossPlatformColor to have a .clear property which returns UIColor.clear or NSColor.clear?

I hope this post won't annoy you. I tend to say it is merely a humble opinion, considering I'm making a suggestion on your suggestion (of making the cache a PNG).

You see, when setting an image with URL, the developer might not know the exact codec of the image. So when using RoundCornerImageProcessor without specifying the backgroundColor, which is then left as nil, we're kinda having an inconsistency here.

The first time the processor crops the image, the cropped area is transparent. But in the next app launch, it becomes white.

For whoever gets confused here, the image behind the URL is a JPEG, which lacks the alpha channel, got cached onto the disk.

We can, of course, cache it as PNG, but it would costs more space than a JPG file.

We can specify the backgroundColor specifically as .clear, but what about the consistency? Having a nil backgroundColor, we are getting different results not only between different image codecs, but also between app launches.

What do you think?

Hope I don't sound aggressive.

[onevcat]

@EdgarDegas

Thank you for starting this discussion. You are being too polite. Please, there's no need for such modesty 😂.

The issue here is that Kingfisher will by default save the processed result on the disk according to the original format of the image. Since JPEG images do not have an alpha channel, even if the background color is set to .clear as in your example, it will still be forcibly converted to a white background upon storage if the original image is a JPEG. Therefore, whether it's nil or .clear, when saving as JPG and PNG respectively, there will inevitably be inconsistencies.

It's possible that I haven't fully understood your point... If what you're suggesting is something else, please clarify further. Thank you!

---

Regarding the current design, I'd like to add a bit more. This was actually a rather difficult choice:

Users of RoundCornerImageProcessor are likely to expect a transparent background in the rounded corners. For the sake of API usability, we could indeed force the use of PNG serializer when encountering the RoundCornerImageProcessor. However, compared to the potential complications and understanding difficulties that this implicit setting might bring, the current approach of "requiring users to explicitly set PNG format" is a way of putting the issue "out in the open".

[EdgarDegas]

Thank you for replying so quickly.

After a little reading on the source code, I find my backgroundColor: KFCrossPlatformColor = .clear thing is pointless. Please ignore that.

My point is, if I leave the background color nil, I'm expecting it to be transparent on the cropped area. So I was hoping that the cached JPEG file would go through the round-cornering process again.

To put it into pseudo code:

func retrieveImageInDiskCache(
    forKey key: String,
    options: KingfisherParsedOptionsInfo
) -> UIImage {
    let data = diskCache.retrieve(forKey: key)
    if data.isJPEG && options.wantsRoundCornerWithTransparency {
        let processors = options.allRoundCornerProcessors  // seems impossible for now
        let dataProcessor = ImageDataProcessor(data: data, options: .processor(processors))
        return dataProcessor.process()
    } else {
        let image = options.cacheSerializer.image(with: data)
        return image
    }
}

It seems to me that we would need another way to describe the processors.

I don't know if I expressed myself clearly this time. 😂