Repository: JuliaPlots/Makie.jl (julia)

Issue:
managing colorrange and colorbar in the recipe arrows() 
fig = Figure();
ax = Axis(fig[1,1], aspect = 1.0)
ar = arrows!(ax, x_range, y_range, u_matrix, v_matrix, 
    color = vec(color_matrix), 
    colorrange = (0.0, 1.0),
    colormap = :viridis, 
)
Colorbar(fig[1,2], ar)

Above is the ideal code of using arrows() recipe with colors, but it doesn't work.

First, the Colorbar() function returns an error saying: 
Multiple colormaps found for plot Arrows{Tuple{Vector{Point{2, Float32}}, Vector{Vec{2, Float32}}}}, please specify which one to use manually. Please overload Makie.extract_colormap(::Arrows{Tuple{Vector{Point{2, Float32}}, Vector{Vec{2, Float32}}}})` to allow for the automatical creation of a Colorbar.

I find that ar consists 2 plots. One is the plot of arrow lines and the other is of arrowheads. They may have different colormaps, although in most cases we set them the same.

julia> ar.plots
2-element Vector{AbstractPlot}:
 LineSegments{Tuple{Base.ReinterpretArray{Point{2, Float32}, 1, Pair{Point{2, Float32}, Point{2, Float32}}, Vector{Pair{Point{2, Float32}, Point{2, Float32}}}, false}}}
 Scatter{Tuple{Vector{Point{2, Float32}}}}

So the idea is to generate colorbar for just one of them. For example:
Colorbar(fig[1,2], ar.plots[1])

The second thing is that the colorrange keywords doesn't really make any difference, as it changes the attribute of object ar but doesn't affect the attribute of the two plots in ar. 

julia> ar.attributes[:colorrange]
Observable{Any}((0.0, 1.0))
julia> ar.plots[1].attributes[:colorrange]
Observable{Any}(MakieCore.Automatic())
julia> ar.plots[2].attributes[:colorrange]
Observable{Any}(MakieCore.Automatic())

So I need to manually modify them.
ar.plots[1].attributes[:colorrange] = (0.0, 1.0)
ar.plots[2].attributes[:colorrange] = (0.0, 1.0)

Although, I believe these functions should be implemented natively in the official methods arrow() and Colorbar().

Pull request:
Closes #3273, #3266, #3268.
Also fixes a regression for `broadcasted_foreach`, which stopped working for 0 dim arrays, which introduced a regression in ablines for AoG (AoG turned `mapping([0], [1])` into `fill(0), fill(1)`, which worked well with ablines before).

Patch:
diff --git a/WGLMakie/src/particles.jl b/WGLMakie/src/particles.jl
index 381d8076853..c62a3f3f68b 100644
--- a/WGLMakie/src/particles.jl
+++ b/WGLMakie/src/particles.jl
@@ -230,31 +230,13 @@ value_or_first(x) = x
 
 function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.GlyphCollection, <:AbstractVector{<:Makie.GlyphCollection}}}})
     glyphcollection = plot[1]
-    res = map(x->Vec2f(widths(x)), pixelarea(scene))
-    projview = scene.camera.projectionview
     transfunc = Makie.transform_func_obs(plot)
     pos = plot.position
     space = plot.space
-    markerspace = plot.markerspace
     offset = plot.offset
 
-    # TODO: This is a hack before we get better updating of plot objects and attributes going.
-    # Here we only update the glyphs when the glyphcollection changes, if it's a singular glyphcollection.
-    # The if statement will be compiled away depending on the parameter of Text.
-    # This means that updates of a text vector and a separate position vector will still not work if only the text
-    # vector is triggered, but basically all internal objects use the vector of tuples version, and that triggers
-    # both glyphcollection and position, so it still works
-    if glyphcollection[] isa Makie.GlyphCollection
-        # here we use the glyph collection observable directly
-        gcollection = glyphcollection
-    else
-        # and here we wrap it into another observable
-        # so it doesn't trigger dimension mismatches
-        # the actual, new value gets then taken in the below lift with to_value
-        gcollection = Observable(glyphcollection)
-    end
     atlas = wgl_texture_atlas()
-    glyph_data = map(pos, gcollection, offset, transfunc, space) do pos, gc, offset, transfunc, space
+    glyph_data = map(pos, glyphcollection, offset, transfunc, space; ignore_equal_values=true) do pos, gc, offset, transfunc, space
         Makie.text_quads(atlas, pos, to_value(gc), offset, transfunc, space)
     end
 
@@ -281,10 +263,8 @@ function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.Gl
         end
     end
 
-    cam = scene.camera
     plot_attributes = copy(plot.attributes)
     plot_attributes.attributes[:calculated_colors] = uniform_color
-
     uniforms = Dict(
         :model => plot.model,
         :shape_type => Observable(Cint(3)),
diff --git a/src/basic_recipes/arrows.jl b/src/basic_recipes/arrows.jl
index 27fa6bfd7c3..83a2a38f76c 100644
--- a/src/basic_recipes/arrows.jl
+++ b/src/basic_recipes/arrows.jl
@@ -19,7 +19,7 @@ grid.
 
 `arrows` can also work in three dimensions.
 
-If a `Function` is provided in place of `u, v, [w]`, then it must accept 
+If a `Function` is provided in place of `u, v, [w]`, then it must accept
 a `Point` as input, and return an appropriately dimensioned `Point`, `Vec`,
 or other array-like output.
 
@@ -165,7 +165,8 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
 
         linesegments!(
             arrowplot, headstart,
-            color = line_c, colormap = colormap, colorscale = colorscale, linestyle = linestyle,
+                      color=line_c, colormap=colormap, colorscale=colorscale, linestyle=linestyle,
+                      colorrange=arrowplot.colorrange,
             linewidth=lift(lw -> lw === automatic ? 1.0f0 : lw, arrowplot, linewidth),
             fxaa = fxaa_bool, inspectable = inspectable,
             transparency = transparency, visible = visible,
@@ -176,7 +177,7 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
             marker=marker_head,
             markersize = lift(as-> as === automatic ? theme(scene, :markersize)[] : as, arrowplot, arrowsize),
             color = arrow_c, rotations = rotations, strokewidth = 0.0,
-            colormap = colormap, markerspace = arrowplot.markerspace,
+                 colormap=colormap, markerspace=arrowplot.markerspace, colorrange=arrowplot.colorrange,
             fxaa = fxaa_bool, inspectable = inspectable,
             transparency = transparency, visible = visible
         )
@@ -212,7 +213,7 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
             start, rotations = directions,
             marker=marker_tail,
             markersize = msize,
-            color = line_c, colormap = colormap, colorscale = colorscale,
+                     color=line_c, colormap=colormap, colorscale=colorscale, colorrange=arrowplot.colorrange,
             fxaa = fxaa_bool, ssao = ssao,
             diffuse = diffuse,
             specular = specular, shininess = shininess, inspectable = inspectable,
@@ -223,7 +224,7 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
             start, rotations = directions,
             marker=marker_head,
             markersize = markersize,
-            color = arrow_c, colormap = colormap, colorscale = colorscale,
+                     color=arrow_c, colormap=colormap, colorscale=colorscale, colorrange=arrowplot.colorrange,
             fxaa = fxaa_bool, ssao = ssao,
             diffuse = diffuse,
             specular = specular, shininess = shininess, inspectable = inspectable,
diff --git a/src/figureplotting.jl b/src/figureplotting.jl
index c11b3502a37..968758dacfd 100644
--- a/src/figureplotting.jl
+++ b/src/figureplotting.jl
@@ -1,5 +1,5 @@
 struct AxisPlot
-    axis
+    axis::Any
     plot::AbstractPlot
 end
 
@@ -17,8 +17,7 @@ function _validate_nt_like_keyword(@nospecialize(kw), name)
             The $name keyword argument received an unexpected value $(repr(kw)).
             The $name keyword expects a collection of Symbol => value pairs, such as NamedTuple, Attributes, or AbstractDict{Symbol}.
             The most common cause of this error is trying to create a one-element NamedTuple like (key = value) which instead creates a variable `key` with value `value`.
-            Write (key = value,) or (; key = value) instead."""
-        ))
+            Write (key = value,) or (; key = value) instead."""))
     end
 end
 
@@ -28,35 +27,43 @@ function _disallow_keyword(kw, attributes)
     end
 end
 
-function plot(P::PlotFunc, args...; axis = NamedTuple(), figure = NamedTuple(), kw_attributes...)
-
-    _validate_nt_like_keyword(axis, "axis")
-    _validate_nt_like_keyword(figure, "figure")
-
-    fig = Figure(; figure...)
-
-    axis = Dict(pairs(axis))
-    if haskey(axis, :type)
-        axtype = axis[:type]
-        pop!(axis, :type)
-        ax = axtype(fig; axis...)
+@nospecialize
+function get_axis(fig, P, axis_kw::Dict, plot_attr, plot_args)
+    if haskey(axis_kw, :type)
+        axtype = axis_kw[:type]
+        pop!(axis_kw, :type)
+        ax = axtype(fig; axis_kw...)
     else
         proxyscene = Scene()
-        attrs = Attributes(kw_attributes)
-        delete!(attrs, :show_axis)
-        delete!(attrs, :limits)
-        plot!(proxyscene, P, attrs, args...)
+        # We dont forward the attributes to the plot, since we only need the arguments to determine the axis type
+        # Remove arguments may not work with plotting into the scene
+        delete!(plot_attr, :show_axis)
+        delete!(plot_attr, :limits)
+        delete!(plot_attr, :color) # Color may contain Cycled(1), which needs the axis to get resolved to a color
+        plot!(proxyscene, P, Attributes(plot_attr), plot_args...)
         if is2d(proxyscene)
-            ax = Axis(fig; axis...)
+            ax = Axis(fig; axis_kw...)
         else
-            ax = LScene(fig; axis...)
+            ax = LScene(fig; axis_kw...)
         end
         empty!(proxyscene)
     end
+    return ax
+end
+@specialize
+
+function plot(P::PlotFunc, args...; axis=NamedTuple(), figure=NamedTuple(), kw_attributes...)
+    _validate_nt_like_keyword(axis, "axis")
+    _validate_nt_like_keyword(figure, "figure")
+
+    fig = Figure(; figure...)
+
+    axis = Dict(pairs(axis))
+    ax = get_axis(fig, P, axis, Dict{Symbol, Any}(kw_attributes), args)
 
     fig[1, 1] = ax
     p = plot!(ax, P, Attributes(kw_attributes), args...)
-    FigureAxisPlot(fig, ax, p)
+    return FigureAxisPlot(fig, ax, p)
 end
 
 # without scenelike, use current axis of current figure
@@ -66,16 +73,13 @@ function plot!(P::PlotFunc, args...; kw_attributes...)
     isnothing(figure) && error("There is no current figure to plot into.")
     ax = current_axis(figure)
     isnothing(ax) && error("There is no current axis to plot into.")
-    plot!(P, ax, args...; kw_attributes...)
+    return plot!(P, ax, args...; kw_attributes...)
 end
 
-function plot(P::PlotFunc, gp::GridPosition, args...; axis = NamedTuple(), kwargs...)
-
+function plot(P::PlotFunc, gp::GridPosition, args...; axis=NamedTuple(), kw_attributes...)
     _validate_nt_like_keyword(axis, "axis")
 
-    f = get_top_parent(gp)
-
-    c = contents(gp, exact = true)
+    c = contents(gp; exact=true)
     if !isempty(c)
         error("""
         You have used the non-mutating plotting syntax with a GridPosition, which requires an empty GridLayout slot to create an axis in, but there are already the following objects at this layout position:
@@ -88,42 +92,28 @@ function plot(P::PlotFunc, gp::GridPosition, args...; axis = NamedTuple(), kwarg
     end
 
     axis = Dict(pairs(axis))
-
-    if haskey(axis, :type)
-        axtype = axis[:type]
-        pop!(axis, :type)
-        ax = axtype(f; axis...)
-    else
-        proxyscene = Scene()
-        plot!(proxyscene, P, Attributes(kwargs), args...)
-        if is2d(proxyscene)
-            ax = Axis(f; axis...)
-        else
-            ax = LScene(f; axis...)
-        end
-    end
+    fig = get_top_parent(gp)
+    ax = get_axis(fig, P, axis, Dict{Symbol,Any}(kw_attributes), args)
 
     gp[] = ax
-    p = plot!(P, ax, args...; kwargs...)
-    AxisPlot(ax, p)
+    p = plot!(P, ax, args...; kw_attributes...)
+    return AxisPlot(ax, p)
 end
 
 function plot!(P::PlotFunc, gp::GridPosition, args...; kwargs...)
-
-    c = contents(gp, exact = true)
+    c = contents(gp; exact=true)
     if !(length(c) == 1 && can_be_current_axis(c[1]))
         error("There needs to be a single axis-like object at $(gp.span), $(gp.side) to plot into.\nUse a non-mutating plotting command to create an axis implicitly.")
     end
     ax = first(c)
-    plot!(P, ax, args...; kwargs...)
+    return plot!(P, ax, args...; kwargs...)
 end
 
-function plot(P::PlotFunc, gsp::GridSubposition, args...; axis = NamedTuple(), kwargs...)
-
+function plot(P::PlotFunc, gsp::GridSubposition, args...; axis=NamedTuple(), kw_attributes...)
     _validate_nt_like_keyword(axis, "axis")
 
-    layout = GridLayoutBase.get_layout_at!(gsp.parent, createmissing = true)
-    c = contents(gsp, exact = true)
+    GridLayoutBase.get_layout_at!(gsp.parent; createmissing=true)
+    c = contents(gsp; exact=true)
     if !isempty(c)
         error("""
         You have used the non-mutating plotting syntax with a GridSubposition, which requires an empty GridLayout slot to create an axis in, but there are already the following objects at this layout position:
@@ -136,41 +126,25 @@ function plot(P::PlotFunc, gsp::GridSubposition, args...; axis = NamedTuple(), k
     end
 
     fig = get_top_parent(gsp)
-
     axis = Dict(pairs(axis))
-
-    if haskey(axis, :type)
-        axtype = axis[:type]
-        pop!(axis, :type)
-        ax = axtype(fig; axis...)
-    else
-        proxyscene = Scene()
-        plot!(proxyscene, P, Attributes(kwargs), args...)
-
-        if is2d(proxyscene)
-            ax = Axis(fig; axis...)
-        else
-            ax = LScene(fig; axis..., scenekw = (camera = automatic,))
-        end
-    end
+    ax = get_axis(fig, P, axis, Dict{Symbol,Any}(kw_attributes), args)
 
     gsp.parent[gsp.rows, gsp.cols, gsp.side] = ax
-    p = plot!(P, ax, args...; kwargs...)
-    AxisPlot(ax, p)
+    p = plot!(P, ax, args...; kw_attributes...)
+    return AxisPlot(ax, p)
 end
 
 function plot!(P::PlotFunc, gsp::GridSubposition, args...; kwargs...)
-
-    layout = GridLayoutBase.get_layout_at!(gsp.parent, createmissing = false)
+    layout = GridLayoutBase.get_layout_at!(gsp.parent; createmissing=false)
 
     gp = layout[gsp.rows, gsp.cols, gsp.side]
 
-    c = contents(gp, exact = true)
+    c = contents(gp; exact=true)
     if !(length(c) == 1 && can_be_current_axis(c[1]))
         error("There is not just one axis at $(gp).")
     end
     ax = first(c)
-    plot!(P, ax, args...; kwargs...)
+    return plot!(P, ax, args...; kwargs...)
 end
 
 update_state_before_display!(f::FigureAxisPlot) = update_state_before_display!(f.figure)
diff --git a/src/makielayout/blocks/axis.jl b/src/makielayout/blocks/axis.jl
index 44ecafcf582..1e354ade703 100644
--- a/src/makielayout/blocks/axis.jl
+++ b/src/makielayout/blocks/axis.jl
@@ -781,7 +781,6 @@ function Makie.plot!(
 
     _disallow_keyword(:axis, allattrs)
     _disallow_keyword(:figure, allattrs)
-
     cycle = get_cycle_for_plottype(allattrs, P)
     add_cycle_attributes!(allattrs, P, cycle, la.cycler, la.palette)
 
diff --git a/src/makielayout/blocks/colorbar.jl b/src/makielayout/blocks/colorbar.jl
index 188d3435de9..1330308e364 100644
--- a/src/makielayout/blocks/colorbar.jl
+++ b/src/makielayout/blocks/colorbar.jl
@@ -43,7 +43,7 @@ function extract_colormap(@nospecialize(plot::AbstractPlot))
     end
 end
 
-function extract_colormap(plot::StreamPlot)
+function extract_colormap(plot::Union{Arrows, StreamPlot})
     return extract_colormap(plot.plots[1])
 end
 
diff --git a/src/utilities/utilities.jl b/src/utilities/utilities.jl
index 5f8e01a5628..1db23b13cbc 100644
--- a/src/utilities/utilities.jl
+++ b/src/utilities/utilities.jl
@@ -177,6 +177,7 @@ attr_broadcast_length(x::ScalarOrVector) = x.sv isa Vector ? length(x.sv) : 1
 attr_broadcast_getindex(x::NativeFont, i) = x
 attr_broadcast_getindex(x::VecTypes, i) = x # these are our rules, and for what we do, Vecs are usually scalars
 attr_broadcast_getindex(x::AbstractVector, i) = x[i]
+attr_broadcast_getindex(x::AbstractArray{T, 0}, i) where T = x[1]
 attr_broadcast_getindex(x::AbstractPattern, i) = x
 attr_broadcast_getindex(x, i) = x
 attr_broadcast_getindex(x::Ref, i) = x[] # unwrap Refs just like in normal broadcasting, for protecting iterables


Test patch:
diff --git a/test/pipeline.jl b/test/pipeline.jl
index 3faf0ca29ff..9412face4ff 100644
--- a/test/pipeline.jl
+++ b/test/pipeline.jl
@@ -34,3 +34,12 @@ end
     @test p.xmax === xmax
     fig
 end
+
+@testset "Cycled" begin
+    # Test for https://github.com/MakieOrg/Makie.jl/issues/3266
+    f, ax, pl = lines(1:4; color=Cycled(2))
+    cpalette = ax.palette[:color][]
+    @test pl.calculated_colors[] == cpalette[2]
+    pl2 = lines!(ax, 1:4; color=Cycled(1))
+    @test pl2.calculated_colors[] == cpalette[1]
+end
diff --git a/test/primitives.jl b/test/primitives.jl
new file mode 100644
index 00000000000..17a2acaaacb
--- /dev/null
+++ b/test/primitives.jl
@@ -0,0 +1,50 @@
+@testset "ablines" begin
+    # Test ablines with 0 dim arrays
+    f, ax, pl = ablines(fill(0), fill(1))
+    reset_limits!(ax)
+    points = pl.plots[1][1]
+    @test points[] == [Point2f(0), Point2f(10)]
+    limits!(ax, 5, 15, 6, 17)
+    @test points[] == [Point2f(5), Point2f(15)]
+end
+
+
+@testset "arrows" begin
+    # Test for:
+    # https://github.com/MakieOrg/Makie.jl/issues/3273
+    directions = decompose(Point2f, Circle(Point2f(0), 1))
+    points = decompose(Point2f, Circle(Point2f(0), 0.5))
+    color = range(0, 1, length=length(directions))
+    fig, ax, pl = arrows(points, directions; color=color)
+    cbar = Colorbar(fig[1, 2], pl)
+    @test cbar.limits[] == Vec2f(0, 1)
+    pl.colorrange = (0.5, 0.6)
+    @test cbar.limits[] ≈ Vec2f(0.5, 0.6)
+end
+
+
+# TODO, test all primitives and argument conversions
+
+# text()
+# fig, ax, p = meshscatter(1:5, (1:5) .+ 5, rand(5))
+# fig, ax, p = scatter(1:5, rand(5))
+# fig, ax, p = mesh(Sphere(Point3f(0), 1.0))
+# fig, ax, p = linesegments(1:5, rand(5))
+# fig, ax, p = lines(1:5, rand(5))
+# fig, ax, p = surface(rand(4, 7))
+# fig, ax, p = volume(rand(4, 4, 4))
+# begin
+#     fig, ax, p = heatmap(rand(4, 4))
+#     scatter!(Makie.point_iterator(p) |> collect, color=:red, markersize=10)
+#     display(fig)
+# end
+
+# fig, ax, p = image(rand(4, 5))
+# scatter!(Makie.point_iterator(p) |> collect, color=:red, markersize=10)
+# display(fig)
+
+# begin
+#     fig, ax, p = scatter(1:5, rand(5))
+#     linesegments!(Makie.data_limits(ax.scene), color=:red)
+#     display(fig)
+# end
diff --git a/test/runtests.jl b/test/runtests.jl
index 3ade28d3d1b..6b0ce30ad40 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -18,6 +18,7 @@ using Makie: volume
         @test all(hi .>= (8,8,10))
     end
 
+    include("primitives.jl")
     include("pipeline.jl")
     include("record.jl")
     include("scenes.jl")
diff --git a/test/test_primitives.jl b/test/test_primitives.jl
deleted file mode 100644
index 851d56866a4..00000000000
--- a/test/test_primitives.jl
+++ /dev/null
@@ -1,25 +0,0 @@
-using Makie
-
-text()
-fig, ax, p = meshscatter(1:5, (1:5) .+ 5, rand(5))
-fig, ax, p = scatter(1:5, rand(5))
-fig, ax, p = mesh(Sphere(Point3f(0), 1.0))
-fig, ax, p = linesegments(1:5, rand(5))
-fig, ax, p = lines(1:5, rand(5))
-fig, ax, p = surface(rand(4, 7))
-fig, ax, p = volume(rand(4, 4, 4))
-begin
-    fig, ax, p = heatmap(rand(4, 4))
-    scatter!(Makie.point_iterator(p) |> collect, color=:red, markersize=10)
-    display(fig)
-end
-
-fig, ax, p = image(rand(4, 5))
-scatter!(Makie.point_iterator(p) |> collect, color=:red, markersize=10)
-display(fig)
-
-begin
-    fig, ax, p = scatter(1:5, rand(5))
-    linesegments!(Makie.data_limits(ax.scene), color=:red)
-    display(fig)
-end