From 9a625480692c9f4887813cb0822ab3a86397d058 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sun, 12 Sep 2021 08:15:08 -0500 Subject: [PATCH] WIP: Towards a more "Julian" package This is a proposed redesign that may be a bit more natural to work with from the Julia perspective. The principal difference are: - Support eltypes of the form `SVector{T,N}`: this makes this wrapper behave more like OpenCV itself, which (unlike Python) does not use an array dimension to encode the number of color channels. - Support `N0f8` and `N0f16`, JuliaImages preferred interpretation of 8- and 16-bit unsigned intensity data. (See FixedPointNumbers and https://juliaimages.org/latest/tutorials/arrays_colors/#fixedpoint; interestingly, OpenCV itself suffers from the exact same problem, see https://stackoverflow.com/questions/14539498/change-type-of-mat-object-from-cv-32f-to-cv-8u for an example of a user who got bit by the "divide by 255" rule.) - Work towards support for multidimensional arrays (see https://github.com/archit120/OpenCV.jl/issues/4) - Validate arguments more carefully and improve correctness in a few key places --- src2/jl_cxx_files/Mat.jl | 48 ++++++++++++++------ src2/jl_cxx_files/Vec.jl | 33 +++++++------- src2/jl_cxx_files/cv_cxx.jl | 28 +++++------- src2/jl_cxx_files/mat_conversion.jl | 69 +++++++++++++---------------- 4 files changed, 94 insertions(+), 84 deletions(-) diff --git a/src2/jl_cxx_files/Mat.jl b/src2/jl_cxx_files/Mat.jl index 416f951..68083fe 100644 --- a/src2/jl_cxx_files/Mat.jl +++ b/src2/jl_cxx_files/Mat.jl @@ -1,22 +1,43 @@ -#Adapted from IndirectArray +struct Mat{T, N, A<:AbstractArray{T,N}} <: DenseArray{T, N} + mat::CxxMat + data::A -struct Mat{T <: dtypes} <: AbstractArray{T,3} - mat - data_raw - data + function Mat{T, N, A}(mat, data_raw::A) where {T, N, A} + ok, msg = validate(data_raw) + ok || error(msg) + new{T, N, A}(mat, data_raw) + end - @inline function Mat{T}(mat, data_raw::AbstractArray{T,3}) where {T <: dtypes} - data = reinterpret(T, data_raw) - new{T}(mat, data_raw, data) + function Mat{T, N, A}(data_raw::A) where {T, N, A} + ok, msg = validate(data_raw) + ok || error(msg) + new{T, N, A}(nothing, data_raw) end +end +Mat{T, N}(mat, data::StridedArray{T, N}) = Mat{T, N, typeof(data)}(mat, data) +Mat{T, N}(data::StridedArray{T, N}) = Mat{T, N, typeof(data)}(data) +Mat{T, N}(mat, data::AbstractArray{T, N}) = Mat{T, N}(mat, copy(data)::StridedArray) +Mat{T, N}(data::AbstractArray{T, N}) = Mat{T, N}(copy(data)::StridedArray) +Mat{T}(mat, data::AbstractArray{T}) = Mat{T, ndims(data)}(mat, data) +Mat{T}(data::AbstractArray{T}) = Mat{T, ndims(data)}(data) +Mat(data) = Mat{eltype(data)}(data) +Mat(data::AbstractArray{Color{T, N}}) where {T, N} = Mat(reinterpret(SVector{N, T}, data)) +Mat(data::AbstractArray{ColorAlpha{C, T, N}}) where {C, T, N} = Mat(reinterpret(SVector{N, T}, data)) - @inline function Mat(data_raw::AbstractArray{T, 3}) where {T <: dtypes} - data = reinterpret(T, data_raw) - mat = nothing - new{T}(mat, data_raw, data) +function validate(data) + s = strides(data) + s[1] == 1 || return false, "first stride must be 1" + sz = size(data) + for i = 2:length(s) + s[i] == s[i-1] * sz[i-1] || return false, "array is not dense" end + # eltype(eltype(data)) lets us support SVector{N, T<:dtype}, + # and eltype(eltype(UInt8)) === UInt8 so it's safe even for dtypes + eltype(eltype(data)) <: dtypes || return false, "unsupported element type $(eltype(eltype(data)))" + return true, "" end +# why is this needed? function Base.deepcopy_internal(x::Mat{T}, y::IdDict) where {T} if haskey(y, x) return y[x] @@ -28,7 +49,7 @@ end Base.size(A::Mat) = size(A.data) Base.axes(A::Mat) = axes(A.data) -Base.IndexStyle(::Type{Mat{T}}) where {T} = IndexCartesian() +Base.IndexStyle(::Type{Mat{T, N, A}}) where {T, N, A} = IndexStyle(A) Base.strides(A::Mat{T}) where {T} = strides(A.data) Base.copy(A::Mat{T}) where {T} = Mat(copy(A.data_raw)) @@ -45,5 +66,4 @@ end @inline function Base.setindex!(A::Mat, x, I::Vararg{Int,3}) @boundscheck checkbounds(A.data, I...) A.data[I...] = x - return A end diff --git a/src2/jl_cxx_files/Vec.jl b/src2/jl_cxx_files/Vec.jl index 05f0400..1c1ec69 100644 --- a/src2/jl_cxx_files/Vec.jl +++ b/src2/jl_cxx_files/Vec.jl @@ -1,22 +1,26 @@ -#Adapted from IndirectArray - -struct Vec{T, N} <: AbstractArray{T,1} +struct Vec{T, N, V<:AbstractVector{T}} <: AbstractVector{T} cpp_object - data::AbstractArray{T, 1} - cpp_allocated::Bool - @inline function Vec{T, N}(obj) where {T, N} + data::V + cpp_allocated::Bool # this seems unused - new{T, N}(obj, Base.unsafe_wrap(Array{T, 1}, Ptr{T}(obj.cpp_object), N), true) + function Vec{T, N, V}(obj) where {T, N, V} + # if obj is not cpp_allocated, is setting that to `true` a lie? + new{T, N, V}(obj.cpp_object, Base.unsafe_wrap(V, Ptr{T}(obj.cpp_object), N), true) end - @inline function Vec{T, N}(data_raw::AbstractArray{T, 1}) where {T, N} - if size(data_raw, 1) != N - throw("Array is improper Size for Vec declared") + function Vec{T, N, V}(data_raw::V) where {T, N, V} + if length(data_raw) != N + throw(DimensionMismatch("Array is improper Size for Vec declared")) end - new{T, N}(nothing, data_raw, false) + # Because strides is set at (1,) below, we have to check it here + strides(data_raw) == (1,) || error("stride must be 1 (copy input first if needed)") + new{T, N, V}(nothing, data_raw, false) end end +Vec{T, N}(data::StridedVector) = Vec{T, N, typeof(data)}(data) +Vec{T, N}(data::AbstractVector) = Vec{T, N}(copy(data)::StridedVector) +# why is this needed? function Base.deepcopy_internal(x::Vec{T,N}, y::IdDict) where {T, N} if haskey(y, x) return y[x] @@ -26,11 +30,11 @@ function Base.deepcopy_internal(x::Vec{T,N}, y::IdDict) where {T, N} return ret end -Base.size(A::Vec) = Base.size(A.data) +Base.size(A::Vec{T,N}) = (N,) Base.axes(A::Vec) = Base.axes(A.data) Base.IndexStyle(::Type{Vec{T,N}}) where {T, N} = IndexLinear() -Base.strides(A::Vec{T,N}) where {T, N} = (1) +Base.strides(A::Vec{T,N}) where {T, N} = (1,) function Base.copy(A::Vec{T,N}) where {T, N} return Vec{T, N}(copy(A.data)) end @@ -46,5 +50,4 @@ end @inline function Base.setindex!(A::Vec, x, I::Int) @boundscheck checkbounds(A.data, I) A.data[I] = x - return A -end \ No newline at end of file +end diff --git a/src2/jl_cxx_files/cv_cxx.jl b/src2/jl_cxx_files/cv_cxx.jl index 4eeb6eb..59b7795 100644 --- a/src2/jl_cxx_files/cv_cxx.jl +++ b/src2/jl_cxx_files/cv_cxx.jl @@ -1,8 +1,11 @@ -# using StaticArrays +using StaticArrays +using FixedPointNumbers +using ColorTypes include("typestructs.jl") include("Vec.jl") -const dtypes = Union{UInt8, Int8, UInt16, Int16, Int32, Float32, Float64} +const rawtypes = Union{UInt8, N0f8, Int8, UInt16, N0f16, Int16, Int32, Float32, Float64} +const dtypes = Union{dtypes, SVector{N, <:rawtypes} where N} size_t = UInt64 using CxxWrap @@ -18,26 +21,17 @@ const Scalar = Union{Tuple{}, Tuple{Number}, Tuple{Number, Number}, Tuple{Number include("Mat.jl") -const InputArray = Union{AbstractArray{T, 3} where {T <: dtypes}, CxxMat} +const InputArray = Union{AbstractArray{<:dtypes}, CxxMat} include("mat_conversion.jl") include("types_conversion.jl") -function cpp_to_julia(var) - return var -end -function julia_to_cpp(var) - return var -end +# Fallbacks +cpp_to_julia(var) = var +julia_to_cpp(var) = var -function cpp_to_julia(var::Tuple) - ret_arr = Array{Any, 1}() - for it in var - push!(ret_arr, cpp_to_julia(it)) - end - return tuple(ret_arr...) -end +cpp_to_julia(var::Tuple) = map(cpp_to_julia, var) include("cv_cxx_wrap.jl") -include("cv_manual_wrap.jl") \ No newline at end of file +include("cv_manual_wrap.jl") diff --git a/src2/jl_cxx_files/mat_conversion.jl b/src2/jl_cxx_files/mat_conversion.jl index 746e699..b2bf3fc 100644 --- a/src2/jl_cxx_files/mat_conversion.jl +++ b/src2/jl_cxx_files/mat_conversion.jl @@ -35,57 +35,50 @@ function cpp_to_julia(mat::CxxMat) else error("Bad type returned from OpenCV") end - steps = [rets[6]/sizeof(dtype), rets[7]/sizeof(dtype)] + if rets[3] != 1 + dtype = SVector{rets[3], dtype} + end + # steps = [rets[6]/sizeof(dtype), rets[7]/sizeof(dtype)] # println(steps[1]/rets[3], steps[2]/rets[3]/rets[4]) #TODO: Implement views when steps do not result in continous memory - arr = Base.unsafe_wrap(Array{dtype, 3}, Ptr{dtype}(rets[1].cpp_object), (rets[3], rets[4], rets[5])) + # Does this ever happen? At + # https://docs.opencv.org/4.5.3/d3/d63/classcv_1_1Mat.html#details + # Mat is described as "dense". However, the CxxMat constructor obviously takes + # strides, so...? + arr = Base.unsafe_wrap(Matrix{dtype}, Ptr{dtype}(rets[1].cpp_object), (rets[4], rets[5])) #Preserve Mat so that array allocated by C++ isn't deallocated return Mat{dtype}(mat, arr) end -function julia_to_cpp(img::InputArray) - if typeof(img) <: CxxMat - return img - end - steps = 0 - try - steps = strides(img) - catch - # Copy array since array is not strided +julia_to_cpp(img::CxxMat) = img +function julia_to_cpp(img::StridedArray) + ok, msg = validate(img) + if !ok img = img[:, :, :] - steps = strides(img) + ok, msg = validate(img) + ok || error(msg) end - if steps[1] <= steps[2] <= steps[3] && steps[1]==1 - steps_a = Array{size_t, 1}() - ndims_a = Array{Int32, 1}() - sz = sizeof(eltype(img)) - push!(steps_a, UInt64(steps[3]*sz)) - push!(steps_a, UInt64(steps[2]*sz)) - push!(steps_a, UInt64(steps[1]*sz)) + etype = eltype(eltype(img)) + sz = sizeof(etype) + steps_a = size_t[step*sz for step in reverse(strides(img))] + ndims_a = Cint[sz for sz in reverse(size(img))] - push!(ndims_a, Int32(size(img)[3])) - push!(ndims_a, Int32(size(img)[2])) - if eltype(img) == UInt8 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_8U, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - elseif eltype(img) == UInt16 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_16U, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - elseif eltype(img) == Int8 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_8S, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - elseif eltype(img) == Int16 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_16S, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - elseif eltype(img) == Int32 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_32S, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - elseif eltype(img) == Float32 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_32F, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - elseif eltype(img) == Float64 - return CxxMat(2, pointer(ndims_a), CV_MAKE_TYPE(CV_64F, size(img)[1]), Ptr{Nothing}(pointer(img)), pointer(steps_a)) - end + cvtype = etype === UInt8 ? CV_8U : + etype === UInt16 ? CV_16U : + etype === Int8 ? CV_8S : + etype === Int16 ? CV_16S : + etype === Int32 ? CV_32S : + etype === Float32 ? CV_32F : + etype === Float64 ? CV_64F : error("unexpected eltype $(eltype(img))") + if eltype(img) <: SVector + cvtype = CV_MAKE_TYPE(cvtype, length(eltype(img))) else - # Copy array, invalid config - return julia_to_cpp(img[:, :, :]) + cvtype = CV_MAKE_TYPE(cvtype, 1) end + + return CxxMat(2, pointer(ndims_a), cvtype, Ptr{Nothing}(pointer(img)), pointer(steps_a)) end function julia_to_cpp(var::Array{T, 1}) where {T <: InputArray}