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

imrotate(img, pi/2) changes size of image #145

Closed
anandijain opened this issue Sep 7, 2021 · 3 comments · Fixed by #148
Closed

imrotate(img, pi/2) changes size of image #145

anandijain opened this issue Sep 7, 2021 · 3 comments · Fixed by #148

Comments

@anandijain
Copy link

img = rand(RGB{N0f8}, 1000, 1000)
img2 = imrotate(img, π/2)
size(img) # (1000, 1000)
size(img2) # (1002, 1002)

I realize it's pretty naive to just ask "why doesn't this work as expected?". I get the sense that "simply" rotating an image is not so simple. (I know very little about image processing)

I'm wondering how I should get the desired result for rotations where the degree is multiples of pi/2. This might be related to #119 although I'm not sure.

Would it make sense to just define a new imrotate specifically for these factors of pi/2?

@johnnychen94
Copy link
Member

johnnychen94 commented Sep 8, 2021

The expectation that "imrotate(img, pi/2) should have the same size as img" can be somehow illusive due to the numerical error. A trick we use in this package is to discretization on ratio so that we consistently have imrotate(img, theta) == imrotate(img, theta+2pi) but that doesn't handle special pi/2, pi and 3pi/2 case.

θ = floor(mod(θ,2pi)*typemax(Int16))/typemax(Int16) # periodic discretezation

To get the same size you can use imrorate(img, π/2, axes(img)).


Would it make sense to just define a new imrotate specifically for these factors of pi/2?

Yes it totally makes sense, I did some experiment a long time ago in #79 but there are performance concerns.

@johnnychen94
Copy link
Member

Would it make sense to just define a new imrotate specifically for these factors of pi/2?

There're Base functions rotl90, rotr90 and rot180 for this very purpose. In the meantime, I still think #79 is needed.

I believe that we also need more documentation on this behavior because other image processing frameworks don't have this axes stuff.

@timholy
Copy link
Member

timholy commented Oct 10, 2021

This is also fixed by #148

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

Successfully merging a pull request may close this issue.

3 participants