Fix depwarn for cairomakie lines (#3200)

* Fix depwarn for cairomakie lines * clean up get_attribute
MakieOrg · Sep 1, 2023 · ac02141 · ac02141
1 parent 48fa1f9
commit ac02141
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 41 deletions.
diff --git a/CairoMakie/src/primitives.jl b/CairoMakie/src/primitives.jl
@@ -3,8 +3,7 @@
 ################################################################################
 
 function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Union{Lines, LineSegments}))
-    fields = @get_attribute(primitive, (color, linewidth, linestyle))
-    linestyle = Makie.convert_attribute(linestyle, Makie.key"linestyle"())
+    @get_attribute(primitive, (color, linewidth, linestyle))
     ctx = screen.context
     model = primitive[:model][]
     positions = primitive[1][]
@@ -245,7 +244,7 @@ function draw_multi(primitive::Lines, ctx, positions, colors::AbstractArray, lin
                     this_linewidth != prev_linewidth && error("Encountered two different linewidth values $prev_linewidth and $this_linewidth in `lines` at index $(i-1). Different linewidths in one line are only permitted in CairoMakie when separated by a NaN point.")
                     Cairo.line_to(ctx, this_position...)
                     prev_continued = true
-                    
+
                     if i == lastindex(positions)
                         # this is the last element so stroke this
                         Cairo.set_line_width(ctx, this_linewidth)
@@ -293,8 +292,7 @@ end
 ################################################################################
 
 function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scatter))
-    fields = @get_attribute(primitive, (markersize, strokecolor, strokewidth, marker, marker_offset, rotations))
-    @get_attribute(primitive, (transform_marker,))
+    @get_attribute(primitive, (markersize, strokecolor, strokewidth, marker, marker_offset, rotations, transform_marker))
 
     ctx = screen.context
     model = primitive.model[]
@@ -306,22 +304,15 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scat
 
     colors = to_color(primitive.calculated_colors[])
 
-    markerspace = to_value(get(primitive, :markerspace, :pixel))
-    space = to_value(get(primitive, :space, :data))
-
+    markerspace = primitive.markerspace[]
+    space = primitive.space[]
     transfunc = Makie.transform_func(primitive)
 
-    marker_conv = _marker_convert(marker)
-
-    draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker_conv, marker_offset, rotations, model, positions, size_model, font, markerspace, space)
+    return draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker,
+                               marker_offset, rotations, model, positions, size_model, font, markerspace,
+                               space)
 end
 
-# an array of markers is converted to string by itself, which is inconvenient for the iteration logic
-_marker_convert(markers::AbstractArray) = map(m -> convert_attribute(m, key"marker"(), key"scatter"()), markers)
-_marker_convert(marker) = convert_attribute(marker, key"marker"(), key"scatter"())
-# image arrays need to be converted as a whole
-_marker_convert(marker::AbstractMatrix{<:Colorant}) = [ convert_attribute(marker, key"marker"(), key"scatter"()) ]
-
 function draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker, marker_offset, rotations, model, positions, size_model, font, markerspace, space)
     broadcast_foreach(positions, colors, markersize, strokecolor,
             strokewidth, marker, marker_offset, remove_billboard(rotations)) do point, col,
@@ -336,15 +327,14 @@ function draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokeco
         Cairo.set_source_rgba(ctx, rgbatuple(col)...)
 
         Cairo.save(ctx)
-        marker_converted = Makie.to_spritemarker(m)
         # Setting a markersize of 0.0 somehow seems to break Cairos global state?
         # At least it stops drawing any marker afterwards
         # TODO, maybe there's something wrong somewhere else?
         if !(norm(scale) ≈ 0.0)
-            if marker_converted isa Char
-                draw_marker(ctx, marker_converted, best_font(m, font), pos, scale, strokecolor, strokewidth, offset, rotation)
+            if m isa Char
+                draw_marker(ctx, m, best_font(m, font), pos, scale, strokecolor, strokewidth, offset, rotation)
             else
-                draw_marker(ctx, marker_converted, pos, scale, strokecolor, strokewidth, offset, rotation)
+                draw_marker(ctx, m, pos, scale, strokecolor, strokewidth, offset, rotation)
             end
         end
         Cairo.restore(ctx)
@@ -691,7 +681,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
     t = Makie.transform_func(primitive)
     identity_transform = (t === identity || t isa Tuple && all(x-> x === identity, t)) && (abs(model[1, 2]) < 1e-15)
     regular_grid = xs isa AbstractRange && ys isa AbstractRange
-    xy_aligned = let 
+    xy_aligned = let
         # Only allow scaling and translation
         pv = scene.camera.projectionview[]
         M = Mat4f(
@@ -720,7 +710,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
     xymax = project_position(scene, space, Point2f(last.(imsize)), model)
     w, h = xymax .- xy
 
-    can_use_fast_path = !(is_vector && !interpolate) && regular_grid && identity_transform && 
+    can_use_fast_path = !(is_vector && !interpolate) && regular_grid && identity_transform &&
         (interpolate || xy_aligned)
     use_fast_path = can_use_fast_path && !disable_fast_path
 
@@ -862,23 +852,19 @@ nan2zero(x) = !isnan(x) * x
 
 
 function draw_mesh3D(scene, screen, attributes, mesh; pos = Vec4f(0), scale = 1f0)
-    # Priorize colors of the mesh if present
-    @get_attribute(attributes, (color,))
+    @get_attribute(attributes, (shading, diffuse, specular, shininess, faceculling))
 
     matcap = to_value(get(attributes, :matcap, nothing))
-
     meshpoints = decompose(Point3f, mesh)::Vector{Point3f}
     meshfaces = decompose(GLTriangleFace, mesh)::Vector{GLTriangleFace}
     meshnormals = decompose_normals(mesh)::Vector{Vec3f}
     meshuvs = texturecoordinates(mesh)::Union{Nothing, Vector{Vec2f}}
 
+    # Priorize colors of the mesh if present
     color = hasproperty(mesh, :color) ? mesh.color : to_value(attributes.calculated_colors)
 
     per_face_col = per_face_colors(color, matcap, meshfaces, meshnormals, meshuvs)
 
-    @get_attribute(attributes, (shading, diffuse,
-        specular, shininess, faceculling))
-
     model = attributes.model[]::Mat4f
     space = to_value(get(attributes, :space, :data))::Symbol
     func = Makie.transform_func(attributes)
@@ -1052,8 +1038,6 @@ end
 
 function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Makie.MeshScatter))
     @get_attribute(primitive, (model, marker, markersize, rotations))
