-
-
Notifications
You must be signed in to change notification settings - Fork 532
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
Image scaling bug #791
Comments
@kean any luck investigating this? Would be amazing to get this fixed |
Submitted a PR here #793 |
Hey, @prabhuamol! I've looked into it, and I might need some clarification as I'm not sure I follow this entirely. By default, all images fetched by Nuke are returned with a scale of For example, let's say you download an icon for a button from a server. The server returns an image with a resolution of 88×88 pixels but a logical size of 44×44 pixels. In the iOS terminology, it will be an image with a scale of If you need to change the scale of the feched image, you can use ImageRequest.UserInfoKey.scaleKey. But the way I typically approach it is by never relying on the logical scale of the images downloaded from the internet and instead restricting the point size of the views in which they are displayed.
It works as expected. When you change the I think there may be some confusion in terms of what to expect from this API. I've been actually already considering removing the |
Hey @kean thanks for the response. So the issue is that the returned UIImage from resize process does not take device scale into account. So lets say i wanted to resize an image to CGSize of 44 on a device with scale of 2. The returned image has pixel size of 8888 which makes sense but associated UIImage.size is also 8888 (which is not correct) The size of the UIImage should take the device scale into account and should not be a 1:1 mapping to the CGImage pixel size. The PR i submitted here fixes that #793 and also adds related tests. Also the scaleKey doc says that by default it matches the scale of the display which seems to be not true based on your comment above .
Currently you can also see the tests you have in the app will fail as i have shown in the screenshot above. Hope this helps clear things up |
Good catch. I updated it to the following:
|
I'd also suggest checking the Apple documentation about |
I read the doc but I think that is very specific to images inside a bundle. So can you point us to how do we make sure that the resized image we get back is returned with the right point size information? i.e the image is getting resized correctly but its just not returning the point size information accurately (i feel like this is a bug). It should follow the following logic as far as i can tell UIImage.size (points) = CGImage.size (pixels) * UIScreen.main.scale. I feel like the scale information is just lost when we resize images. So from consumer side the image returned is "larger" than it should be because the size info is not correct and is always matching 1:1 to pixel size irrespective of the device. On a related note i dont see the scale key being used anywhere in the thumbnail creation |
This is a bug, and PRs with a fix would be more than welcome. In the future major version, I have it on my list to see if the scale support could be moved to the later stages of the pipeline. Applying it early has implications for caching and request coalescing. For example , |
PR is here #793 |
Fixed in v12.8.0. Thanks @prabhuamol for sticking with in and getting a PR out. |
We are sometimes noticing when images are scaled using .thumbnailKey they appear blurry. I wonder if its cause in
func 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.
Nuke/Sources/Nuke/Internal/Graphics.swift
Line 369 in 084076d
return PlatformImage(cgImage: image, scale: scale, orientaion: orientation)
instead of returnPlatformImage(cgImage: image)
I had to do that in an app i was working on as wellThe text was updated successfully, but these errors were encountered: