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

Add scale parameter to scale methods to allow setting the output image's scale. #360

Closed
wants to merge 1 commit into from

Conversation

felipeferri
Copy link

Goals ⚽

At some point I needed a way of cropping an image and setting its size in pixels. I couldn't use af_imageAspectScaled for that because internally it would use the device's main screen scale factor. So, for example, if i scaled an image to the size CGSize(width: 320, height: 320), on an iPhone 6 the resulting image would have the dimensions 640x640 (in pixels) and in an iPhone 6+ the dimenstions would be 960x960.

My goal on this pull request is to allow explicitly setting the scale when scaling images.

Implementation Details 🚧

Modified the methods af_imageScaled(to size:), af_imageAspectScaled(toFill size:) and af_imageAspectScaled(toFit size:) to receive an additional parameter scale: CGFloat, which defaults to 0.0 (which is the defaul value set on UIGraphicsBeginImageContextWithOptions()).

Testing Details 🔍

I've added the test UIImageTestCase.testThatImageIsScaledWithProperScale.

af_imageAspectScaled(toFit size:) and
af_imageAspectScaled(toFill size:) in order to allow setting
the resulting image's scale.
* Add corresponding unit test.
@cnoon
Copy link
Member

cnoon commented Feb 22, 2020

Hi @felipeferri, thank you for taking the time to put this together...much appreciated! 🍻

Instead of taking these changes alone, I've decided to rework the approach. I've just pushed change 61a029d into master which will go out shortly in AFI 4.0.0. It changes all the UIImage extensions to expose a scale parameter which defaults to nil allowing you to override the scale value used in UIGraphicsBeginImageContextWithOptions API. I've also changed the default behavior to no longer use a value of 0.0, but instead the scale of the image. This should result in much more consistent and expected behavior across the board. It was never really correct to have these functions setting the scale value to UIScreen.main.scale.

As a secondary addition, I've added the ability to customize the ImageResponseSerializer per download through the ImageDownloader and extension APIs on UIImageView and UIButton. This allows you to change the scale factor of the image download through both APIs which could only be done previously if you used the Alamofire.Session directly which no one wants to actually do. This should make the APIs much more flexible to do what you want in all situations regardless of whether you're using an Alamofire.Session, AlamofireImage.ImageDownloader, or UIImageView or UIButton extension.

Cheers. 🍻

@cnoon cnoon closed this Feb 22, 2020
@cnoon cnoon self-assigned this Feb 22, 2020
@cnoon cnoon added this to the 4.0.0 milestone Feb 22, 2020
@felipeferri
Copy link
Author

Thanks for the feedback. Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants