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

Optimize multiple crops #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paulnovo
Copy link
Contributor

Update CroppedProjectiveTranform to optimize projective transforms followed by multiple crops, not just one crop. For instance:

Rotate(10) |> CenterCrop((100, 100)) |> RandomCrop((50, 50))

Is now optimized to warp only into the 50x50 region, instead of to the 100x100 region.

One caveat of this implementation is that crops that enlarge the region after a smaller crop will be different due to extrapolation of the smaller crop being skipped. ie this will give different results now:

Rotate(10) |> CenterCrop((50, 50)) |> RandomCrop((100, 100))

I wonder if enlarging crops should be disallowed anyways, since offsetcropbounds doesn't appear to be written with it in mind (ie sz>bounds), unless I am missing something. Thoughts?

PR Checklist

  • Tests are added
  • Documentation, if applicable

Update CroppedProjectiveTranform to optimize projective transforms
followed by multiple crops, not just one crop. For instance:

    Rotate(10) |> CenterCrop((100, 100)) |> RandomCrop((50, 50))

Is now optimized to warp only into the 50x50 region, instead of to the
100x100 region.
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.

1 participant