-
Notifications
You must be signed in to change notification settings - Fork 625
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
Check dimensions of dynamic image before resizing it #1817
Check dimensions of dynamic image before resizing it #1817
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple stylistic suggestions, but the code itself looks good
As a side note, trying to change the return type to |
Yeah, for the return by reference to work, I would have needed to change the common case when the dimensions do not match. I didn't want to go down that way. |
There's now just a small rustfmt issue to resolve: /home/runner/.cargo/bin/cargo fmt -- --check
Diff in /home/runner/work/image/image/src/dynimage.rs at line 668:
return self.clone();
}
let (width2, height2) =
- resize_dimensions(self.width(), self.height(), nwidth, nheight, false);
+ resize_dimensions(self.width(), self.height(), nwidth, nheight, false);
self.resize_exact(width2, height2, filter)
} |
Should be fixed now |
Thanks! |
Should fix the issue: #1738
Not super happy with it since I'm cloning self but I didn't want to change the return type to &DynamicImage e.g.
Let me know if you have a better idea on how to make this more efficient :)