ImageTransformations.jl icon indicating copy to clipboard operation
ImageTransformations.jl copied to clipboard

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

Open johnnychen94 opened this issue 4 years ago • 3 comments

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])

topleft

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])

center

[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

johnnychen94 avatar Jul 27 '21 08:07 johnnychen94

Codecov Report

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

Impacted file tree graph

@@            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%) :arrow_down:

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.

codecov[bot] avatar Jul 27 '21 08:07 codecov[bot]

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.

timholy avatar Jul 28 '21 08:07 timholy

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 avatar Aug 19 '21 09:08 johnnychen94