JuliaImages / Images.jl

An image library for Julia
http://juliaimages.org/
Other
533 stars 141 forks source link

Move Percentile and HomogeneousPoint definitions to another file/package #733

Closed juliohm closed 5 years ago

juliohm commented 6 years ago

Dear all,

What is this code for?

"""
    Percentile(x)
Indicate that `x` should be interpreted as a [percentile](https://en.wikipedia.org/wiki/Percentile) rather than an absolute value. For example,
- `canny(img, 1.4, (80, 20))` uses absolute thresholds on the edge magnitude image
- `canny(img, 1.4, (Percentile(80), Percentile(20)))` uses percentiles of the edge magnitude image as threshold
"""
struct Percentile{T} <: Real p::T end

"""
HomogeneousPoint(x::NTuple{N, T})
In projective geometry [homogeneous coordinates](https://en.wikipedia.org/wiki/Homogeneous_coordinates) are the
natural coordinates for describing points and lines.
For instance, the homogeneous coordinates for a planar point are a triplet of real numbers ``(u, v ,w)``, with ``w \\neq 0``.
This triple can be associated with a point ``P = (x,y)`` in Cartesian coordinates, where ``x = \\frac{u}{w}`` and ``y = \\frac{v}{w}``
[(more details)](http://www.geom.uiuc.edu/docs/reference/CRC-formulas/node6.html#SECTION01140000000000000000).
In particular, the `HomogeneousPoint((10.0,5.0,1.0))` is the standardised projective representation of the Cartesian
point `(10.0,5.0)`.
"""
struct HomogeneousPoint{T <: AbstractFloat,N}
    coords::NTuple{N, T}
end

# By overwriting Base.to_indices we can define how to index into an N-dimensional array
# given an (N+1)-dimensional [`HomogeneousPoint`](@ref) type.
# We do this by converting the homogeneous coordinates to Cartesian coordinates
# and rounding to nearest integer.
#
# For homogeneous coordinates of a planar point we return
# a tuple of permuted Cartesian coordinates, (y,x), since matrices
# are indexed  according to row and then column.
# For homogeneous coordinates of other dimensions we do not permute
# the corresponding Cartesian coordinates.
Base.to_indices(A::AbstractArray, p::Tuple{<: HomogeneousPoint}) = homogeneous_point_to_indices(p[1])

function homogeneous_point_to_indices(p::HomogeneousPoint{T,3}) where T
    if  p.coords[end] == 1
        return round(Int,  p.coords[2]), round(Int, p.coords[1])
    else
        return round(Int,  p.coords[2] / p.coords[end]), round(Int, p.coords[1] / p.coords[end])
    end
end

function homogeneous_point_to_indices(p::HomogeneousPoint)
    if  p.coords[end] == 1
        return round.(Int, p.coords)
    else
        return round.(Int, p.coords ./ p.coords[end])
    end
end

and should it be moved somewhere else other than src/Images.jl?

timholy commented 6 years ago

I'm not quite sure how to answer that, to me the help text for Percentile explains its purpose pretty clearly. HomogenousPoint could just as well move to CoordinateTransformations or something.

juliohm commented 6 years ago

Thank you Tim, so these two could live in another source file or another package in the project? I find it strange that these two concepts currently live in the main project file alone.

timholy commented 6 years ago

The location is reasonably arbitrary, however we don't (yet) need them in any lower-down dependency. I'm fine with moving them if that makes more sense.

zygmuntszpak commented 6 years ago

I wrote the homogeneous_point_to_indices as part of the sub-pixel corner detection corner2subpixel code in corner.jl. Homogeneous coordinates are used throughout computer vision for various projective geometry operations so I thought that the sub-pixel corners may as well be already stored as homogeneous points. However, I have subsequently discovered the StaticArrays package and now I tend to store points as length-2 static arrays, and just append a 1 to convert to homogeneous coordinates by using the push function of SVector whenever I need to do a projective geometry operation.

Hence, I would actually like to move away from the current definition of HomogeneousPoint type and use an SVector instead. However, there was some issue with creating custom indexing behavior for an SVector

I reckon that we still need to overwrite Base.to_indices and add some kind of subpixel_to_indices function somewhere in the Image ecosystem. The reason for this is that we would like to be able to plot corner points that are known with subpixel precision, or we might want to extract a local neighbourhood of pixels around a corner point which is known to subpixel precision. CartesianIndex assumes integer values, but we actually need some way to index into an array with floating point values.

juliohm commented 6 years ago

Thank you @zygmuntszpak for clarifying :+1: I will not touch the code given that you have nicer plans for it.

Please feel free to close this issue when you have it solved. The src/Images.jl file is already a little cleaner in https://github.com/JuliaImages/Images.jl/pull/734 but that PR will only be merged after the issues with ColorTypes.jl and Colors.jl are fixed.

juliohm commented 6 years ago

@zygmuntszpak can I move the definitions to corner.jl meanwhile and close this issue?

zygmuntszpak commented 6 years ago

Yes, absolutely!