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

photon::transform additions #179

Merged
merged 17 commits into from
Jun 11, 2024
Merged

Conversation

volbot
Copy link
Contributor

@volbot volbot commented May 24, 2024

Changes:

1. Added transform::{shearx, sheary}; added example (examples/shear.rs)
Note: The change to image size is a lot more drastic than before, and pretty much any usage/implementation will need some kind of cropping function.

2. Rewrote transform::rotate to use Paeth rotation; added interpolation; added example (examples/rotate.rs)
Resolves: #170 #178
Note: Slightly less performant than previously. This could hypothetically be faster if all three shears were incorporated into a single loop, but the extra considerations needed in order to properly interpolate made the gains paltry in every case I tried.

3. Changed signature of transform::crop to not require a mutable reference
Resolves: #177

@volbot volbot marked this pull request as ready for review May 25, 2024 10:17
@volbot
Copy link
Contributor Author

volbot commented May 27, 2024

I just removed the content-aware crop, ultimately leaving the cropping of the resultant image to the user (in line with previous behavior). After some personal testing, I realized that it sometimes cuts too aggressively, especially on small images with concave sections of alpha. Other than that, everything appears to work correctly.

@volbot
Copy link
Contributor Author

volbot commented Jun 8, 2024

I just fixed the Rustfmt issues and synced the recent multiple.rs changes to the branch, so it should(?) be mergeable now.

@silvia-odwyer
Copy link
Owner

@volbot It appears the rustfmt test isn't passing, could you run cargo fmt on the codebase? It will do any other additional formatting required, and I'll then be able to merge the PR ✅

Copy link
Owner

@silvia-odwyer silvia-odwyer left a comment

Choose a reason for hiding this comment

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

@volbot That's great, thanks very much again for your excellent contribution! It's greatly appreciated. I'm delighted that the library will have the new rotation implementation, as well as the added shear functions. 🎉 Everything looks good to me, so I'm going to merge this ✅

@silvia-odwyer silvia-odwyer merged commit 56b7d38 into silvia-odwyer:master Jun 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants