[WIP] fixed point perspective for `imresize`, `zoom` and `imrotate` by johnnychen94 · Pull Request #142 · JuliaImages/ImageTransformations.jl

johnnychen94 · 2021-07-27T08:51:04Z

Let me recite our newly-added documentation for the symbols and terminology:

A (backward-mode) warp operation consists of two parts: the value estimator τ (which we always use Interpolations.jl in this package), and the backward coordinate map ϕ. For some special warping operations such as scale(imresize), zoom, and rotation: unless it's an identity map, there exists one and only one point that satisfies ϕ(p) == p, and this p is defined as the fixed point.

This PR aims to provide consistent reasoning on this "fixed point" concept for these three operations.

`imresize`

Because imresize always spans the entire source domain, i.e., CartesianIndices(src_img), exposing a control keyword for fixed point only changes the offset of the output. For example: imresize(rand(10, 10), (5, 5); fixed_point = (5, 5)) requires the output array (an offset array) axes be (3:7, 3:7) so that ϕ((5, 5)) == (5, 5).

Also note that imresize(rand(10, 10), (5, 5)) returns an Array in previous ImageTransformations versions, and I don't think we should change this because 1) it exists for so long and Array as the base julia type has the best support among the entire ecosystem, and 2) OffsetArray introduces an extra step when computing the indices, so it might hurt the performance (although in many cases that I've observed, Julia compiler just optimizes this overhead away).

Thus for type stability, we can't make fixed_point a keyword, and this is why we did CenterPoint thing in #129 and it works pretty well.

However, one key question that #129 doesn't answer is: what is the default fixed point? And trying to answer this leads me to open a new PR here.

The current master version of ImageTransformation doesn't have it well-defined: for Array it is (1, 1). But for OffsetArray it's quite inconsistant because it maps p0 = map(first, axes(offset_img)) to q0 = (1, 1), which says the fixed point is neither (1, 1) or p0.

The first commit of this PR 605be80 does two things:

it fixes the type instability by unconditionally make the imresize(img, inds) method output OffsetArray.
(breaking change) the fixed point defaults to map(first, axes(img)): imresize(offset_img, ...) now outputs an OffsetArray instead of an Array. This is no doubt a breaking change but it's for a more consistent result when speaking of the fixed point things.

To support #129, we then just need some manual axes shifts on the output:

img = testimage("cameraman")
fixed_point = OffsetArrays.center(img)

o = @. fixed_point ÷ 2 + 1
out = imresize(img; ratio=0.5, method=Constant()) # we use nearest interpolation to avoid interpolation on grid point
outo = OffsetArray(out, OffsetArrays.Origin(o))
outo[fixed_point...] == img[fixed_point...] # true

I'm now unsure if we should support fixed point concept for imresize because it is really just a matter of offset shift.

[WIP] `zoom`

zoom works quite similar to imresize except that the canvas size isn't changed, and the fixed point concept plays a key role in zoom operation.

The second commit 33b5559 is a WIP for this new high-level API.

The currently implemented API is zoom(img; ratio, [fixed_point], [method], [fillvalue]), I might also want to introduce zoom(img, size_or_indices; [fixed_point], kwargs...) but I still need to think about if it's possible to interpret the indices input.

Top-left point as fixed point : map(first, axes(img))

img = imresize(testimage("camera"), 128, 128)
ImageShow.gif([zoom(img; ratio=r, fixed_point=(1, 1)) for r in 0.5:0.1:2])

Center point as fixed point: OffsetArrays.center(img)

img = imresize(testimage("camera"), 128, 128)
ImageShow.gif([zoom(img; ratio=r, fixed_point=(64, 64)) for r in 0.5:0.1:2])

[TODO] `imrotate`

The proposed API is to add fixed_point keyword.

Todo:

fix method ambiguities for imresize
add zoom
add keyword fixed_point for imrotate

cc: @ginkulv

- (Breaking change) `imresize(::OffsetArray, ...)` now returns also an `OffsetArray` and uses `map(first, axes(img))` as the fixed point. - (Enhancement) `imresize(img, ::Indices)` now unconditionally returns an `OffsetArray`; this fixes the legacy type instability.

codecov · 2021-07-27T08:56:03Z

Codecov Report

Merging #142 (33b5559) into master (226d504) will decrease coverage by 6.08%.
The diff coverage is 48.38%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   92.09%   86.00%   -6.09%     
==========================================
  Files           8        9       +1     
  Lines         215      243      +28     
==========================================
+ Hits          198      209      +11     
- Misses         17       34      +17

Impacted Files	Coverage Δ
src/ImageTransformations.jl	`83.33% <ø> (ø)`
src/zoom.jl	`0.00% <0.00%> (ø)`
src/resizing.jl	`98.55% <100.00%> (-1.45%)`	⬇️

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 226d504...33b5559. Read the comment docs.

timholy

LGTM. I didn't check the zoom calculations, but the idea seems nice.

timholy · 2021-07-27T11:52:33Z

src/resizing.jl

    end
 end

+function imresize(original::OffsetArray{T,N}, new_size::Dims{N}; kwargs...) where {T,N}


OffsetArray is not necessarily the only array type that doesn't have indexing starting with 1; there are several other examples. What's the reason for the special method? It looks almost identical to the above.

The only difference here is that the above method returns an Array while this new method for OffsetArray unconditionally returns an OffsetArray.

I also have the same concerns and I'm not very satisfied with this version very much, but it seems there's no other way to work around the type instability if we put them into one method that returns either OffsetArray or Array.

timholy · 2021-07-27T11:54:52Z

src/resizing.jl

+    origin = OffsetArrays.Origin(map(first, new_inds))
    if axes(original) == new_inds
-        copyto!(similar(original, Tnew), original)
+        OffsetArray(copyto!(similar(original, Tnew), original), origin)


Rather than manually create an OffsetArray, how about use similar(original, Tnew, new_inds)? That will create an OffsetArray if new_inds[i] isa UnitRange rather than a Base.OneTo.

I believe this won't be type stable due to the if axes(original) == new_inds branch.

With this change:

- OffsetArray(copyto!(similar(original, Tnew), original), origin) + copyto!(similar(original, Tnew, new_inds), original)

julia> img = testimage("cameraman"); julia> @inferred imresize(img, axes(img)) ERROR: return type Matrix{Gray{N0f8}} does not match inferred return type Union{Matrix{Gray{N0f8}}, OffsetMatrix{Gray{N0f8}, Matrix{Gray{N0f8}}}}

timholy · 2021-07-27T11:59:12Z

test/resizing.jl

+            @test test_imresize_interface(img, (20,10), (1:20, 1:10), ratio = (2, 1)) isa Array
+
+            # indices method always return OffsetArray
+            @test test_imresize_interface(img, (5,5), (1:5, 1:5), (1:5,1:5)) isa OffsetArray


Also add

@test test_imresize_interface(img, (5,5), (1:5, 1:5), (Base.OneTo(5),Base.OneTo(5))) isa Array

timholy · 2021-07-28T08:25:03Z

How about this as a proposal:

imresize(img, sz::Dims) is for the "I live in a 1s-based world, don't worry about the indices". This always returns an Array and throws an error if img has axes that don't start at 1. (Throwing an error is a good way to fix the type instability.)
imresize(img, sz::Dims, fp::Dims) is for a fixed point and always returns an array whose type is dictated by appropriate computations on the axes of img. If they are Base.OneTo then we switch to UnitRange and get an OffsetArray, but if it's, e.g., a CatIndices.URange then it creates that output type.

johnnychen94 · 2021-08-19T09:18:50Z

This PR becomes more delicate than I expected, and it's quite hard to find a way out for imresize. Thus I've converted this into a draft, and plan to split this PR into smaller parts so that it's easier to review and discuss. For instance, 1) adding zoom and 2) add fixed_point keyword for imrotate should be non-controversial.

johnnychen94 added 2 commits July 27, 2021 01:20

WIP: zoom

33b5559

timholy reviewed Jul 27, 2021

View reviewed changes

johnnychen94 mentioned this pull request Jul 28, 2021

ImageTransformations v0.9.0 #122

Merged

13 tasks

johnnychen94 marked this pull request as draft August 19, 2021 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] fixed point perspective for `imresize`, `zoom` and `imrotate`#142

[WIP] fixed point perspective for `imresize`, `zoom` and `imrotate`#142
johnnychen94 wants to merge 2 commits intomasterfrom
jc/fixedpoint

johnnychen94 commented Jul 27, 2021 •

edited

Loading

Uh oh!

codecov bot commented Jul 27, 2021 •

edited

Loading

Uh oh!

timholy left a comment

Uh oh!

timholy Jul 27, 2021

Uh oh!

johnnychen94 Jul 27, 2021

Uh oh!

timholy Jul 27, 2021

Uh oh!

johnnychen94 Jul 27, 2021

Uh oh!

timholy Jul 27, 2021

Uh oh!

timholy commented Jul 28, 2021 •

edited

Loading

Uh oh!

johnnychen94 commented Aug 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

johnnychen94 commented Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

imresize

[WIP] zoom

[TODO] imrotate

Uh oh!

codecov bot commented Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

timholy left a comment

Choose a reason for hiding this comment

Uh oh!

timholy Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

johnnychen94 Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

timholy Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

johnnychen94 Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

timholy Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

timholy commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnnychen94 commented Aug 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

johnnychen94 commented Jul 27, 2021 •

edited

Loading

`imresize`

[WIP] `zoom`

[TODO] `imrotate`

codecov bot commented Jul 27, 2021 •

edited

Loading

timholy commented Jul 28, 2021 •

edited

Loading