-
-    m = convert_attribute(marker, key"marker"(), key"meshscatter"())
     pos = primitive[1][]
     # For correct z-ordering we need to be in view/camera or screen space
     model = copy(model)
@@ -1093,7 +1077,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
         scale = markersize isa Vector ? markersize[i] : markersize
 
         draw_mesh3D(
-            scene, screen, submesh, m, pos = p,
+            scene, screen, submesh, marker, pos = p,
             scale = scale isa Real ? Vec3f(scale) : to_ndim(Vec3f, scale, 1f0)
         )
     end

diff --git a/MakieCore/src/attributes.jl b/MakieCore/src/attributes.jl
@@ -235,7 +235,12 @@ function get_attribute(dict, key, default=nothing)
     if haskey(dict, key)
         value = to_value(dict[key])
         value isa Automatic && return default
-        return convert_attribute(to_value(dict[key]), Key{key}())
+        plot_k = plotkey(dict)
+        if isnothing(plot_k)
+            return convert_attribute(value, Key{key}())
+        else
+            return convert_attribute(value, Key{key}(), Key{plot_k}())
+        end
     else
         return default
     end

diff --git a/MakieCore/src/recipes.jl b/MakieCore/src/recipes.jl
@@ -21,6 +21,7 @@ func2type(f::Function) = Combined{f}
 plotkey(::Type{<: AbstractPlot{Typ}}) where Typ = Symbol(lowercase(func2string(Typ)))
 plotkey(::T) where T <: AbstractPlot = plotkey(T)
 plotkey(::Nothing) = :scatter
+plotkey(any) = nothing
 
 """
      default_plot_signatures(funcname, funcname!, PlotType)

diff --git a/ReferenceTests/src/tests/primitives.jl b/ReferenceTests/src/tests/primitives.jl
@@ -5,7 +5,7 @@
     points = Point2f[(1, 1), (1, 2), (2, 3), (2, 1)]
     linestyles = [
         :solid, :dash, :dot, :dashdot, :dashdotdot,
-        [1, 2, 3], [1, 2, 4, 5]
+        Linestyle([1, 2, 3]), Linestyle([1, 2, 4, 5])
     ]
     for linewidth in 1:10
         for (i, linestyle) in enumerate(linestyles)
@@ -270,13 +270,13 @@ end
 
     # Same as above
     markers = [
-        :rect, :circle, :cross, :x, :utriangle, :rtriangle, :dtriangle, :ltriangle, :pentagon, 
+        :rect, :circle, :cross, :x, :utriangle, :rtriangle, :dtriangle, :ltriangle, :pentagon,
         :hexagon, :octagon, :star4, :star5, :star6, :star8, :vline, :hline, 'x', 'X'
     ]
 
     for (i, marker) in enumerate(markers)
         scatter!(
-            Point2f.(1:5, i), marker = marker, 
+            Point2f.(1:5, i), marker = marker,
             markersize = range(10, 30, length = 5), color = :orange,
             strokewidth = 2, strokecolor = :black
         )
@@ -451,9 +451,9 @@ end
     lab1 = L"\int f(x) dx"
     lab2 = lab1
     # lab2 = L"\frac{a}{b} - \sqrt{b}" # this will not work until #2667 is fixed
-    
+
     barplot(fig[1,1], [1, 2], [0.5, 0.2], bar_labels = [lab1, lab2], flip_labels_at = 0.3, direction=:x)
     barplot(fig[1,2], [1, 2], [0.5, 0.2], bar_labels = [lab1, lab2], flip_labels_at = 0.3)
 
     fig
-end
+end
diff --git a/src/utilities/utilities.jl b/src/utilities/utilities.jl
@@ -169,20 +169,20 @@ end
 
 attr_broadcast_length(x::NativeFont) = 1
 attr_broadcast_length(x::VecTypes) = 1 # these are our rules, and for what we do, Vecs are usually scalars
-attr_broadcast_length(x::AbstractArray) = length(x)
+attr_broadcast_length(x::AbstractVector) = length(x)
 attr_broadcast_length(x::AbstractPattern) = 1
 attr_broadcast_length(x) = 1
 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::AbstractArray, i) = x[i]
+attr_broadcast_getindex(x::AbstractVector, i) = x[i]
 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
 attr_broadcast_getindex(x::ScalarOrVector, i) = x.sv isa Vector ? x.sv[i] : x.sv
 
-is_vector_attribute(x::AbstractArray) = true
+is_vector_attribute(x::AbstractVector) = true
 is_vector_attribute(x::NativeFont) = false
 is_vector_attribute(x::Quaternion) = false
 is_vector_attribute(x::VecTypes) = false