allow lines of offsetarrays

[yakir12]

Super simple tiny change that allows plotting lines of OffsetArrays, suggested by @SimonDanisch on slack. It solves #1259.

This needs tests (it passes locally), but I'm not sure exactly how to properly test it. My naive approach would be to:

1. add OffsetArrays dependency to the Project file in the test repo
2. add a new test file for OffsetArrays
3. include said file in runtests.jl

Would that be good enough...?

[yakir12]

Apart from the issue of the tests (let me know what you'd have me do here), plotting a warped image still fails here.

MWE:

using GLMakie, ImageTransformations, CoordinateTransformations, Rotations, StaticArrays

img = rand(GLMakie.RGB, 100, 100)
sar = LinearMap(SDiagonal(0.5, 1))
rot = recenter(RotMatrix(pi/2), center(img))
tfm = sar ∘ rot
wimg = warp(img, tfm) # isa OffsetMatrix{RGB{Float64}, Matrix{RGB{Float64}}}

image(wimg, axis = (aspect = DataAspect(), ))

errors with:

Error showing value of type Makie.FigureAxisPlot:
ERROR: MethodError: no method matching GLMakie.GLAbstraction.Texture(::OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}; minfilter=:linear)
Closest candidates are:
  (::Type{T})(::Observable; kw...) where T<:GLMakie.GLAbstraction.GPUArray at /home/yakir/.julia/dev/Makie/GLMakie/src/GLAbstraction/AbstractGPUArray.jl:209
  GLMakie.GLAbstraction.Texture(::Array{Matrix{T}, 1}; internalformat, texturetype, format, parameters...) where T<:Union{Real, ColorTypes.Colorant, StaticVector} at /home/yakir/.julia/dev/Makie/GLMakie/src/GLAbstraction/GLTexture.jl:131
  GLMakie.GLAbstraction.Texture(::ShaderAbstractions.Sampler{T, N}; kwargs...) where {T, N} at /home/yakir/.julia/dev/Makie/GLMakie/src/GLAbstraction/GLTexture.jl:117
  ...
Stacktrace:
  [1] GLMakie.GLAbstraction.Texture(x::Observable{OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}; kw::Base.Pairs{Symbol, Symbol, Tuple{Symbol}, NamedTuple{(:minfilter,), Tuple{Symbol}}})
    @ GLMakie.GLAbstraction ~/.julia/dev/Makie/GLMakie/src/GLAbstraction/AbstractGPUArray.jl:210
  [2] (::GLMakie.var"#142#143"{Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}}})(gl_attributes::Dict{Symbol, Any})
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/drawing_primitives.jl:424
  [3] (::GLMakie.var"#76#82"{GLMakie.var"#142#143"{Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}}}, GLMakie.Screen, Scene, Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}}})()
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/drawing_primitives.jl:75
  [4] get!(default::GLMakie.var"#76#82"{GLMakie.var"#142#143"{Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}}}, GLMakie.Screen, Scene, Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}}}, h::Dict{UInt64, GLMakie.GLAbstraction.RenderObject}, key::UInt64)
    @ Base ./dict.jl:464
  [5] cached_robj!(robj_func::GLMakie.var"#142#143"{Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}}}, screen::GLMakie.Screen, scene::Scene, x::Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}})
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/drawing_primitives.jl:51
  [6] draw_atomic(screen::GLMakie.Screen, scene::Scene, x::Image{Tuple{IntervalSets.ClosedInterval{Float32}, IntervalSets.ClosedInterval{Float32}, OffsetArrays.OffsetMatrix{ColorTypes.RGB{Float32}, Matrix{ColorTypes.RGB{Float32}}}}})
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/drawing_primitives.jl:419
  [7] insert!(screen::GLMakie.Screen, scene::Scene, x::Combined)
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/drawing_primitives.jl:176
  [8] insertplots!(screen::GLMakie.Screen, scene::Scene)
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/screen.jl:56
  [9] #34
    @ ~/.julia/dev/Makie/GLMakie/src/screen.jl:58 [inlined]
 [10] foreach(f::GLMakie.var"#34#36"{GLMakie.Screen}, itr::Vector{Scene})
    @ Base ./abstractarray.jl:2606
 [11] insertplots!(screen::GLMakie.Screen, scene::Scene)
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/screen.jl:58
 [12] backend_display(screen::GLMakie.Screen, scene::Scene)
    @ GLMakie ~/.julia/dev/Makie/GLMakie/src/display.jl:18
 [13] backend_display
    @ ~/.julia/dev/Makie/GLMakie/src/display.jl:4 [inlined]
 [14] display(scene::Scene; update::Bool)
    @ Makie ~/.julia/dev/Makie/src/display.jl:60
 [15] display
    @ ~/.julia/dev/Makie/src/display.jl:56 [inlined]
 [16] #display#902
    @ ~/.julia/dev/Makie/src/display.jl:52 [inlined]
 [17] display
    @ ~/.julia/dev/Makie/src/display.jl:52 [inlined]
 [18] #display#901
    @ ~/.julia/dev/Makie/src/display.jl:51 [inlined]
 [19] display(fap::Makie.FigureAxisPlot)
    @ Makie ~/.julia/dev/Makie/src/display.jl:51
 [20] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [21] invokelatest
    @ ./essentials.jl:714 [inlined]
 [22] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
    @ REPL ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:288
 [23] (::REPL.var"#45#46"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:272
 [24] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:505
 [25] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:270
 [26] (::REPL.var"#do_respond#66"{Bool, Bool, REPL.var"#77#87"{REPL.LineEditREPL, REPL.REPLHistoryProvider}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:841
 [27] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [28] invokelatest
    @ ./essentials.jl:714 [inlined]
 [29] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/LineEdit.jl:2493
 [30] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:1227
 [31] (::REPL.var"#49#54"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:411

And a heatmap of a warped matrix of numbers:

m = rand(100, 100)
wm = warp(m, tfm) # isa OffsetMatrix{Float64, Matrix{Float64}}

heatmap(wm, axis = (aspect = DataAspect(), ))

also fails:

ERROR: TypeError: in typeassert, expected Matrix{Float32}, got a value of type OffsetArrays.OffsetMatrix{Float32, Matrix{Float32}}
Stacktrace:
 [1] el32convert(x::OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}})
   @ Makie ~/.julia/dev/Makie/src/conversions.jl:591
 [2] convert_arguments(#unused#::DiscreteSurface, data::OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}})
   @ Makie ~/.julia/dev/Makie/src/conversions.jl:312
 [3] convert_arguments(T::Type{Heatmap{Tuple{OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}}}}, args::OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}; kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Makie ~/.julia/dev/Makie/src/conversions.jl:10
 [4] convert_arguments
   @ ~/.julia/dev/Makie/src/conversions.jl:8 [inlined]
 [5] plot!(scene::Scene, P::Type{Heatmap}, attributes::Attributes, args::OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}; kw_attributes::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:show_axis,), Tuple{Bool}}})
   @ Makie ~/.julia/dev/Makie/src/interfaces.jl:321
 [6] plot(P::Type{Heatmap}, args::OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}; axis::NamedTuple{(:aspect,), Tuple{DataAspect}}, figure::NamedTuple{(), Tuple{}}, kw_attributes::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Makie ~/.julia/dev/Makie/src/figureplotting.jl:28
 [7] #heatmap#19
   @ ~/.julia/packages/MakieCore/S8PkO/src/recipes.jl:31 [inlined]
 [8] top-level scope
   @ REPL[15]:1

This seems a lot harder to solve...?

[SimonDanisch]

This seems a lot harder to solve...?

Not sure, I don't really know how OffsetArrays work :D
What do you expect to happen? Can you create the plot you want manually, like you did with lines:

x = keys(xy)
y = values(xy)
lines(x, y)

[yakir12]

Yes. One way would be to:

x1, y1 = wm.offsets .+ 1
nx, ny = size(wm)
x = range(x1, length = nx)
y = range(y1, length = ny)
v = parent(wm)
heatmap(x, y, v, axis = (aspect = DataAspect(), ))

Offsetarrays is everywhere in the Images.jl ecosystem, it has ~900 packages depending on it. It's fairly useful.

[SimonDanisch]

how about adding this then:

function convert_arguments(sl::SurfaceLike, wm::OffsetArray) 
  x1, y1 = wm.offsets .+ 1
  nx, ny = size(wm)
  x = range(x1, length = nx)
  y = range(y1, length = ny)
  v = parent(wm)
  return convert_arguments(sl, x, y, v)
end

[yakir12]

Wow, that, just, worked...

But I had to add a dependency on OffsetArrays.

Also, while this works for image(some_offsetarray), it does not work for heatmap.

[yakir12]

Not sure what to do with this. My needs to plot offset arrays have changed, but I bet someone will need it at some point. So if there is anything else I can/should do to get this merged let me know.

[SimonDanisch]

I see... I think this is mergeable right?

[yakir12]

Technically it is mergeable, but while this works for image(some_offsetarray), it does not work for heatmap. So not sure it's in a fantastic state.

[SimonDanisch]

Lets merge this... We can polish it later - the offsetarray depedency seems fine, since several packages we depend on already depend on it