Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix depwarn for cairomakie lines #3200

Merged
merged 2 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 16 additions & 32 deletions CairoMakie/src/primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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][]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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[]
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion MakieCore/src/attributes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions MakieCore/src/recipes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions ReferenceTests/src/tests/primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions src/utilities/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down