Skip to content
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

Feature request: more flexibility in specifying thumbnail sizes #3622

Open
jaltreuter opened this issue Oct 12, 2023 · 12 comments
Open

Feature request: more flexibility in specifying thumbnail sizes #3622

jaltreuter opened this issue Oct 12, 2023 · 12 comments

Comments

@jaltreuter
Copy link

Issue Info

Info Value
Platform Name ios
Platform Version 17.0
SDWebImage Version 5.18.2
Integration Method cocoapods
Xcode Version Xcode 15
Repro rate N/A
Repro with our demo prj N/A
Demo project link N/A

Feature request

Right now if I specify a context like [.imageThumbnailPixelSize: CGSize(200, 200)] and the downloaded image has a 1:2 aspect ratio, the resulting image will have a size of 100 x 200. This is convenient when using the .scaleAspectFit content mode of UIImageView, because the pixel size of the image will exactly match the pixel size in which my image is rendered.

However if I'm using the .scaleAspectFill content mode, it doesn't work as well. If I'm using the same context and downloaded image as described above, the resulting 100 x 200 image will be scaled up to fill the frame. Effectively I end up with a 100 x 100 image in a 200 x 200 frame, and the image appears grainy as a result.

This could be addressed by a new boolean in the context dictionary, which would specify whether - if SDWebImageContextImagePreserveAspectRatio is true - a downloaded image should be adjusted to fit or to fill the SDWebImageContextImageThumbnailPixelSize.

@dreampiggy
Copy link
Contributor

dreampiggy commented Oct 13, 2023

Your feature request means, provide a enum like .scaleMode, not the bool .preserveAspectRatio

See SDImageScaleMode

@dreampiggy
Copy link
Contributor

It's already talked in the original PR 2 years ago. This changes is OK but need breaks current API (because, each coder need to implements a scale size calculation, which rely on SDImageCoder scaledSizeWithImageSize:)

Maybe it's better to kept for 6.0.0 milestone.

@dreampiggy
Copy link
Contributor

Same as #3089

@dreampiggy dreampiggy added this to the 6.0.0 milestone Oct 13, 2023
@dreampiggy
Copy link
Contributor

dreampiggy commented Oct 13, 2023

Actually it's can be done in 5.x version. The old PreserveAspectRatio will be deprecated and translated into SDImageScaleModeAspectFit (for true), and SDImageScaleModeFill (for false).

You can provide SDImageScaleModeAspectFill for new option.

But this need upgrade at least 3 repos, I'm afraid I don't have enough time on open-source project for now.

Repos (each repo need upgrade and receive the new options to use that for calculate the scale size):

SDWebImageWebPCoder - coder for WebP format. iOS 8+/macOS 10.10+. Based on libwebp
SDWebImageHEIFCoder - coder for HEIF format, iOS 8+/macOS 10.10+ support. Based on libheif
SDWebImageAVIFCoder - coder for AVIF (AV1-based) format. Based on libavif

@dreampiggy
Copy link
Contributor

dreampiggy commented Oct 21, 2023

Decide to mark this feature request in 5.19.0.

Introduce the new SDWebImageContextImageScaleMode context option.

The exists SDWebImageContextImagePreserveAspectRatio will marked as deprecated.

@dreampiggy
Copy link
Contributor

dreampiggy commented Oct 27, 2023

@jaltreuter Please, if you have a time, please check if this PR match your feature request ? There are many issues I need to talk with you.

And I'll release v5.18.4 for another user, the 5.19.0 is kept for this PR's changes.

@dreampiggy
Copy link
Contributor

@jaltreuter There are a known issue. (edge case)

Using asepctFill, the thumbnail image will be larger than the full size image, which breaks our history assumptions

For example

  • image size : 200 x 40
  • thumbnail size: 200 x 200
  • mode: aspectFill

actually, we need to return you a 1000 x 200 thumbnail image

But ask yourself, is this 1000 x 200 actually a thumbnail ? Even bigger than the full image pixel count ?

@jaltreuter
Copy link
Author

Ultimately what I want is for the bitmap image to have the same pixel resolution as the physical pixels on the device that will be rendering my image, so it's as crisp as possible without incurring extra memory overhead.

In the case you described above it's not possible to have a crisp image regardless of the thumbnail size I specify, so my expectation would be that the display should have the image as crisp as possible, and the runtime memory usage should not be inflated by specifying a thumbnail image that's greater resolution than the source image.

If this is complicated for you to implement, maybe there's an alternate approach: allow the user to specify the thumbnail size using a block/closure instead of a pre-specified value. The block would execute after the image is downloaded and have the image size as an argument, so the developer may return a thumbnail size suitable for the layout and the specific image.

@dreampiggy
Copy link
Contributor

dreampiggy commented Oct 28, 2023

Intrpduce a closure during decoding is really ugly API design, and you can not easily put closure inside Objective-C NSDictionary. Because, only Objc block use _NSBlock and it's a OS_Object, the Swift closure is just function pointer in implementation.

This is why the API we use that response modifier, request modifier, data decrypter, using a strange protocol to put into SDWebImageContext. And a dummy object with closure initializer (Still many people don't even notice why this exists)

So, for your updated feature request, I decide to still mark this as 6.0 feature request, and blocked by the re-written of API using Swift. p

@dreampiggy
Copy link
Contributor

@jaltreuter Or for your feature request

Can you use Transformer instead ? Transformer can tweak to get a UIImage in, and output a UIImage out.

So you can grab image size, do calculation by what you want. The call the proper API like our UIImage.sd_scaledImageWithSize or third party API for output

Read more:
https://github.com/SDWebImage/SDWebImage/wiki/Advanced-Usage#image-transformer-50

@dreampiggy
Copy link
Contributor

You can create a closure based transformer who take a context. for example, your image view size. Like

let transfomer = MyClosureTransformer {
image in
    let thumbnailSize = imageView.size
    let imageSize = image.size
    let scaledSize = doMath(imageSize, thumbnailSize)
    let output = image.sd_scaledImage(size: scaledSize, scaledMode : .aspectFit)
    return output
}

@dreampiggy
Copy link
Contributor

The only difference between thumbail decoding and resize transfomer is whether it need the full size image. Transformer need additional full size memory just for temporary, but the final image is dependends on what your output is.

@dreampiggy dreampiggy removed this from the 5.19.0 milestone Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants