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

Make initializer failable in case the pixel format is unsupported #129

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

ntnmrndn
Copy link
Collaborator

@ntnmrndn ntnmrndn commented Jun 7, 2021

No description provided.

…s for image source creation

Fix crash on extreme aspect ratios images
Comment on lines 294 to 296
let targetSize: CGSize = {
imageSize.scaled(maxPixelSize: maxPixelSize)
}()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntnmrndn
any reason? but I don't care

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore. I edited it


/// Checks that the pixel format is supported by core graphics.
/// Throws in case an error is found.
public static func validatePixelFormat(_ image: UIImage) throws {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Comment on lines +269 to +282
public static func makeResizedCGImage(
from sourceImage: CGImage,
maxPixelSizeHint: CGFloat
) -> CGImage? {
let maxPixelSize: CGFloat = {
let largestSide = max(sourceImage.size.width, sourceImage.size.height)
let smallestSide = min(sourceImage.size.width, sourceImage.size.height)
guard smallestSide >= maxPixelSizeHint else {
return largestSide
}
return largestSide * maxPixelSizeHint / smallestSide
}()
return makeResizedCGImage(from: sourceImage, maxPixelSize: maxPixelSize)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for fixing it.
Should we make the original one internal method? is it okay to keep it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which means, integrating this operation and below primitive operation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it's ok to have both; since they do different things. If an user is confident that his image can use the maxPixelSize version it's ok to use it.

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