Change maxPixelSize default value. Please comment
dmytro-anokhin opened this issue · 5 comments
URLImage package uses maxPixelSize property to limit image resolution when decoding. Decoding full resolution images, especially when displaying lists with multiple items, can create unneccessary memory pressure.
maxPixelSize value is set by default to 300x300px
The number is taken from assumption, that commonly UIs with lists don't show images at full resolution, raster images usually scale pretty well for people not to see artifacts.
You can set this value to something more suitable for your situation via URLImageService.shared.defaultOptions property, or make package decode full resolution images by setting it to nil. This can be done once in your application(_:didFinishLaunchingWithOptions:) implementation.
I would like to know what are more common image size that I can use as the default value of this property. If there will be enough comments I will change it to the average number. Note, this must be a size of the image view you display.
Alternatively, ideas if you would prefer this number to be calculated based on a device type, or a screen resolution, etc.
Hey @dmytro-anokhin!
Thanks for spinning up this discussion. IMO I wouldn't assume people will always use this library to show images in a list. The use cases are way more. In my case for example, I'm showing a big image as a header and this size of 300x300 doesn't work at all.
My point here is that it's actually pretty difficult to come up with a magic number because of the number of factors in place: thumbnail in a list or large image, using CDN or not, iPhone SE or iPad Pro screen, etc.
Thus, my suggestion is to not try to optimise for something we don't know but instead let the developer tweak that number based on the use case. If I'm showing profile pictures in a list I will set a small size but if I only have one big image I want the original image size. I may not have memory pressure or if I have I can use that property to fix it but IMO the default should be nil.
When is this an issue? In what cases? I think it might be good to document a specific use case and a solution for when it happens.
When is this an issue? In what cases? I think it might be good to document a specific use case and a solution for when it happens.
In my case I'm showing an image as a header that takes half of the screen. The image has good quality and is big enough but by default the URLImage library decodes it at 300x300px. Then when I set the image using .resizable().aspectRatio(contentMode: .fit) it gets completely blurry because the container size is bigger than 300x300 and the system stretches it interpolating the content.
So this resolution should only be set as a security measure or if you don't have properly sized images?
For example, I use cloudinary so all my images are the size I want them to be. This to me makes it sound like an edge case and should instead default to no max and be up to the developer to increase when needed.
After a month I think it's time to make a decision. Unfortunately the issue didn't give me the data about image sizes I was looking for. There are only two votes in favour of removing the limit, and I find this not enough. I consider that everyone else is pretty happy with how it is now.
However, I decided to increase the limit by default to 1000px for all platforms, except Apple Watch where 300px limit stays. I may revisit this decision in future.
Nevertheless, thanks everyone for suggestions.