From 8fd5a1ac36172f4abe0c018133b1ed92bf57d12e Mon Sep 17 00:00:00 2001 From: SimonDanisch Date: Wed, 27 Sep 2023 13:34:20 +0200 Subject: [PATCH] fix plotspec + clean up --- GLMakie/src/drawing_primitives.jl | 24 ++++++------- MakieCore/src/recipes.jl | 10 +++--- MakieCore/src/types.jl | 2 +- src/basic_recipes/plotspec.jl | 35 ++++++++++++++----- src/interfaces.jl | 56 ++++++++++++------------------- src/scenes.jl | 16 --------- 6 files changed, 66 insertions(+), 77 deletions(-) diff --git a/GLMakie/src/drawing_primitives.jl b/GLMakie/src/drawing_primitives.jl index 43001d2b9ad..540e1e7d62d 100644 --- a/GLMakie/src/drawing_primitives.jl +++ b/GLMakie/src/drawing_primitives.jl @@ -42,33 +42,31 @@ function connect_camera!(plot, gl_attributes, cam, space = gl_attributes[:space] gl_attributes[key] = lift(identity, plot, getfield(cam, key)) end get!(gl_attributes, :view) do - get!(cam.calculated_values, :view) do + # get!(cam.calculated_values, Symbol("view_$(space[])")) do return lift(plot, cam.view, space) do view, space return is_data_space(space) ? view : Mat4f(I) end - end + # end end get!(gl_attributes, :normalmatrix) do - get!(cam.calculated_values, :normalmatrix) do - return lift(plot, gl_attributes[:view], gl_attributes[:model]) do v, m - i = Vec(1, 2, 3) - return transpose(inv(v[i, i] * m[i, i])) - end + return lift(plot, gl_attributes[:view], gl_attributes[:model]) do v, m + i = Vec(1, 2, 3) + return transpose(inv(v[i, i] * m[i, i])) end end get!(gl_attributes, :projection) do - get!(cam.calculated_values, :projection) do + # return get!(cam.calculated_values, Symbol("projection_$(space[])")) do return lift(cam.projection, cam.pixel_space, space) do _, _, space return Makie.space_to_clip(cam, space, false) end - end + # end end get!(gl_attributes, :projectionview) do - get!(cam.calculated_values, :projectionview) do + # get!(cam.calculated_values, Symbol("projectionview_$(space[])")) do return lift(plot, cam.projectionview, cam.pixel_space, space) do _, _, space Makie.space_to_clip(cam, space, true) end - end + # end end # resolution in real hardware pixels, not scaled pixels/units get!(gl_attributes, :resolution) do @@ -76,7 +74,7 @@ function connect_camera!(plot, gl_attributes, cam, space = gl_attributes[:space] return lift(*, plot, gl_attributes[:px_per_unit], cam.resolution) end end - + delete!(gl_attributes, :space) delete!(gl_attributes, :markerspace) return nothing @@ -156,7 +154,7 @@ function cached_robj!(robj_func, screen, scene, x::AbstractPlot) return robj end -Makie.Base.insert!(screen::GLMakie.Screen, scene::Scene, x::Makie.PlotList) = nothing +Base.insert!(::GLMakie.Screen, ::Scene, ::Makie.PlotList) = nothing function Base.insert!(screen::Screen, scene::Scene, x::Combined) ShaderAbstractions.switch_context!(screen.glscreen) diff --git a/MakieCore/src/recipes.jl b/MakieCore/src/recipes.jl index 19d6d55ff4f..2f3f564cfb9 100644 --- a/MakieCore/src/recipes.jl +++ b/MakieCore/src/recipes.jl @@ -31,28 +31,28 @@ function create_figurelike! end function figurelike_return end function figurelike_return! end -function _create_plot(F, attributes, args...) +function _create_plot(F, attributes::Dict, args...) figlike, plot_kw, plot_args = create_figurelike(Combined{F}, attributes, args...) plot = Combined{F}(plot_args, plot_kw) plot!(figlike, plot) return figurelike_return(figlike, plot) end -function _create_plot!(F, attributes, args...) +function _create_plot!(F, attributes::Dict, args...) figlike, plot_kw, plot_args = create_figurelike!(Combined{F}, attributes, args...) plot = Combined{F}(plot_args, plot_kw) plot!(figlike, plot) return figurelike_return!(figlike, plot) end -function _create_plot!(F, kw, scene::SceneLike, args...) +function _create_plot!(F, kw::Dict, scene::SceneLike, args...) plot = Combined{F}(args, kw) plot!(scene, plot) return plot end -plot(args...; kw...) = _create_plot(plot, Dict(kw), args...) -plot!(args...; kw...) = _create_plot!(plot, Dict(kw), args...) +plot(args...; kw...) = _create_plot(plot, Dict{Symbol, Any}(kw), args...) +plot!(args...; kw...) = _create_plot!(plot, Dict{Symbol, Any}(kw), args...) """ diff --git a/MakieCore/src/types.jl b/MakieCore/src/types.jl index 17fe6e86f5a..a3c8c711aba 100644 --- a/MakieCore/src/types.jl +++ b/MakieCore/src/types.jl @@ -65,7 +65,7 @@ mutable struct Combined{Typ, T} <: ScenePlot{Typ} deregister_callbacks::Vector{Observables.ObserverFunction} parent::Union{AbstractScene,Combined} - function Combined{Typ,T}(transformation, kw, args) where {Typ,T} + function Combined{Typ,T}(transformation, kw::Dict{Symbol, Any}, args::Vector{Any}) where {Typ,T} return new{Typ,T}(transformation, kw, args, (), Attributes(), Combined[], Observables.ObserverFunction[]) end diff --git a/src/basic_recipes/plotspec.jl b/src/basic_recipes/plotspec.jl index b7172153783..de5034a12bb 100644 --- a/src/basic_recipes/plotspec.jl +++ b/src/basic_recipes/plotspec.jl @@ -48,6 +48,10 @@ function apply_convert!(P, attributes::Attributes, x::PlotSpec{S}) where {S} return (plottype(S, P), (args...,)) end +function apply_convert!(P, ::Attributes, x::AbstractVector{<:PlotSpec}) + return (PlotList, (x,)) +end + function MakieCore.argtypes(plot::PlotSpec{P}) where {P} args_converted = convert_arguments(P, plot.args...) return MakieCore.argtypes(args_converted) @@ -81,7 +85,7 @@ function update_plot!(plot::AbstractPlot, spec::PlotSpec) for i in eachindex(spec.args) # we should only call update_plot!, if compare_spec(spec_plot_got_created_from, spec) == true, # Which should guarantee, that args + kwargs have the same length and types! - arg_obs = plot.input_args[i] + arg_obs = plot.args[i] if to_value(arg_obs) != spec.args[i] # only update if different @debug("updating arg $i") arg_obs[] = spec.args[i] @@ -135,7 +139,7 @@ plots[] = [ end convert_arguments(::Type{<:AbstractPlot}, args::AbstractArray{<:PlotSpec}) = (args,) -plottype(::AbstractArray{<:PlotSpec}) = PlotList +plottype(::AbstractVector{<:PlotSpec}) = PlotList # Since we directly plot into the parent scene (hacky), we need to overload these Base.insert!(::MakieScreen, ::Scene, ::PlotList) = nothing @@ -148,6 +152,22 @@ quote ] end +function Base.show(io::IO, ::MIME"text/plain", spec::PlotSpec{P}) where {P} + args = join(map(x -> string("::", typeof(x)), spec.args), ", ") + kws = join([string(k, " = ", typeof(v)) for (k, v) in spec.kwargs], ", ") + println(io, "P.", plotfunc(P), "($args; $kws)") +end + +function Base.show(io::IO, spec::PlotSpec{P}) where {P} + args = join(map(x -> string("::", typeof(x)), spec.args), ", ") + kws = join([string(k, " = ", typeof(v)) for (k, v) in spec.kwargs], ", ") + return println(io, "P.", plotfunc(P), "($args; $kws)") +end + +function to_combined(ps::PlotSpec{P}) where {P} + return P((ps.args...,), copy(ps.kwargs)) +end + function Makie.plot!(p::PlotList{<: Tuple{<: AbstractArray{<: PlotSpec}}}) # Cache plots here so that we aren't re-creating plots every time; # if a plot still exists from last time, update it accordingly. @@ -163,11 +183,8 @@ function Makie.plot!(p::PlotList{<: Tuple{<: AbstractArray{<: PlotSpec}}}) if isnothing(idx) @debug("Creating new plot for spec") # Create new plot, store it into our `cached_plots` dictionary - plot = plot!(scene, - typeof(plotspec).parameters[1], - Attributes(plotspec.kwargs), - plotspec.args..., - ) + plot = plot!(scene, to_combined(plotspec)) + push!(p.plots, plot) push!(cached_plots, plotspec => plot) push!(used_plots, length(cached_plots)) else @@ -182,13 +199,15 @@ function Makie.plot!(p::PlotList{<: Tuple{<: AbstractArray{<: PlotSpec}}}) # Next, delete all plots that we haven't used # TODO, we could just hide them, until we reach some max_plots_to_be_cached, so that we re-create less plots. for idx in unused_plots - spec, plot = cached_plots[idx] + _, plot = cached_plots[idx] delete!(scene, plot) + filter!(x -> x !== plot, p.plots) end splice!(cached_plots, sort!(collect(unused_plots))) end end +# Prototype for Pluto + Ijulia integration with Observable(ListOfPlots) function Base.showable(::Union{MIME"juliavscode/html",MIME"text/html"}, ::Observable{<: AbstractVector{<:PlotSpec}}) return true end diff --git a/src/interfaces.jl b/src/interfaces.jl index 34bd71c6441..ae2920ae8fd 100644 --- a/src/interfaces.jl +++ b/src/interfaces.jl @@ -129,7 +129,9 @@ function Combined{Func}(args::Tuple, plot_attributes::Dict) where {Func} return Combined{Func}(Base.tail(args), plot_attributes) end P = Combined{Func} - args_converted = convert_arguments(P, map(to_value, args)...) + used_attrs = used_attributes(P, to_value.(args)...) + kw = [Pair(k, to_value(v)) for (k, v) in plot_attributes if k in used_attrs] + args_converted = convert_arguments(P, map(to_value, args)...; kw...) PNew, converted = apply_convert!(P, Attributes(), args_converted) trans = get!(plot_attributes, :transformation, automatic) transval = to_value(trans) @@ -194,19 +196,6 @@ function seperate_tuple(args::Observable{<: NTuple{N, Any}}) where N end end -function plot(scene::Scene, plot::AbstractPlot) - # plot object contains local theme (default values), and user given values (from constructor) - # fill_theme now goes through all values that are missing from the user, and looks if the scene - # contains any theming values for them (via e.gg. css rules). If nothing founds, the values will - # be taken from local theme! This will connect any values in the scene's theme - # with the plot values and track those connection, so that we can separate them - # when doing delete!(scene, plot)! - complete_theme!(scene, plot) - # we just return the plot... whoever calls plot (our pipeline usually) - # will need to push!(scene, plot) etc! - return plot -end - ## generic definitions # If the Combined has no plot func, calculate them plottype(::Type{<: Combined{Any}}, argvalues...) = plottype(argvalues...) @@ -248,15 +237,32 @@ plottype(P1::Type{<: Combined{T}}, P2::Type{<: Combined}) where T = P1 # all the plotting functions that get a plot type const PlotFunc = Union{Type{Any},Type{<:AbstractPlot}} - function plot!(::Combined{F}) where {F} if !(F in atomic_functions) error("No recipe for $(F)") end end +function connect_plot!(scene::SceneLike, plot::Combined{F}) where {F} + plot.parent = scene + # TODO, move transformation into attributes? + # This hacks around transformation being already constructed in the constructor + # So here we don't want to connect to the scene if an explicit Transformation was passed to the plot + kw = getfield(plot, :kw) + attr = getfield(plot, :attributes) + t = to_value(get(() -> get(kw, :transformation, nothing), attr, :transformation)) + if t isa Automatic + connect!(transformation(scene), transformation(plot)) + end + apply_theme!(parent_scene(scene), plot) + convert_arguments!(plot) + calculated_attributes!(Combined{F}, plot) + plot!(plot) + return plot +end + function plot!(scene::SceneLike, plot::Combined) - prepare_plot!(scene, plot) + connect_plot!(scene, plot) push!(scene, plot) return plot end @@ -278,21 +284,3 @@ function apply_theme!(scene::Scene, plot::P) where {P<: Combined} end return merge!(plot.attributes, plot_theme) end - -function prepare_plot!(scene::SceneLike, plot::Combined{F}) where {F} - plot.parent = scene - # TODO, move transformation into attributes? - # This hacks around transformation being already constructed in the constructor - # So here we don't want to connect to the scene if an explicit Transformation was passed to the plot - kw = getfield(plot, :kw) - attr = getfield(plot, :attributes) - t = to_value(get(() -> get(kw, :transformation, nothing), attr, :transformation)) - if t isa Automatic - connect!(transformation(scene), transformation(plot)) - end - apply_theme!(parent_scene(scene), plot) - convert_arguments!(plot) - calculated_attributes!(Combined{F}, plot) - plot!(plot) - return plot -end diff --git a/src/scenes.jl b/src/scenes.jl index 6b0c2ba25e2..5c17e6e906f 100644 --- a/src/scenes.jl +++ b/src/scenes.jl @@ -473,7 +473,6 @@ end function Base.push!(scene::Scene, plot::AbstractPlot) push!(scene.plots, plot) - plot isa Combined || (plot.parent = scene) for screen in scene.current_screens insert!(screen, scene, plot) end @@ -511,21 +510,6 @@ function Base.delete!(scene::Scene, plot::AbstractPlot) free(plot) end -function Base.push!(scene::Scene, child::Scene) - push!(scene.children, child) - disconnect!(child.camera) - observables = map([:view, :projection, :projectionview, :resolution, :eyeposition]) do field - return lift(getfield(scene.camera, field)) do val - getfield(child.camera, field)[] = val - getfield(child.camera, field)[] = val - return - end - end - cameracontrols!(child, observables) - child.parent = scene - return scene -end - events(x) = events(get_scene(x)) events(scene::Scene) = scene.events events(scene::SceneLike) = events(scene.parent)