specialize imrotate for 0, 90, 180, 270 degrees by johnnychen94 · Pull Request #79 · JuliaImages/ImageTransformations.jl

johnnychen94 · 2019-12-14T12:41:12Z

img = testimage("cameraman") # (512, 512)

# Before: (514, 514)
# After: (512, 512)
size(imrotate(img, pi/2))

# Before: sizes are not equal
# After: true
imrotate(imrotate(imrotate(imrotate(img, pi/2), pi/2), pi/2), pi/2) == img

# Before: sizes are not equal
# After: true
imrotate(imrotate(img, pi), pi) == img

codecov · 2019-12-14T13:59:57Z

Codecov Report

Merging #79 into master will increase coverage by 0.56%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   88.56%   89.12%   +0.56%     
==========================================
  Files           7        7              
  Lines         271      285      +14     
==========================================
+ Hits          240      254      +14     
  Misses         31       31

Impacted Files	Coverage Δ
src/warp.jl	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c18de9...ca777ff. Read the comment docs.

johnnychen94 · 2019-12-16T10:02:50Z

I believe this PR is ready for review and merge

Evizero · 2019-12-16T10:41:24Z

is it type stable? was it before this PR?

timholy

This looks good except for concerns about non-inferrability. I suppose an alternative would be to write specialized functions (rotate90, rotate180, rotate270) for specific rotations.

timholy · 2019-12-18T16:04:43Z

                @test_nowarn imrotate(img, π/4, axes(img))
                @test_nowarn imrotate(img, π/4, axes(img), Constant())
-                @test isequal(channelview(imrotate(img,π/4)), channelview(imrotate(img, π/4, Linear()))) # TODO: if we remove channelview the test will break for Float
+                @test isequal(channelview(imrotate(img, π/4)), channelview(imrotate(img, π/4, Linear()))) # NaN != NaN


The meaning of this comment is unclear. Do you mean that you wanted to write == but instead have to use isequal?

johnnychen94 · 2019-12-18T21:35:28Z

Thanks for the comment, I'm already aware of the type stability issue, I'll come back and try to fix that later when I finished the upgrading of Augmentor.jl made by @Evizero (for my own research interest)

kimikage · 2019-12-20T22:44:45Z

BTW, 360 ° seems to be missing.

kimikage · 2019-12-20T23:09:31Z

+    # 2. typemax(Int16) is 32767, we choose 32760 to make sure the
+    # discretization result of pi/2 is exactly pi/2 (or 90°)
+    max_num_angles = 32760


Although the same goes for the current code, why don't you use 32768?

Well... It's like I randomly chose the first number I found that meets the requirement, and I feel this choice doesn't matter for image processing tasks.

kimikage · 2019-12-20T23:19:10Z

+    θ = round(Int, 180*floor(mod(θ, 2pi)/pi*max_num_angles)/max_num_angles)
+    tform = recenter(RotMatrix{2}(θ/180*pi), center(img))


Don't use the θ! 😱

mileslucas · 2020-03-14T05:48:34Z

+    elseif θ == 90
+        idx = StepRange.(axes(img))
+        perm_img = PermutedDimsArray(img, (2, 1))
+        return view(perm_img, idx[2], reverse(idx[1]))
+    elseif θ == 180
+        idx = map(i->1:1:length(i), axes(img))
+        return view(img, reverse(idx[1]), reverse(idx[2]))
+    elseif θ == 270
+        idx = StepRange.(axes(img))
+        perm_img = PermutedDimsArray(img, (2, 1))
+        return view(perm_img, reverse(idx[2]), idx[1])


any reason to not use some of the tools in base?

https://docs.julialang.org/en/v1/base/arrays/#Base.rot180
https://docs.julialang.org/en/v1/base/arrays/#Base.rotl90
https://docs.julialang.org/en/v1/base/arrays/#Base.rotr90

I don't think @johnnychen94 want to reallocate the memory and relocate the elements.:smile:

True, and there won't be type stability issues if we collect them and return an Array; but that would be slower than views.

I don't have a good fix in mind for this at the moment.

kimikage · 2020-03-14T06:31:50Z

BTW, I think it is better to round the rotation matrix instead of quantizing the angle.

johnnychen94 · 2020-03-14T06:33:30Z

That sounds promising to me!

mkitti · 2020-12-21T11:12:47Z

It seems a bit dangerous to specialize on an approximation for an irrational number. Maybe it would be better to have a degree based version of imrotate and specialize on multiples of 90 like the title suggests? The discretized imrotate could then map to that.

Otherwise, I would really appreciate something like this. It's absence was duly noted when attempting something AoC2020 Day 20: https://adventofcode.com/2020/day/20

mkitti · 2020-12-25T17:14:30Z

sinpi and cospi could also have some relevance here.
https://docs.julialang.org/en/v1/base/math/#Base.Math.sinpi

timholy · 2021-10-10T19:08:09Z

I'll let @johnnychen94 decide what to do here, but I put all the tests from this PR into #149 and they pass.

crop images for special rotation degrees

7badcac

do not use warp for these special degress

f208d6b

johnnychen94 changed the title ~~crop images for special rotation degrees~~ specialize imrotate for 0, 90, 180, 270 degrees Dec 16, 2019

johnnychen94 added 2 commits December 16, 2019 17:59

add one more test case

dd3b7e2

ImageTransformations v0.8.1

ca777ff

timholy reviewed Dec 18, 2019

View reviewed changes

kimikage reviewed Dec 20, 2019

View reviewed changes

mileslucas reviewed Mar 14, 2020

View reviewed changes

johnnychen94 mentioned this pull request Sep 8, 2021

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

Closed

timholy mentioned this pull request Oct 10, 2021

Fix imrotate for Irrational θ #149

Merged

timholy added a commit that referenced this pull request Oct 10, 2021

add more tests from #79

4c7b7c2

		θ = round(Int, 180floor(mod(θ, 2pi)/pimax_num_angles)/max_num_angles)
		tform = recenter(RotMatrix{2}(θ/180*pi), center(img))

Conversation

johnnychen94 commented Dec 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Dec 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

johnnychen94 commented Dec 16, 2019

Uh oh!

Evizero commented Dec 16, 2019

Uh oh!

timholy left a comment

Choose a reason for hiding this comment

Uh oh!

timholy Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

johnnychen94 commented Dec 18, 2019

Uh oh!

kimikage commented Dec 20, 2019

Uh oh!

kimikage Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

johnnychen94 Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimikage Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

mileslucas Mar 14, 2020

Choose a reason for hiding this comment

Uh oh!

kimikage Mar 14, 2020

Choose a reason for hiding this comment

Uh oh!

johnnychen94 Mar 14, 2020

Choose a reason for hiding this comment

Uh oh!

kimikage commented Mar 14, 2020

Uh oh!

johnnychen94 commented Mar 14, 2020

Uh oh!

mkitti commented Dec 21, 2020

Uh oh!

mkitti commented Dec 25, 2020

Uh oh!

timholy commented Oct 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

johnnychen94 commented Dec 14, 2019 •

edited

Loading

codecov Bot commented Dec 14, 2019 •

edited

Loading

johnnychen94 Dec 20, 2019 •

edited

Loading