kean / Nuke

Image loading system
https://kean.blog/nuke
MIT License
7.94k stars 519 forks source link

Image scaling bug #791

Open prabhuamol opened 3 weeks ago

prabhuamol commented 3 weeks ago

We are sometimes noticing when images are scaled using .thumbnailKey they appear blurry. I wonder if its cause infunc makeThumbnail(data: Data, options: ImageRequest.ThumbnailOptions) -> PlatformImage? scale is not taken into account when returning back the UIImage. Which is causing the dimensions of the image to be reported incorrectly.

We do some post processing via prepareThumbnail if the returned thumbnail size does not match the view size exactly and the scaled down image returned using https://developer.apple.com/documentation/uikit/uiimage/3750845-preparethumbnailofsize is too blurry.

Heres a sample request and response to show the issue

▿ ImageRequest(resource: "some url (hidden for privacy)", priority: normal, processors: [], options: Options(rawValue: 0), userInfo: [Nuke.ImageRequest.UserInfoKey(rawValue: "github.com/kean/nuke/thumbnail"): Nuke.ImageRequest.ThumbnailOptions(targetSize: Nuke.ImageRequest.ThumbnailOptions.TargetSize.fixed(474.0), createThumbnailFromImageIfAbsent: true, createThumbnailFromImageAlways: true, createThumbnailWithTransform: true, shouldCacheImmediately: true), Nuke.ImageRequest.UserInfoKey(rawValue: "imageSize"): (110.0, 158.0)]) ▿ ref : <Container: 0x600001819c40>

(lldb) po response.image <UIImage:0x6000030307e0 anonymous {338, 474} renderingMode=automatic(original)>

(lldb) po response.image.size ▿ (338.0, 474.0)

As you can see image size is reported in pixel size and not points. The fix might be that when we return PlatformImage in Nuke.makeThumbnail use scale initializer for UIImage. https://github.com/kean/Nuke/blob/084076db4f9b9ee4db51177ef5a4879689c1aa09/Sources/Nuke/Internal/Graphics.swift#L369

return PlatformImage(cgImage: image, scale: scale, orientaion: orientation) instead of return PlatformImage(cgImage: image) I had to do that in an app i was working on as well

prabhuamol commented 3 weeks ago

You can also verify this by just chaning any of the ImageProcessorsResizeTests to use points and all of them fail

Screenshot 2024-06-14 at 2 50 28 PM

I also ran the testThatImageIsResized and verified that image size and sizeInPixels is the same. Where one should be in points and smaller

Screenshot 2024-06-14 at 2 53 56 PM
prabhuamol commented 2 weeks ago

@kean any luck investigating this? Would be amazing to get this fixed

prabhuamol commented 2 weeks ago

Submitted a PR here https://github.com/kean/Nuke/pull/793