From 4f045d7a804a4325d567951397e711be36c9d4fe Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 29 Oct 2024 20:06:03 +0100 Subject: [PATCH 01/17] Fix CI slowdown by relying on new bonito version with new HTTP.jl (#4542) properly fix CI slowdown by relying on new bonito version with new http version --- WGLMakie/Project.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/WGLMakie/Project.toml b/WGLMakie/Project.toml index 3ac4cf07a6b..bf3a3d33a64 100644 --- a/WGLMakie/Project.toml +++ b/WGLMakie/Project.toml @@ -11,7 +11,6 @@ FreeTypeAbstraction = "663a7486-cb36-511b-a19d-713bb74d65c9" GeometryBasics = "5c1252a2-5f33-56bf-86c9-59e7332b4326" Hyperscript = "47d2ed2b-36de-50cf-bf87-49c2cf4b8b91" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" -LoggingExtras = "e6f89c97-d47a-5376-807f-9c37f3926c36" Makie = "ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a" Observables = "510215fc-4207-5dde-b226-833fc4488ee2" PNGFiles = "f57f5aa1-a3ce-4bc8-8ab9-96f992907883" @@ -21,14 +20,13 @@ ShaderAbstractions = "65257c39-d410-5151-9873-9b3e5be5013e" StaticArrays = "90137ffa-7385-5640-81b9-e52037218182" [compat] -Bonito = "3.2.1" +Bonito = "3.2.4" Colors = "0.11, 0.12, 0.13" FileIO = "1.1" FreeTypeAbstraction = "0.10" GeometryBasics = "0.4.11" Hyperscript = "0.0.3, 0.0.4, 0.0.5" LinearAlgebra = "1.0, 1.6" -LoggingExtras = "<1.1.0" Makie = "=0.21.15" Observables = "0.5.1" PNGFiles = "0.3, 0.4" From 22863fd53f9ffd122ec624735e537ec479dcf2d8 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:17:39 +0100 Subject: [PATCH 02/17] Fix docstrings for 1.11 (#4548) --- MakieCore/src/recipes.jl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/MakieCore/src/recipes.jl b/MakieCore/src/recipes.jl index 4b1d9aaff97..fd44f41fdbe 100644 --- a/MakieCore/src/recipes.jl +++ b/MakieCore/src/recipes.jl @@ -392,6 +392,14 @@ end function types_for_plot_arguments end +function extract_docstring(str) + if VERSION >= v"1.11" && str isa Base.Docs.DocStr + return only(str.text::Core.SimpleVector) + else + return str + end +end + function create_recipe_expr(Tsym, args, attrblock) funcname_sym = to_func_name(Tsym) funcname!_sym = Symbol("$(funcname_sym)!") @@ -421,7 +429,7 @@ function create_recipe_expr(Tsym, args, attrblock) Core.@__doc__ $(esc(docs_placeholder)) = nothing binding = Docs.Binding(@__MODULE__, $(QuoteNode(docs_placeholder))) user_docstring = if haskey(Docs.meta(@__MODULE__), binding) - _docstring = @doc($docs_placeholder) + _docstring = extract_docstring(@doc($docs_placeholder)) delete!(Docs.meta(@__MODULE__), binding) _docstring else @@ -462,7 +470,7 @@ function create_recipe_expr(Tsym, args, attrblock) end $(arg_type_func) - docstring_modified = make_recipe_docstring($PlotType, $(QuoteNode(Tsym)), $(QuoteNode(funcname_sym)),user_docstring) + docstring_modified = make_recipe_docstring($PlotType, $(QuoteNode(Tsym)), $(QuoteNode(funcname_sym)), user_docstring) @doc docstring_modified $funcname_sym @doc "`$($(string(Tsym)))` is the plot type associated with plotting function `$($(string(funcname_sym)))`. Check the docstring for `$($(string(funcname_sym)))` for further information." $Tsym @doc "`$($(string(funcname!_sym)))` is the mutating variant of plotting function `$($(string(funcname_sym)))`. Check the docstring for `$($(string(funcname_sym)))` for further information." $funcname!_sym From 70e5d2ca235f8cae79ec87743c36d372b5cd90dc Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 31 Oct 2024 12:40:14 +0100 Subject: [PATCH 03/17] implement S.Colorbar(plotspec) (#4520) * implement S.Colorbar(plotspec) * improve name * implement lookup_default, to better get colorbar defaults from spec argument * fix tests * Update CHANGELOG.md --- CHANGELOG.md | 1 + MakieCore/src/recipes.jl | 188 ++++++++++++++-------------- ReferenceTests/src/tests/specapi.jl | 40 ++++++ src/specapi.jl | 108 ++++++++++++++-- 4 files changed, 235 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b23539527..7e70668c2f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] - Added `subsup` and `left_subsup` functions that offer stacked sub- and superscripts for `rich` text which means this style can be used with arbitrary fonts and is not limited to fonts supported by MathTeXEngine.jl [#4489](https://github.com/MakieOrg/Makie.jl/pull/4489). +- Implement S.Colorbar(plotspec) [#4520](https://github.com/MakieOrg/Makie.jl/pull/4520). ## [0.21.15] - 2024-10-25 diff --git a/MakieCore/src/recipes.jl b/MakieCore/src/recipes.jl index fd44f41fdbe..6e8da443ba6 100644 --- a/MakieCore/src/recipes.jl +++ b/MakieCore/src/recipes.jl @@ -72,7 +72,7 @@ theme(x::AbstractPlot, key; default=nothing) = deepcopy(get(x.attributes, key, d Attributes(x::AbstractPlot) = x.attributes -default_theme(scene, T) = Attributes() +function default_theme end """ # Plot Recipes in `Makie` @@ -209,17 +209,58 @@ attribute_names(_) = nothing Base.@kwdef struct AttributeMetadata docstring::Union{Nothing,String} + default_value::Any default_expr::String # stringified expression, just needed for docs purposes end update_metadata(am1::AttributeMetadata, am2::AttributeMetadata) = AttributeMetadata( am2.docstring === nothing ? am1.docstring : am2.docstring, + am2.default_value, am2.default_expr # TODO: should it be possible to overwrite only a docstring by not giving a default expr? ) struct DocumentedAttributes d::Dict{Symbol,AttributeMetadata} - closure::Function +end + +struct Inherit + key::Symbol + fallback::Any +end + +function lookup_default(meta::AttributeMetadata, theme) + default = meta.default_value + if default isa Inherit + if haskey(theme, default.key) + to_value(theme[default.key]) # only use value of theme entry + else + if isnothing(default.fallback) + error("Inherited key $(default.key) not found in theme with no fallback given.") + else + return default.fallback + end + end + else + return default + end +end + +function get_default_expr(default) + if default isa Expr && default.head === :macrocall && default.args[1] === Symbol("@inherit") + if length(default.args) ∉ (3, 4) + error("@inherit works with 1 or 2 arguments, expression was $default") + end + if !(default.args[3] isa Symbol) + error("Argument 1 of @inherit must be a Symbol, got $(default.args[3])") + end + key = default.args[3] + _default = get(default.args, 4, :(nothing)) + # first check scene theme + # then default value + return :($(MakieCore.Inherit)($(QuoteNode(key)), $(esc(_default)))) + else + return esc(default) + end end macro DocumentedAttributes(expr::Expr) @@ -256,40 +297,20 @@ macro DocumentedAttributes(expr::Expr) if !(sym isa Symbol) error("$sym should be a symbol") end - - push!(metadata_exprs, quote - am = AttributeMetadata(; docstring = $docs, default_expr = $(_default_expr_string(default))) - if haskey(d, $(QuoteNode(sym))) - d[$(QuoteNode(sym))] = update_metadata(d[$(QuoteNode(sym))], am) + qsym = QuoteNode(sym) + metadata = quote + am = AttributeMetadata(; + docstring = $docs, + default_value = $(get_default_expr(default)), + default_expr = $(default_expr_string(default)) + ) + if haskey(d, $(qsym)) + d[$(qsym)] = update_metadata(d[$(qsym)], am) else - d[$(QuoteNode(sym))] = am - end - end) - - if default isa Expr && default.head === :macrocall && default.args[1] === Symbol("@inherit") - if length(default.args) ∉ (3, 4) - error("@inherit works with 1 or 2 arguments, expression was $d") + d[$(qsym)] = am end - if !(default.args[3] isa Symbol) - error("Argument 1 of @inherit must be a Symbol, got $(default.args[3])") - end - key = default.args[3] - _default = get(default.args, 4, :(error("Inherited key $($(QuoteNode(key))) not found in theme with no fallback given."))) - # first check scene theme - # then default value - d = :( - dict[$(QuoteNode(sym))] = if haskey(thm, $(QuoteNode(key))) - to_value(thm[$(QuoteNode(key))]) # only use value of theme entry - else - $(esc(_default)) - end - ) - push!(closure_exprs, d) - else - push!(closure_exprs, :( - dict[$(QuoteNode(sym))] = $(esc(default)) - )) end + push!(metadata_exprs, metadata) elseif is_mixin_line # this intermediate variable is needed to evaluate each mixin only once # and is inserted at the start of the final code block @@ -302,14 +323,6 @@ macro DocumentedAttributes(expr::Expr) end end) - # the actual runtime values of the mixed in defaults - # are computed using the closure stored in the DocumentedAttributes - closure_exp = quote - # `scene` and `dict` here are defined below where this exp is interpolated into - merge!(dict, $gsym.closure(scene)) - end - push!(closure_exprs, closure_exp) - # docstrings and default expressions of the mixed in # DocumentedAttributes are inserted metadata_exp = quote @@ -330,13 +343,7 @@ macro DocumentedAttributes(expr::Expr) $(mixin_exprs...) d = Dict{Symbol,AttributeMetadata}() $(metadata_exprs...) - closure = function (scene) - thm = theme(scene) - dict = Dict{Symbol,Any}() - $(closure_exprs...) - return dict - end - DocumentedAttributes(d, closure) + DocumentedAttributes(d) end end @@ -392,6 +399,43 @@ end function types_for_plot_arguments end +documented_attributes(_) = nothing + +function attribute_names(T::Type{<:Plot}) + attr = documented_attributes(T) + isnothing(attr) && return nothing + return keys(attr.d) +end + +function lookup_default(::Type{T}, scene, attribute::Symbol) where {T<:Plot} + thm = theme(scene) + metas = documented_attributes(T).d + psym = plotsym(T) + if haskey(thm, psym) + overwrite = thm[psym] + if haskey(overwrite, attribute) + return to_value(overwrite[attribute]) + end + end + if haskey(metas, attribute) + return lookup_default(metas[attribute], thm) + else + return nothing + end +end + +function default_theme(scene, T::Type{<: Plot}) + metas = documented_attributes(T) + attr = Attributes() + isnothing(metas) && return attr + thm = theme(scene) + _attr = attr.attributes + for (k, meta) in metas.d + _attr[k] = lookup_default(meta, thm) + end + return attr +end + function extract_docstring(str) if VERSION >= v"1.11" && str isa Base.Docs.DocStr return only(str.text::Core.SimpleVector) @@ -461,13 +505,6 @@ function create_recipe_expr(Tsym, args, attrblock) _create_plot!($funcname, kwdict, args...) end - function $(MakieCore).attribute_names(T::Type{<:$(PlotType)}) - keys(documented_attributes(T).d) - end - - function $(MakieCore).default_theme(scene, T::Type{<:$(PlotType)}) - Attributes(documented_attributes(T).closure(scene)) - end $(arg_type_func) docstring_modified = make_recipe_docstring($PlotType, $(QuoteNode(Tsym)), $(QuoteNode(funcname_sym)), user_docstring) @@ -490,6 +527,8 @@ function create_recipe_expr(Tsym, args, attrblock) return q end + + function make_recipe_docstring(P::Type{<:Plot}, Tsym, funcname_sym, docstring) io = IOBuffer() @@ -528,8 +567,8 @@ function rmlines(x::Expr) end end -_default_expr_string(x) = string(rmlines(x)) -_default_expr_string(x::String) = repr(x) +default_expr_string(x) = string(rmlines(x)) +default_expr_string(x::String) = repr(x) function extract_attribute_metadata(arg) has_docs = arg isa Expr && arg.head === :macrocall && arg.args[1] isa GlobalRef @@ -561,41 +600,6 @@ function extract_attribute_metadata(arg) (docs = docs, symbol = attr_symbol, type = type, default = default) end -function make_default_theme_expr(attrs, scenesym::Symbol) - - exprs = map(attrs) do a - - d = a.default - if d isa Expr && d.head === :macrocall && d.args[1] == Symbol("@inherit") - if length(d.args) != 4 - error("@inherit works with exactly 2 arguments, expression was $d") - end - if !(d.args[3] isa QuoteNode) - error("Argument 1 of @inherit must be a :symbol, got $(d.args[3])") - end - key, default = d.args[3:4] - # first check scene theme - # then default value - d = quote - if haskey(thm, $key) - to_value(thm[$key]) # only use value of theme entry - else - $default - end - end - end - - :(attr[$(QuoteNode(a.symbol))] = $d) - end - - quote - thm = theme($scenesym) - attr = Attributes() - $(exprs...) - attr - end -end - function expand_mixins(attrblock::Expr) Expr(:block, mapreduce(expand_mixin, vcat, attrblock.args)...) end diff --git a/ReferenceTests/src/tests/specapi.jl b/ReferenceTests/src/tests/specapi.jl index f12a39585ee..ab5847915d9 100644 --- a/ReferenceTests/src/tests/specapi.jl +++ b/ReferenceTests/src/tests/specapi.jl @@ -116,3 +116,43 @@ end sync_step!(st) st end + +function to_plot(plots) + axes = map(permutedims(plots)) do plot + ax = S.Axis(; + plots=[plot], xticksvisible=false, + yticksvisible=false, yticklabelsvisible=false, + xticklabelsvisible=false) + return S.GridLayout([ax S.Colorbar(plot)]) + end + return S.GridLayout(axes) +end + +@reference_test "Colorbar from Plots" begin + data = vcat((1:4)', (4:-1:1)') + plots = [S.Heatmap(data), + S.Image(data), + S.Lines(1:4; linewidth=4, color=1:4), + S.Scatter(1:4; markersize=20, color=1:4)] + obs = Observable(to_plot(plots)) + fig = plot(obs; figure=(; size=(700, 150))) + img1 = copy(colorbuffer(fig)) + plots = [S.Heatmap(data; colormap=:inferno), + S.Image(data; colormap=:inferno), + S.Lines(1:4; linewidth=4, color=1:4, colormap=:inferno), + S.Scatter(1:4; markersize=20, color=1:4, colormap=:inferno)] + obs[] = to_plot(plots) + img2 = copy(colorbuffer(fig)) + + plots = [S.Heatmap(data; colorrange=(2, 3)), + S.Image(data; colorrange=(2, 3)), + S.Lines(1:4; linewidth=4, color=1:4, colorrange=(2, 3)), + S.Scatter(1:4; markersize=20, color=1:4, colorrange=(2, 3))] + obs[] = to_plot(plots) + img3 = copy(colorbuffer(fig)) + + imgs = hcat(rotr90.((img3, img2, img1))...) + s = Scene(; size=size(imgs)) + image!(s, imgs; space=:pixel) + s +end diff --git a/src/specapi.jl b/src/specapi.jl index 6120d0ad08e..2f5e1a6e08d 100644 --- a/src/specapi.jl +++ b/src/specapi.jl @@ -190,6 +190,7 @@ function Base.getproperty(p::BlockSpec, k::Symbol) end Base.propertynames(p::BlockSpec) = Tuple(keys(p.kwargs)) + function BlockSpec(typ::Symbol, args...; plots::Vector{PlotSpec}=PlotSpec[], kw...) attr = Dict{Symbol,Any}(kw) if typ == :Legend @@ -201,8 +202,16 @@ function BlockSpec(typ::Symbol, args...; plots::Vector{PlotSpec}=PlotSpec[], kw. attr[:entrygroups] = entrygroups return BlockSpec(typ, attr, plots) else + if typ == :Colorbar && !isempty(args) + if length(args) == 1 && args[1] isa PlotSpec + attr[:plotspec] = args[1] + args = () + else + error("Only one argument `arg::PlotSpec` is supported for S.Colorbar. Found: $(args)") + end + end if !isempty(args) - error("BlockSpecs, with an exception for Legend, don't support positional arguments yet.") + error("BlockSpecs, with an exception for Legend and Colorbar, don't support positional arguments yet.") end return BlockSpec(typ, attr, plots) end @@ -395,7 +404,6 @@ function Base.getproperty(::_SpecApi, field::Symbol) end end - function update_plot!(obs_to_notify, plot::AbstractPlot, oldspec::PlotSpec, spec::PlotSpec) # Update args in plot `input_args` list for i in eachindex(spec.args) @@ -423,6 +431,22 @@ function update_plot!(obs_to_notify, plot::AbstractPlot, oldspec::PlotSpec, spec push!(obs_to_notify, old_attr) end end + + reset_to_default = setdiff(keys(oldspec.kwargs), keys(spec.kwargs)) + filter!(x -> x != :cycle, reset_to_default) # dont reset cycle + if !isempty(reset_to_default) + for k in reset_to_default + old_attr = plot[k] + new_value = MakieCore.lookup_default(typeof(plot), parent_scene(plot), k) + # In case of e.g. dim_conversions + isnothing(new_value) && continue + # only update if different + if is_different(old_attr[], new_value) + old_attr.val = new_value + push!(obs_to_notify, old_attr) + end + end + end # Cycling needs to be handled separately sadly, # since they're implicitely mutating attributes, e.g. if I re-use a plot # that has been on cycling position 2, and now I re-use it for the first plot in the list @@ -437,7 +461,6 @@ function update_plot!(obs_to_notify, plot::AbstractPlot, oldspec::PlotSpec, spec end end end - if !isempty(uncycled) # remove all attributes that don't need cycling for (attr_vec, _) in cycle.cycle @@ -450,6 +473,7 @@ function update_plot!(obs_to_notify, plot::AbstractPlot, oldspec::PlotSpec, spec return end + """ plotlist!( [ @@ -645,10 +669,64 @@ function add_observer!(block::BlockSpec, obs::AbstractVector{<:ObserverFunction} return end +function get_numeric_colors(plot::PlotSpec) + if plot.type in [:Heatmap, :Image, :Surface] + z = plot.args[end] + if z isa AbstractMatrix{<:Real} + return z + end + else + if haskey(plot.kwargs, :color) && plot.kwargs[:color] isa AbstractArray{<:Real} + return plot.kwargs[:color] + end + end + return nothing +end + +# TODO it's really hard to get from PlotSpec -> Plot object in the +# Colorbar constructor (to_layoutable), +# since the plot may not be created yet and may change when calling +# update_layoutable!. So for now, we manually extract the Colorbar arguments from the spec +# Which is a bit brittle and won't work for Recipes which overload the Colorbar api (extract_colormap) +# We hope to improve the situation after the observable refactor, which may bring us a bit closer to +# Being able to use the Plot object itself instead of a spec. +function extract_colorbar_kw(legend::BlockSpec, scene::Scene) + if haskey(legend.kwargs, :plotspec) + kw = copy(legend.kwargs) + spec = pop!(kw, :plotspec) + pt = plottype(spec) + for k in [:colorrange, :colormap, :lowclip, :highclip] + get!(kw, k) do + haskey(spec.kwargs, k) && return spec.kwargs[k] + if k === :colorrange + color = get_numeric_colors(spec) + if !isnothing(color) + return nan_extrema(color) + end + else + MakieCore.lookup_default(pt, scene, k) + end + end + end + return kw + else + return legend.kwargs + end +end + function to_layoutable(parent, position::GridLayoutPosition, spec::BlockSpec) BType = getfield(Makie, spec.type) - # TODO forward kw - block = BType(get_top_parent(parent); spec.kwargs...) + fig = get_top_parent(parent) + + block = if spec.type === :Colorbar + # We use the root scene to extract any theming + # This means, we dont support a separate theme per scene + # Which I think has been bitrotting anyways. + kw = extract_colorbar_kw(spec, root(get_scene(fig))) + BType(fig; kw...) + else + BType(fig; spec.kwargs...) + end parent[position...] = block for func in spec.then_funcs observers = func(block) @@ -675,8 +753,17 @@ end function update_layoutable!(block::T, plot_obs, old_spec::BlockSpec, spec::BlockSpec) where T <: Block unhide!(block) - old_attr = keys(old_spec.kwargs) - new_attr = keys(spec.kwargs) + if spec.type === :Colorbar + # To get plot defaults for Colorbar(specapi), we need a theme / scene + # So we have to look up the kwargs here instead of the BlockSpec constructor. + old_kw = extract_colorbar_kw(old_spec, root(block.blockscene)) + new_kw = extract_colorbar_kw(spec, root(block.blockscene)) + else + old_kw = old_spec.kwargs + new_kw = spec.kwargs + end + old_attr = keys(old_kw) + new_attr = keys(new_kw) # attributes that have been set previously and need to get unset now reset_to_defaults = setdiff(old_attr, new_attr) if !isempty(reset_to_defaults) @@ -688,7 +775,7 @@ function update_layoutable!(block::T, plot_obs, old_spec::BlockSpec, spec::Block # Attributes needing an update to_update = setdiff(new_attr, reset_to_defaults) for key in to_update - val = spec.kwargs[key] + val = new_kw[key] prev_val = to_value(getproperty(block, key)) if is_different(val, prev_val) setproperty!(block, key, val) @@ -816,7 +903,6 @@ get_layout!(fig::Figure) = fig.layout get_layout!(gp::Union{GridSubposition,GridPosition}) = GridLayoutBase.get_layout_at!(gp; createmissing=true) - delete_layoutable!(block::Block) = delete!(block) function delete_layoutable!(grid::GridLayout) gc = grid.layoutobservables.gridcontent[] @@ -836,7 +922,9 @@ function update_gridlayout!(target_layout::GridLayout, layout_spec::GridLayoutSp update_gridlayout!(target_layout, 1, nothing, layout_spec, unused_layoutables, new_layoutables) foreach(unused_layoutables) do (p, (block, obs)) # disconnect! all unused layoutables, so they dont show up anymore - disconnect!(block) + if block isa Block + disconnect!(block) + end return end layouts_to_update = Set{GridLayout}([target_layout]) From 379bb69bc7d1acd77fbfbfd1ebab2ded40eacd6f Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Thu, 31 Oct 2024 09:18:53 -0700 Subject: [PATCH 04/17] Fix legend for plotlist with multiple plots (#4546) * Fix `legend` for plotlist with multiple plots * Add a test * Update CHANGELOG.md --------- Co-authored-by: SimonDanisch --- CHANGELOG.md | 1 + src/makielayout/blocks/legend.jl | 8 +++++++- test/specapi.jl | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e70668c2f1..05384f15cd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] - Added `subsup` and `left_subsup` functions that offer stacked sub- and superscripts for `rich` text which means this style can be used with arbitrary fonts and is not limited to fonts supported by MathTeXEngine.jl [#4489](https://github.com/MakieOrg/Makie.jl/pull/4489). +- Expand PlotList plots to expose their child plots to the legend interface, allowing `axislegend`show plots within PlotSpecs as individual entries. [#4546](https://github.com/MakieOrg/Makie.jl/pull/4546) - Implement S.Colorbar(plotspec) [#4520](https://github.com/MakieOrg/Makie.jl/pull/4520). ## [0.21.15] - 2024-10-25 diff --git a/src/makielayout/blocks/legend.jl b/src/makielayout/blocks/legend.jl index 8344667b28e..90a3168dc39 100644 --- a/src/makielayout/blocks/legend.jl +++ b/src/makielayout/blocks/legend.jl @@ -661,7 +661,8 @@ end function get_labeled_plots(ax; merge::Bool, unique::Bool) lplots = filter(get_plots(ax)) do plot - haskey(plot.attributes, :label) + haskey(plot.attributes, :label) || + plot isa PlotList && any(x -> haskey(x.attributes, :label), plot.plots) end labels = map(lplots) do l l.label[] @@ -713,6 +714,11 @@ function get_labeled_plots(ax; merge::Bool, unique::Bool) end get_plots(p::AbstractPlot) = [p] +# NOTE: this is important, since we know that `get_plots` is only ever called on the toplevel, +# we can assume that any plotlist on the toplevel should be decomposed into individual plots. +# However, if the user passes a label argument with a legend override, what do we do? +get_plots(p::PlotList) = haskey(p.attributes, :label) && p.attributes[:label] isa Pair ? [p] : p.plots + get_plots(ax::Union{Axis, Axis3}) = get_plots(ax.scene) get_plots(lscene::LScene) = get_plots(lscene.scene) function get_plots(scene::Scene) diff --git a/test/specapi.jl b/test/specapi.jl index d948768d821..cea645640aa 100644 --- a/test/specapi.jl +++ b/test/specapi.jl @@ -170,3 +170,20 @@ end @test isempty(f.content) @test isempty(f.layout.content) end + +@testset "Legend construction" begin + f, ax, pl = plotlist([S.Scatter(1:4, 1:4; marker = :circle, label="A"), S.Scatter(1:6, 1:6; marker = :rect, label="B")]) + leg = axislegend(ax) + # Test that the legend has two scatter plots + @test count(x -> x isa Makie.Scatter, leg.scene.plots) == 2 + + # Test that the scatter plots have the correct markers + # This is too internal and fragile, so we won't actually test this + # @test leg.scene.plots[2].marker[] == :circle + # @test leg.scene.plots[3].marker[] == :rect + + # Test that the legend has the correct labels. + # Again, I consider this too fragile to work with! + # @test contents(contents(leg.grid)[1])[2].text[] == "A" + # @test contents(contents(leg.grid)[2])[4].text[] == "B" +end From 8b914d05ebd42bdfc8cd10f9a00397091aac2de6 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com> Date: Thu, 31 Oct 2024 18:42:28 +0100 Subject: [PATCH 05/17] Median ratio bootstrap plot for benchmarks (#4553) * add violin plots of bootstrapped median ratios * 1000 samples should be enough * remove manually set ticks * add colored background to visualize good vs bad change in 5% bands --- metrics/ttfp/Project.toml | 1 + metrics/ttfp/run-benchmark.jl | 55 +++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/metrics/ttfp/Project.toml b/metrics/ttfp/Project.toml index ef310db892d..a77f1a9977f 100644 --- a/metrics/ttfp/Project.toml +++ b/metrics/ttfp/Project.toml @@ -1,5 +1,6 @@ [deps] AlgebraOfGraphics = "cbdf2221-f076-402e-a563-3d30da359d67" +Bootstrap = "e28b5b4c-05e8-5b66-bc03-6f0c0a0a06e0" CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0" DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0" JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6" diff --git a/metrics/ttfp/run-benchmark.jl b/metrics/ttfp/run-benchmark.jl index 7384031413b..e1f916ece07 100644 --- a/metrics/ttfp/run-benchmark.jl +++ b/metrics/ttfp/run-benchmark.jl @@ -10,7 +10,8 @@ Pkg.instantiate() pkg"registry up" Pkg.update() -using JSON, AlgebraOfGraphics, CairoMakie, DataFrames +using JSON, AlgebraOfGraphics, CairoMakie, DataFrames, Bootstrap +using Statistics: median Package = ARGS[1] n_samples = length(ARGS) > 1 ? parse(Int, ARGS[2]) : 7 base_branch = length(ARGS) > 2 ? ARGS[3] : "master" @@ -19,7 +20,7 @@ base_branch = length(ARGS) > 2 ? ARGS[3] : "master" # n_samples = 2 # base_branch = "breaking-release" -@info("Benchmarking $(Package) against $(base_branch) with $(n_samples)") +@info("Benchmarking $(Package) against $(base_branch) with $(n_samples) samples") function run_benchmarks(projects; n=n_samples) @@ -64,24 +65,26 @@ Pkg.add(pkgs) @time Pkg.precompile() projects = [project1, project2] +projnames = map(basename, [project1, project2]) run_benchmarks(projects) -json_files = map([project1, project2]) do p - "$(basename(p))-benchmark.json" +json_files = map(projnames) do pname + "$(pname)-benchmark.json" end colnames = ["using", "first create", "first display", "create", "display"] -df = reduce(vcat, map(json_files) do filename - name = replace(filename, r"-benchmark.*" => "") +df = reduce(vcat, map(json_files, projnames) do filename, pname arrs = map(x -> map(identity, x), JSON.parsefile(filename)) df = DataFrame(colnames .=> arrs) - df.name .= name + df.name .= pname df end) -plt = AlgebraOfGraphics.data(df) * +## + +fgrid = AlgebraOfGraphics.data(df) * mapping(:name, colnames .=> (x -> x / 1e9) .=> "time (s)", color = :name, layout = dims(1) => renamer(colnames)) * visual(RainClouds, orientation = :horizontal, markersize = 5, show_median = false, plot_boxplots = false) |> draw( @@ -91,5 +94,39 @@ plt = AlgebraOfGraphics.data(df) * figure = (; title = "$Package Benchmarks") ) +df_current_pr = df[df.name .== projnames[1], :] +df_base_branch = df[df.name .== projnames[2], :] + +medians_df = map(names(df_current_pr, Not(:name))) do colname + col_base = df_base_branch[!, colname] + col_pr = df_current_pr[!, colname] + medians_base = bootstrap(median, col_base, Bootstrap.BasicSampling(1000)) + medians_pr = bootstrap(median, col_pr, Bootstrap.BasicSampling(1000)) + ratios = Bootstrap.straps(medians_pr)[1] ./ Bootstrap.straps(medians_base)[1] + colname => ratios +end |> DataFrame + +specmedians = AlgebraOfGraphics.data(stack(medians_df)) * + mapping(:variable => presorted => "", :value => "Ratios of medians\n$(projnames[1]) / $(projnames[2])") * visual(Violin, show_median = true) + +background_bands = AlgebraOfGraphics.pregrouped([0.75:0.05:1.20], [0.8:0.05:1.25]) * + AlgebraOfGraphics.visual(HSpan, color = range(-1, 1, length = 10), colormap = [:green, :white, :tomato], alpha = 0.5) + +zeroline = AlgebraOfGraphics.pregrouped([1]) * AlgebraOfGraphics.visual(HLines, color = :gray60) + +spec = background_bands + zeroline + specmedians + +AlgebraOfGraphics.draw!(fgrid.figure[2, 3], spec, axis = (; + yaxisposition = :right, + xticklabelrotation = pi/4, + title = "Bootstrapped median ratios", + yautolimitmargin = (0, 0), + yticks = WilkinsonTicks(7, k_min = 5), +)) + +resize_to_layout!(fgrid.figure) + +## + mkpath("benchmark_results") -save(joinpath("benchmark_results", "$Package.svg"), plt) +save(joinpath("benchmark_results", "$Package.svg"), fgrid) From f5528a663789349d1ea56c75cd7e7153f8638566 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com> Date: Thu, 31 Oct 2024 20:40:30 +0100 Subject: [PATCH 06/17] Use ABBA pattern for benchmarking (#4555) --- metrics/ttfp/run-benchmark.jl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/metrics/ttfp/run-benchmark.jl b/metrics/ttfp/run-benchmark.jl index e1f916ece07..c36efce30d1 100644 --- a/metrics/ttfp/run-benchmark.jl +++ b/metrics/ttfp/run-benchmark.jl @@ -25,8 +25,16 @@ base_branch = length(ARGS) > 2 ? ARGS[3] : "master" function run_benchmarks(projects; n=n_samples) benchmark_file = joinpath(@__DIR__, "benchmark-ttfp.jl") - # go A, B, A, B, A, etc. - for project in repeat(projects, n) + # go A, A, B, B, A, A, B, B, etc. because if A or B have some effect on their + # subsequent run, then we distribute those more evenly. If we used A, B, A, B then + # B would always influence A and A always B which might bias the results (something + # that can carry over separate processes like thermal throttling or so) + + A, B = projects + As = Iterators.partition(fill(A, n), 2) + Bs = Iterators.partition(fill(B, n), 2) + + for project in Iterators.flatten(Iterators.flatten(zip(As, Bs))) println(basename(project)) run(`$(Base.julia_cmd()) --startup-file=no --project=$(project) $benchmark_file $Package`) end From 9d5501073def1806af0439a2d18f30b6643310fe Mon Sep 17 00:00:00 2001 From: t-bltg Date: Fri, 1 Nov 2024 13:07:59 +0100 Subject: [PATCH 07/17] correct spelling (#4522) * correct spelling * check if single quote causes timeouts Seems like the only change in the WGLMakie source to me that could possibly have an effect on anything, if interpolation of that string into javascript has some escaping bugs. --------- Co-authored-by: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com> --- CHANGELOG.md | 8 ++++---- CairoMakie/src/infrastructure.jl | 2 +- CairoMakie/src/primitives.jl | 4 ++-- CairoMakie/test/svg_tests.jl | 2 +- GLMakie/src/GLAbstraction/AbstractGPUArray.jl | 2 +- GLMakie/src/GLAbstraction/GLAbstraction.jl | 2 +- GLMakie/src/GLAbstraction/GLRender.jl | 2 +- GLMakie/src/GLAbstraction/GLRenderObject.jl | 2 +- GLMakie/src/GLAbstraction/GLTexture.jl | 4 ++-- GLMakie/src/GLAbstraction/GLTypes.jl | 2 +- GLMakie/src/GLAbstraction/GLUtils.jl | 6 +++--- GLMakie/src/glshaders/lines.jl | 8 ++++---- GLMakie/src/screen.jl | 4 ++-- GLMakie/test/glmakie_refimages.jl | 2 +- GLMakie/test/unit_tests.jl | 2 +- MakieCore/src/conversion.jl | 2 +- RPRMakie/src/meshes.jl | 2 +- ReferenceTests/src/tests/categorical.jl | 4 ++-- ReferenceTests/src/tests/primitives.jl | 2 +- ReferenceUpdater/src/local_server.jl | 2 +- ReferenceUpdater/src/reference_images.html | 2 +- WGLMakie/src/display.jl | 8 ++++---- WGLMakie/src/lines.jl | 16 +++++++-------- WGLMakie/src/serialization.jl | 2 +- WGLMakie/src/voxel.jl | 2 +- WGLMakie/test/runtests.jl | 2 +- docs/src/explanations/backends/glmakie.md | 4 ++-- docs/src/explanations/backends/rprmakie.md | 2 +- docs/src/explanations/backends/wglmakie.md | 2 +- docs/src/reference/blocks/colorbar.md | 2 +- docs/src/reference/blocks/intervalslider.md | 2 +- docs/src/reference/blocks/polaraxis.md | 2 +- docs/src/reference/blocks/slider.md | 2 +- docs/src/reference/plots/datashader.md | 2 +- docs/src/reference/plots/heatmap.md | 2 +- docs/src/reference/plots/rainclouds.md | 2 +- docs/src/tutorials/scenes.md | 2 +- src/Makie.jl | 2 +- src/basic_recipes/barplot.jl | 2 +- src/basic_recipes/datashader.jl | 12 +++++------ src/basic_recipes/raincloud.jl | 2 +- src/basic_recipes/text.jl | 2 +- src/basic_recipes/timeseries.jl | 2 +- src/bezier.jl | 2 +- src/configuration.yaml | 2 +- src/conversions.jl | 10 +++++----- src/dim-converts/categorical-integration.jl | 2 +- src/dim-converts/dates-integration.jl | 2 +- src/dim-converts/dim-converts.jl | 2 +- src/display.jl | 6 +++--- src/documentation/documentation.jl | 4 ++-- src/ffmpeg-util.jl | 2 +- src/float32-scaling.jl | 6 +++--- src/interaction/inspector.jl | 4 ++-- src/jl_rasterizer/bmp.jl | 4 ++-- src/makielayout/blocks/colorbar.jl | 20 +++++++++---------- src/makielayout/blocks/polaraxis.jl | 2 +- src/specapi.jl | 4 ++-- src/stats/violin.jl | 2 +- src/theming.jl | 6 +++--- src/types.jl | 2 +- src/utilities/utilities.jl | 4 ++-- test/Plane.jl | 2 +- test/PolarAxis.jl | 2 +- test/convert_arguments.jl | 2 +- test/events.jl | 2 +- test/float32convert.jl | 2 +- test/ray_casting.jl | 2 +- test/scenes.jl | 4 ++-- 69 files changed, 122 insertions(+), 122 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05384f15cd0..a3e4de0309d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -169,7 +169,7 @@ - `boundingbox` overwrites must now include a secondary space argument to work `boundingbox(plot, space::Symbol = :data)` [#3723](https://github.com/MakieOrg/Makie.jl/pull/3723) - `boundingbox` now always consider `transform_func` and `model` - `data_limits(::Scatter)` and `boundingbox(::Scatter)` now consider marker transformations [#3716](https://github.com/MakieOrg/Makie.jl/pull/3716) -- **Breaking** Improved Float64 compatability of Axis [#3681](https://github.com/MakieOrg/Makie.jl/pull/3681) +- **Breaking** Improved Float64 compatibility of Axis [#3681](https://github.com/MakieOrg/Makie.jl/pull/3681) - This added an extra conversion step which only takes effect when Float32 precision becomes relevant. In those cases code using `project()` functions will be wrong as the transformation is not applied. Use `project(plot_or_scene, ...)` or apply the conversion yourself beforehand with `Makie.f32_convert(plot_or_scene, transformed_point)` and use `patched_model = Makie.patch_model(plot_or_scene, model)`. - `Makie.to_world(point, matrix, resolution)` has been deprecated in favor of `Makie.to_world(scene_or_plot, point)` to include float32 conversions. - **Breaking** Reworked line shaders in GLMakie and WGLMakie [#3558](https://github.com/MakieOrg/Makie.jl/pull/3558) @@ -230,7 +230,7 @@ ## [0.20.7] - 2024-02-04 - Equalized alignment point of mirrored ticks to that of normal ticks [#3598](https://github.com/MakieOrg/Makie.jl/pull/3598). -- Fixed stack overflow error on conversion of gridlike data with missings [#3597](https://github.com/MakieOrg/Makie.jl/pull/3597). +- Fixed stack overflow error on conversion of gridlike data with `missing`s [#3597](https://github.com/MakieOrg/Makie.jl/pull/3597). - Fixed mutation of CairoMakie src dir when displaying png files [#3588](https://github.com/MakieOrg/Makie.jl/pull/3588). - Added better error messages for plotting into `FigureAxisPlot` and `AxisPlot` as Plots.jl users are likely to do [#3596](https://github.com/MakieOrg/Makie.jl/pull/3596). - Added compat bounds for IntervalArithmetic.jl due to bug with DelaunayTriangulation.jl [#3595](https://github.com/MakieOrg/Makie.jl/pull/3595). @@ -246,7 +246,7 @@ - Use plot plot instead of scene transform functions in CairoMakie, fixing missplaced h/vspan. [#3552](https://github.com/MakieOrg/Makie.jl/pull/3552) - Fix error printing on shader error [#3530](https://github.com/MakieOrg/Makie.jl/pull/3530). - Update pagefind to 1.0.4 for better headline search [#3534](https://github.com/MakieOrg/Makie.jl/pull/3534). -- Remove unecessary deps, e.g. Setfield [3546](https://github.com/MakieOrg/Makie.jl/pull/3546). +- Remove unnecessary deps, e.g. Setfield [3546](https://github.com/MakieOrg/Makie.jl/pull/3546). - Don't clear args, rely on delete deregister_callbacks [#3543](https://github.com/MakieOrg/Makie.jl/pull/3543). - Add interpolate keyword for Surface [#3541](https://github.com/MakieOrg/Makie.jl/pull/3541). - Fix a DataInspector bug if inspector_label is used with RGB images [#3468](https://github.com/MakieOrg/Makie.jl/pull/3468). @@ -591,7 +591,7 @@ role as `datalimits` in `violin` [#2137](https://github.com/MakieOrg/Makie.jl/pu - **Breaking** Cleaned up `Scene` type [#1192](https://github.com/MakieOrg/Makie.jl/pull/1192), [#1393](https://github.com/MakieOrg/Makie.jl/pull/1393). The `Scene()` constructor doesn't create any axes or limits anymore. All keywords like `raw`, `show_axis` have been removed. A scene now always works like it did when using the deprecated `raw=true`. All the high level functionality like showing an axis and adding a 3d camera has been moved to `LScene`. See the new `Scene` tutorial for more info: https://docs.makie.org/dev/tutorials/scenes/. - **Breaking** Lights got moved to `Scene`, see the [lighting docs](https://docs.makie.org/stable/documentation/lighting) and [RPRMakie examples](https://docs.makie.org/stable/documentation/backends/rprmakie/). - Added ECDF plot [#1310](https://github.com/MakieOrg/Makie.jl/pull/1310). -- Added Order Independent Transparency to GLMakie [#1418](https://github.com/MakieOrg/Makie.jl/pull/1418), [#1506](https://github.com/MakieOrg/Makie.jl/pull/1506). This type of transparency is now used with `transpareny = true`. The old transparency handling is available with `transparency = false`. +- Added Order Independent Transparency to GLMakie [#1418](https://github.com/MakieOrg/Makie.jl/pull/1418), [#1506](https://github.com/MakieOrg/Makie.jl/pull/1506). This type of transparency is now used with `transparency = true`. The old transparency handling is available with `transparency = false`. - Fixed blurry text in GLMakie and WGLMakie [#1494](https://github.com/MakieOrg/Makie.jl/pull/1494). - Introduced a new experimental backend for ray tracing: [RPRMakie](https://docs.makie.org/stable/documentation/backends/rprmakie/). - Added the `Cycled` type, which can be used to select the i-th value from the current cycler for a specific attribute [#1248](https://github.com/MakieOrg/Makie.jl/pull/1248). diff --git a/CairoMakie/src/infrastructure.jl b/CairoMakie/src/infrastructure.jl index 0f70934682b..4dc5d4deba9 100644 --- a/CairoMakie/src/infrastructure.jl +++ b/CairoMakie/src/infrastructure.jl @@ -154,7 +154,7 @@ end function draw_plot_as_image(scene::Scene, screen::Screen{RT}, primitive::Plot, scale::Number = 1) where RT # you can provide `p.rasterize = scale::Int` or `p.rasterize = true`, both of which are numbers - # Extract scene width in device indepentent units + # Extract scene width in device independent units w, h = size(scene) # Create a new Screen which renders directly to an image surface, # specifically for the plot's parent scene. diff --git a/CairoMakie/src/primitives.jl b/CairoMakie/src/primitives.jl index 795359670ca..4cf37ea8b7c 100644 --- a/CairoMakie/src/primitives.jl +++ b/CairoMakie/src/primitives.jl @@ -928,7 +928,7 @@ end function draw_mesh2D(screen, per_face_cols, vs::Vector{<: Point2}, fs::Vector{GLTriangleFace}) ctx = screen.context - # Priorize colors of the mesh if present + # Prioritize colors of the mesh if present # This is a hack, which needs cleaning up in the Mesh plot type! for (f, (c1, c2, c3)) in zip(fs, per_face_cols) @@ -984,7 +984,7 @@ function draw_mesh3D( meshuvs = map(uv -> uv_transform * to_ndim(Vec3f, uv, 1), meshuvs) end - # Priorize colors of the mesh if present + # Prioritize 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) diff --git a/CairoMakie/test/svg_tests.jl b/CairoMakie/test/svg_tests.jl index da30bba0948..8d7f4239437 100644 --- a/CairoMakie/test/svg_tests.jl +++ b/CairoMakie/test/svg_tests.jl @@ -61,7 +61,7 @@ end @test svg_isnt_rasterized(poly(MultiPolyWrapper([poly1, poly1]); color=[:red, :blue])) end -@testset "reproducable svg ids" begin +@testset "reproducible svg ids" begin # https://github.com/MakieOrg/Makie.jl/issues/2406 f, ax, sc = scatter(1:10) save("test1.svg", f) diff --git a/GLMakie/src/GLAbstraction/AbstractGPUArray.jl b/GLMakie/src/GLAbstraction/AbstractGPUArray.jl index 2241931b7f4..edfba611746 100644 --- a/GLMakie/src/GLAbstraction/AbstractGPUArray.jl +++ b/GLMakie/src/GLAbstraction/AbstractGPUArray.jl @@ -192,7 +192,7 @@ max_dim(t) = error("max_dim not implemented for: $(typeof(t)). This happen function (::Type{GPUArrayType})(data::Observable; kw...) where GPUArrayType <: GPUArray gpu_mem = GPUArrayType(data[]; kw...) - # TODO merge these and handle update tracking during contruction + # TODO merge these and handle update tracking during construction obs2 = on(new_data -> update!(gpu_mem, new_data), data) if GPUArrayType <: TextureBuffer push!(gpu_mem.buffer.observers, obs2) diff --git a/GLMakie/src/GLAbstraction/GLAbstraction.jl b/GLMakie/src/GLAbstraction/GLAbstraction.jl index e8c36de91c7..67c6843ed10 100644 --- a/GLMakie/src/GLAbstraction/GLAbstraction.jl +++ b/GLMakie/src/GLAbstraction/GLAbstraction.jl @@ -53,7 +53,7 @@ export update! # updates a gpu array with a Julia array export gpu_data # gets the data of a gpu array as a Julia Array export RenderObject # An object which holds all GPU handles and datastructes to ready for rendering by calling render(obj) -export prerender! # adds a function to a RenderObject, which gets executed befor setting the OpenGL render state +export prerender! # adds a function to a RenderObject, which gets executed before setting the OpenGL render state export postrender! # adds a function to a RenderObject, which gets executed after setting the OpenGL render states export extract_renderable export set_arg! diff --git a/GLMakie/src/GLAbstraction/GLRender.jl b/GLMakie/src/GLAbstraction/GLRender.jl index 146eeb6dd44..357ce23781b 100644 --- a/GLMakie/src/GLAbstraction/GLRender.jl +++ b/GLMakie/src/GLAbstraction/GLRender.jl @@ -160,7 +160,7 @@ function renderinstanced(vao::GLVertexArray{GLBuffer{T}}, amount::Integer, primi end """ -Renders `amount` instances of an not indexed geoemtry geometry +Renders `amount` instances of an not indexed geometry geometry """ function renderinstanced(vao::GLVertexArray, amount::Integer, primitive=GL_TRIANGLES) glDrawElementsInstanced(primitive, length(vao), GL_UNSIGNED_INT, C_NULL, amount) diff --git a/GLMakie/src/GLAbstraction/GLRenderObject.jl b/GLMakie/src/GLAbstraction/GLRenderObject.jl index fe644f5c586..c47b2479849 100644 --- a/GLMakie/src/GLAbstraction/GLRenderObject.jl +++ b/GLMakie/src/GLAbstraction/GLRenderObject.jl @@ -30,7 +30,7 @@ function (sp::StandardPrerender)() glDepthFunc(GL_LEQUAL) end - # Disable cullface for now, untill all rendering code is corrected! + # Disable cullface for now, until all rendering code is corrected! glDisable(GL_CULL_FACE) # glCullFace(GL_BACK) diff --git a/GLMakie/src/GLAbstraction/GLTexture.jl b/GLMakie/src/GLAbstraction/GLTexture.jl index 39d3cf3bb60..e0334e490f7 100644 --- a/GLMakie/src/GLAbstraction/GLTexture.jl +++ b/GLMakie/src/GLAbstraction/GLTexture.jl @@ -430,7 +430,7 @@ default_colorformat_sym(::Type{T}) where {T <: Colorant} = default_colorformat_s @generated function default_colorformat(::Type{T}) where T sym = default_colorformat_sym(T) if !isdefined(ModernGL, sym) - error("$T doesn't have a propper mapping to an OpenGL format") + error("$T doesn't have a proper mapping to an OpenGL format") end :($sym) end @@ -462,7 +462,7 @@ end @generated function default_internalcolorformat(::Type{T}) where T sym = default_internalcolorformat_sym(T) if !isdefined(ModernGL, sym) - error("$T doesn't have a propper mapping to an OpenGL format") + error("$T doesn't have a proper mapping to an OpenGL format") end :($sym) end diff --git a/GLMakie/src/GLAbstraction/GLTypes.jl b/GLMakie/src/GLAbstraction/GLTypes.jl index b9ce99f0167..b580152fcb9 100644 --- a/GLMakie/src/GLAbstraction/GLTypes.jl +++ b/GLMakie/src/GLAbstraction/GLTypes.jl @@ -401,7 +401,7 @@ function RenderObject( program = gl_convert(to_value(program), data) # "compile" lazyshader vertexarray = GLVertexArray(Dict(buffers), program) - # remove all uniforms not occuring in shader + # remove all uniforms not occurring in shader # ssao, instances transparency are special for rendering passes. TODO do this more cleanly special = Set([:ssao, :transparency, :instances, :fxaa, :num_clip_planes]) for k in setdiff(keys(data), keys(program.nametype)) diff --git a/GLMakie/src/GLAbstraction/GLUtils.jl b/GLMakie/src/GLAbstraction/GLUtils.jl index 4176bdc97b9..5fe35eda5c7 100644 --- a/GLMakie/src/GLAbstraction/GLUtils.jl +++ b/GLMakie/src/GLAbstraction/GLUtils.jl @@ -28,7 +28,7 @@ gen_defaults! dict begin a = 55 b = a * 2 # variables, like a, will get made visible in local scope c::JuliaType = X # `c` needs to be of type JuliaType. `c` will be made available with it's original type and then converted to JuliaType when inserted into `dict` - d = x => GLType # OpenGL convert target. Get's only applied if `x` is convertible to GLType. Will only be converted when passed to RenderObject + d = x => GLType # OpenGL convert target. Gets only applied if `x` is convertible to GLType. Will only be converted when passed to RenderObject d = x => \"doc string\" d = x => (GLType, \"doc string and gl target\") end @@ -39,12 +39,12 @@ macro gen_defaults!(dict, args) a = 55 b = a * 2 # variables, like a, will get made visible in local scope c::JuliaType = X # c needs to be of type JuliaType. c will be made available with it's original type and then converted to JuliaType when inserted into data - d = x => GLType # OpenGL convert target. Get's only applied if x is convertible to GLType. Will only be converted when passed to RenderObject + d = x => GLType # OpenGL convert target. Gets only applied if x is convertible to GLType. Will only be converted when passed to RenderObject end") tuple_list = args.args dictsym = gensym() return_expression = Expr(:block) - push!(return_expression.args, :($dictsym = $dict)) # dict could also be an expression, so we need to asign it to a variable at the beginning + push!(return_expression.args, :($dictsym = $dict)) # dict could also be an expression, so we need to assign it to a variable at the beginning push!(return_expression.args, :(gl_convert_targets = get!($dictsym, :gl_convert_targets, Dict{Symbol, Any}()))) # exceptions for glconvert. push!(return_expression.args, :(doc_strings = get!($dictsym, :doc_string, Dict{Symbol, Any}()))) # exceptions for glconvert. # @gen_defaults can be used multiple times, so we need to reuse gl_convert_targets if already in here diff --git a/GLMakie/src/glshaders/lines.jl b/GLMakie/src/glshaders/lines.jl index c4fa81d8cf8..9f29d47d8b7 100644 --- a/GLMakie/src/glshaders/lines.jl +++ b/GLMakie/src/glshaders/lines.jl @@ -1,5 +1,5 @@ function sumlengths(points, resolution) - # normalize w component if availabke + # normalize w component if available f(p::VecTypes{4}) = p[Vec(1, 2)] / p[4] f(p::VecTypes) = p[Vec(1, 2)] @@ -45,10 +45,10 @@ function generate_indices(positions) # if A != F (no loop): 0 A B C D E F 0 # where 0 is NaN # It marks vertices as invalid (0) if they are NaN, valid (1) if they - # are part of a continous line section, or as ghost edges (2) used to + # are part of a continuous line section, or as ghost edges (2) used to # cleanly close a loop. The shader detects successive vertices with # 1-2-0 and 0-2-1 validity to avoid drawing ghost segments (E-A from - # 0-E-A-B and F-B from E-F-B-0 which would dublicate E-F and A-B) + # 0-E-A-B and F-B from E-F-B-0 which would duplicate E-F and A-B) last_start_pos = eltype(ps)(NaN) last_start_idx = -1 @@ -60,7 +60,7 @@ function generate_indices(positions) if not_nan if last_start_idx == -1 # place nan before section of line vertices - # (or dublicate ps[1]) + # (or duplicate ps[1]) push!(indices, i-1) last_start_idx = length(indices) + 1 last_start_pos = p diff --git a/GLMakie/src/screen.jl b/GLMakie/src/screen.jl index 6188b400bd3..31bd584a089 100644 --- a/GLMakie/src/screen.jl +++ b/GLMakie/src/screen.jl @@ -571,7 +571,7 @@ function destroy!(rob::RenderObject) # but we do share the texture atlas, so we check v !== tex, since we can't just free shared resources # TODO, refcounting, or leaving freeing to GC... - # GC is a bit tricky with active contexts, so immediate free is prefered. + # GC is a bit tricky with active contexts, so immediate free is preferred. # I guess as long as we make it hard for users to share buffers directly, this should be fine! GLAbstraction.free(v) end @@ -978,7 +978,7 @@ function renderloop(screen) end if screen.close_after_renderloop try - @debug("Closing screen after quiting renderloop!") + @debug("Closing screen after quitting renderloop!") close(screen) catch e @warn "error closing screen" exception=(e, Base.catch_backtrace()) diff --git a/GLMakie/test/glmakie_refimages.jl b/GLMakie/test/glmakie_refimages.jl index d4e911474db..afc81dd8c17 100644 --- a/GLMakie/test/glmakie_refimages.jl +++ b/GLMakie/test/glmakie_refimages.jl @@ -10,7 +10,7 @@ using ReferenceTests.RNG # Directly access texture parameters: x = Sampler(fill(to_color(:yellow), 100, 100), minfilter=:nearest) scene = image(x) - # indexing will go straight to the GPU, while only transfering the changes + # indexing will go straight to the GPU, while only transferring the changes st = Stepper(scene) x[1:10, 1:50] .= to_color(:red) Makie.step!(st) diff --git a/GLMakie/test/unit_tests.jl b/GLMakie/test/unit_tests.jl index 6e08a85aba3..d62843af195 100644 --- a/GLMakie/test/unit_tests.jl +++ b/GLMakie/test/unit_tests.jl @@ -133,7 +133,7 @@ end end end -@testset "emtpy!(fig)" begin +@testset "empty!(fig)" begin GLMakie.closeall() fig = Figure() ax = Axis(fig[1,1]) diff --git a/MakieCore/src/conversion.jl b/MakieCore/src/conversion.jl index 16c450ffc17..09d91fd9f9b 100644 --- a/MakieCore/src/conversion.jl +++ b/MakieCore/src/conversion.jl @@ -139,7 +139,7 @@ to be overloaded for DimConversions, e.g. for CategoricalConversion: `has_typed_convert(plot_or_trait)` and `should_dim_convert(get_element_type(args))` are true. The former is defined as true by `@convert_target`, i.e. when `convert_arguments_typed` is defined for the given plot type or conversion trait. -The latter marks specific types as convertable. +The latter marks specific types as convertible. If a recipe wants to use dim conversions, it should overload this function: ```julia diff --git a/RPRMakie/src/meshes.jl b/RPRMakie/src/meshes.jl index d635d58a8f7..b77138d10f2 100644 --- a/RPRMakie/src/meshes.jl +++ b/RPRMakie/src/meshes.jl @@ -167,7 +167,7 @@ function to_rpr_object(context, matsys, scene, plot::Makie.Surface) positions = lift(grid, x, y, z, Makie.transform_func_obs(plot)) r = Tesselation(Rect2f((0, 0), (1, 1)), size(z[])) # decomposing a rectangle into uv and triangles is what we need to map the z coordinates on - # since the xyz data assumes the coordinates to have the same neighouring relations + # since the xyz data assumes the coordinates to have the same neighbouring relations # like a grid faces = decompose(GLTriangleFace, r) uv = decompose_uv(r) diff --git a/ReferenceTests/src/tests/categorical.jl b/ReferenceTests/src/tests/categorical.jl index bd2bd4865cb..4740ac2a4a4 100644 --- a/ReferenceTests/src/tests/categorical.jl +++ b/ReferenceTests/src/tests/categorical.jl @@ -11,7 +11,7 @@ using Makie: Categorical end @reference_test "different types without sorting function" begin - # If we set the ticks explicitely, with sortby defaulting to nothing, + # If we set the ticks explicitly, with sortby defaulting to nothing, # we can combine all objects: f = Figure() ax = Axis(f[1, 1]; @@ -42,7 +42,7 @@ end f end -@reference_test "new categories, inbetween old values" begin +@reference_test "new categories, in between old values" begin obs = Observable(Categorical(["a", "c", "e", "g"])) f, ax, p = scatter(1:4, obs, markersize=20, color=1:4, colormap=:viridis) obs[] = Categorical(["b", "d", "f", "h"]) diff --git a/ReferenceTests/src/tests/primitives.jl b/ReferenceTests/src/tests/primitives.jl index f13448dd51d..919e74e903b 100644 --- a/ReferenceTests/src/tests/primitives.jl +++ b/ReferenceTests/src/tests/primitives.jl @@ -573,7 +573,7 @@ end campixel!(scene) # marker is in front, so it should not be smaller than the background rectangle plot_row!(scene, 0, false) - # marker is in the background, so one shouldnt see a single pixel of the marker + # marker is in the background, so one shouldn't see a single pixel of the marker plot_row!(scene, 300, true) center = Point2f(size(scene) ./ 2) diff --git a/ReferenceUpdater/src/local_server.jl b/ReferenceUpdater/src/local_server.jl index 4496911129d..668af3fd6d9 100644 --- a/ReferenceUpdater/src/local_server.jl +++ b/ReferenceUpdater/src/local_server.jl @@ -276,7 +276,7 @@ function group_files(path, input_filename, output_filename) end end - # generate new structed file + # generate new structured file open(joinpath(path, output_filename), "w") do file for (filename, valid) in data println(file, diff --git a/ReferenceUpdater/src/reference_images.html b/ReferenceUpdater/src/reference_images.html index 29f4f69e309..61af9f3e80f 100644 --- a/ReferenceUpdater/src/reference_images.html +++ b/ReferenceUpdater/src/reference_images.html @@ -57,7 +57,7 @@

Images with references

This is the normal case where the selected CI run produced an image and the reference image exists. Each row shows one image per backend from the same reference image test, which can be compared with its reference image. Rows are sorted based on the maximum row score (bigger = more different). - Red cells fail CI (assuming the thresholds are up to date), yellow cells may but likely don't have signficant visual difference and gray cells are visually equivalent. + Red cells fail CI (assuming the thresholds are up to date), yellow cells may but likely don't have significant visual difference and gray cells are visually equivalent.

diff --git a/WGLMakie/src/display.jl b/WGLMakie/src/display.jl index a6070ebeb4b..16ebcb93588 100644 --- a/WGLMakie/src/display.jl +++ b/WGLMakie/src/display.jl @@ -87,13 +87,13 @@ function render_with_init(screen::Screen, session::Session, scene::Scene) if isready(screen.plot_initialized) # plot_initialized contains already an item # This should not happen, but lets check anyways, so it errors and doesn't hang forever - error("Plot inititalized multiple times?") + error("Plot initialized multiple times?") end if initialized == true put!(screen.plot_initialized, true) mark_as_displayed!(screen, scene) else - # Will be an eror from WGLMakie.js + # Will be an error from WGLMakie.js put!(screen.plot_initialized, initialized) end return @@ -230,7 +230,7 @@ function get_screen_session(screen::Screen; timeout=100, success = Bonito.wait_for(() -> isready(screen.plot_initialized); timeout=timeout) # Throw error if error message specified if success !== :success - throw_error("Timed out waiting $(timeout)s for session to get initilize") + throw_error("Timed out waiting $(timeout)s for session to get initialize") return nothing end value = fetch(screen.plot_initialized) @@ -316,7 +316,7 @@ function insert_scene!(session::Session, screen::Screen, scene::Scene) scene_ser = serialize_scene(scene) parent = scene.parent parent_uuid = js_uuid(parent) - err = "Cant find scene js_uuid(scene) == $(parent_uuid)" + err = "Cannot find scene js_uuid(scene) == $(parent_uuid)" evaljs_value(session, js""" $(WGL).then(WGL=> { const parent = WGL.find_scene($(parent_uuid)); diff --git a/WGLMakie/src/lines.jl b/WGLMakie/src/lines.jl index 3a9864657c3..239b3cd2e37 100644 --- a/WGLMakie/src/lines.jl +++ b/WGLMakie/src/lines.jl @@ -41,7 +41,7 @@ function serialize_three(scene::Scene, plot::Union{Lines, LineSegments}) # This is mostly NaN handling. The shader only draws a segment if each # involved point are not NaN, i.e. p1 -- p2 is only drawn if all of - # (p0, p1, p2, p3) are not NaN. So if p3 is NaN we need to dublicate p2 to + # (p0, p1, p2, p3) are not NaN. So if p3 is NaN we need to duplicate p2 to # make the p1 -- p2 segment draw, which is what indices does. indices = Observable(UInt32[]) points_transformed = lift( @@ -76,12 +76,12 @@ function serialize_three(scene::Scene, plot::Union{Lines, LineSegments}) # (nan, i-2, j, i+1) the segment (i-2, j) will not # be drawn (which we want as that segment would overlap) - # tweak dublicated vertices to be loop vertices + # tweak duplicated vertices to be loop vertices push!(indices[], indices[][loop_start_idx+1]) indices[][loop_start_idx-1] = i-2 # nan is inserted at bottom (and not necessary for start/end) - else # no loop, dublicate end point + else # no loop, duplicate end point push!(indices[], i-1) end end @@ -90,7 +90,7 @@ function serialize_three(scene::Scene, plot::Union{Lines, LineSegments}) else if was_nan - # line section start - dublicate point + # line section start - duplicate point push!(indices[], i) # first point in a potential loop loop_start_idx = length(indices[])+1 @@ -102,7 +102,7 @@ function serialize_three(scene::Scene, plot::Union{Lines, LineSegments}) push!(indices[], i) end - # Finish line (insert dublicate end point or close loop) + # Finish line (insert duplicate end point or close loop) if !was_nan if loop_start_idx != -1 && (loop_start_idx + 2 < length(indices[])) && (transformed_points[indices[][loop_start_idx]] ≈ transformed_points[end]) @@ -145,9 +145,9 @@ function serialize_three(scene::Scene, plot::Union{Lines, LineSegments}) prev = scale .* Point2f(clip) ./ clip[4] # calculate cumulative pixel scale length - output[1] = 0f0 # dublicated point + output[1] = 0f0 # duplicated point output[2] = 0f0 # start of first line segment - output[end] = 0f0 # dublicated end point + output[end] = 0f0 # duplicated end point i = 3 # end of first line segment, start of second while i < length(ps) if isfinite(ps[i]) @@ -187,7 +187,7 @@ function serialize_three(scene::Scene, plot::Union{Lines, LineSegments}) uniforms[Symbol("$(name)_end")] = attr else # TODO: to js? - # dublicates per vertex attributes to match positional dublication + # duplicates per vertex attributes to match positional duplication # min(idxs, end) avoids update order issues here attributes[name] = lift(plot, indices, attr) do idxs, vals serialize_buffer_attribute(vals[min.(idxs, end)]) diff --git a/WGLMakie/src/serialization.jl b/WGLMakie/src/serialization.jl index a2942c681b3..64adb5bda0b 100644 --- a/WGLMakie/src/serialization.jl +++ b/WGLMakie/src/serialization.jl @@ -317,7 +317,7 @@ end function serialize_plots(scene::Scene, plots::Vector{Plot}, result=[]) for plot in plots - # if no plots inserted, this truely is an atomic + # if no plots inserted, this truly is an atomic if isempty(plot.plots) plot_data = serialize_three(scene, plot) plot_data[:uuid] = js_uuid(plot) diff --git a/WGLMakie/src/voxel.jl b/WGLMakie/src/voxel.jl index a9d395565db..f8ba21e5242 100644 --- a/WGLMakie/src/voxel.jl +++ b/WGLMakie/src/voxel.jl @@ -89,7 +89,7 @@ function create_shader(scene::Scene, plot::Makie.Voxels) onany(plot, plot.gap, plot.converted[end]) do gap, chunk N = sum(size(chunk)) N_instances = ifelse(gap > 0.01, 2 * N, N + 3) - if N_instances != length(dummy_data[]) # avoid updating unneccesarily + if N_instances != length(dummy_data[]) # avoid updating unnecessarily dummy_data[] = [0f0 for _ in 1:N_instances] end return diff --git a/WGLMakie/test/runtests.jl b/WGLMakie/test/runtests.jl index f625dbe893d..a54d8ebce69 100644 --- a/WGLMakie/test/runtests.jl +++ b/WGLMakie/test/runtests.jl @@ -10,7 +10,7 @@ import Electron @testset for mime in Makie.WEB_MIMES @test showable(mime(), f) end - # I guess we explicitely don't say we can show those since it's highly Inefficient compared to html + # I guess we explicitly don't say we can show those since it's highly Inefficient compared to html # See: https://github.com/MakieOrg/Makie.jl/blob/master/WGLMakie/src/display.jl#L66-L68= @test !showable("image/png", f) @test !showable("image/jpeg", f) diff --git a/docs/src/explanations/backends/glmakie.md b/docs/src/explanations/backends/glmakie.md index fd43d9bd2c6..358e0af89ff 100644 --- a/docs/src/explanations/backends/glmakie.md +++ b/docs/src/explanations/backends/glmakie.md @@ -62,7 +62,7 @@ GLMakie has experimental support for displaying multiple independent figures (or ## Embedding There's experimental support for embedding GLMakie by creating a custom 'window' -type (analagous to the GLFW OS-level window) and grabbing GLMakie's framebuffers +type (analogous to the GLFW OS-level window) and grabbing GLMakie's framebuffers to display in your own GUI. Here's a high-level overview of what you'd need to do: @@ -91,7 +91,7 @@ do: calling `GLMakie.render_frame(screen)` whenever necessary (you can use `GLMakie.requires_update(screen)`). 1. `display(screen, f)` will only draw the figure to a framebuffer. You can get - the color texture attachement of the framebuffer with + the color texture attachment of the framebuffer with `screen.framebuffer.buffers[:color]`, and display that color texture as an image in your chosen GUI framework. 1. If interactivity is desired, you will need to pass input events from the diff --git a/docs/src/explanations/backends/rprmakie.md b/docs/src/explanations/backends/rprmakie.md index e697f59db95..fa3c7efd600 100644 --- a/docs/src/explanations/backends/rprmakie.md +++ b/docs/src/explanations/backends/rprmakie.md @@ -36,7 +36,7 @@ lights = [ ] # Only LScene is supported right now, -# since the other projections don't map to the pysical acurate Camera in RPR. +# since the other projections don't map to the physical accurate Camera in RPR. ax = LScene(fig[1, 1]; show_axis = false, scenekw=(lights=lights,)) # Note that since RPRMakie doesn't yet support text (this is being worked on!), # you can't show a 3d axis yet. diff --git a/docs/src/explanations/backends/wglmakie.md b/docs/src/explanations/backends/wglmakie.md index e2cf49153a5..27a919ea7f9 100644 --- a/docs/src/explanations/backends/wglmakie.md +++ b/docs/src/explanations/backends/wglmakie.md @@ -105,7 +105,7 @@ Bonito allows to record a statemap for all widgets, that satisfy the following i ```julia # must be true to be found inside the DOM is_widget(x) = true -# Updating the widget isn't dependant on any other state (only thing supported right now) +# Updating the widget isn't dependent on any other state (only thing supported right now) is_independant(x) = true # The values a widget can iterate function value_range end diff --git a/docs/src/reference/blocks/colorbar.md b/docs/src/reference/blocks/colorbar.md index a46f4df7e8f..dec42371b99 100644 --- a/docs/src/reference/blocks/colorbar.md +++ b/docs/src/reference/blocks/colorbar.md @@ -75,7 +75,7 @@ fig ``` -We can't use `cgrad(...; categorical=true)` for this, since it has an ambigious meaning for true categorical values. +We can't use `cgrad(...; categorical=true)` for this, since it has an ambiguous meaning for true categorical values. ## Attributes diff --git a/docs/src/reference/blocks/intervalslider.md b/docs/src/reference/blocks/intervalslider.md index c6bcee7b8cb..aec38a7ae92 100644 --- a/docs/src/reference/blocks/intervalslider.md +++ b/docs/src/reference/blocks/intervalslider.md @@ -16,7 +16,7 @@ If the mouse hovers over the central area of the interval and both buttons are e You can double-click the slider to reset it to the values present in `startvalues`. If `startvalues === Makie.automatic`, the full interval will be selected (this is the default). -If you set the attribute `snap = false`, the slider will move continously while dragging and only jump to the closest available values when releasing the mouse. +If you set the attribute `snap = false`, the slider will move continuously while dragging and only jump to the closest available values when releasing the mouse. ```@example intervalslider using GLMakie diff --git a/docs/src/reference/blocks/polaraxis.md b/docs/src/reference/blocks/polaraxis.md index 35a08499b97..04f67285706 100644 --- a/docs/src/reference/blocks/polaraxis.md +++ b/docs/src/reference/blocks/polaraxis.md @@ -80,7 +80,7 @@ Note that by default translations in adjustments of rmin and thetalimits are blo These can be unblocked by calling `autolimits!(ax[, true])` which also tells the PolarAxis to derive r- and thetalimits freely from data, or by setting `ax.fixrmin[] = false` and `ax.thetazoomlock[] = false`. -## Plot type compatability +## Plot type compatibility Not every plot type is compatible with the polar transform. For example `image` is not as it expects to be drawn on a rectangle. diff --git a/docs/src/reference/blocks/slider.md b/docs/src/reference/blocks/slider.md index a05362e9927..99d4c5df7ec 100644 --- a/docs/src/reference/blocks/slider.md +++ b/docs/src/reference/blocks/slider.md @@ -11,7 +11,7 @@ This is necessary to ensure the value is actually present in the `range` attribu You can double-click the slider to reset it (approximately) to the value present in `startvalue`. -If you set the attribute `snap = false`, the slider will move continously while dragging and only jump to the closest available value when releasing the mouse. +If you set the attribute `snap = false`, the slider will move continuously while dragging and only jump to the closest available value when releasing the mouse. ```@figure backend=GLMakie diff --git a/docs/src/reference/plots/datashader.md b/docs/src/reference/plots/datashader.md index 45f0f3a45dc..f000f369a58 100644 --- a/docs/src/reference/plots/datashader.md +++ b/docs/src/reference/plots/datashader.md @@ -203,7 +203,7 @@ datashader(Dict(:category_a => all_points_a, :category_b => all_points_b)) ``` The type of the category doesn't matter, but will get converted to strings internally, to be displayed nicely in the legend. -Categories are currently aggregated in one Canvas per category, and then overlayed with alpha blending. +Categories are currently aggregated in one Canvas per category, and then overlaid with alpha blending. ```@figure backend=GLMakie normaldist = randn(Point2f, 1_000_000) diff --git a/docs/src/reference/plots/heatmap.md b/docs/src/reference/plots/heatmap.md index e2971df597a..ff21cd163b0 100644 --- a/docs/src/reference/plots/heatmap.md +++ b/docs/src/reference/plots/heatmap.md @@ -126,7 +126,7 @@ setting the colorrange explicitly, so that it is independent of the data shown b that particular heatmap. Since the heatmaps in the example below have the same colorrange and colormap, any of them -can be passed to `Colorbar` to give the colorbar the same attributes. Alternativly, +can be passed to `Colorbar` to give the colorbar the same attributes. Alternatively, the colorbar attributes can be set explicitly. ```@figure diff --git a/docs/src/reference/plots/rainclouds.md b/docs/src/reference/plots/rainclouds.md index de99693c2ed..06d571b2ce0 100644 --- a/docs/src/reference/plots/rainclouds.md +++ b/docs/src/reference/plots/rainclouds.md @@ -177,7 +177,7 @@ rainclouds!( plot_boxplots = false, color = colors[indexin(category_labels, unique(category_labels))]) -# Plots wiht more categories +# Plots with more categories # dist_between_categories (0.6, 1.0) # with and without clouds diff --git a/docs/src/tutorials/scenes.md b/docs/src/tutorials/scenes.md index 91d98e929b4..cc62e243eae 100644 --- a/docs/src/tutorials/scenes.md +++ b/docs/src/tutorials/scenes.md @@ -8,7 +8,7 @@ scene = Scene(; clear = true, # the camera struct of the scene. visible = true, - # ssao and light are explained in more detail in `Documetation/Lighting` + # ssao and light are explained in more detail in `Documentation/Lighting` ssao = Makie.SSAO(), # Creates lights from theme, which right now defaults to ` # set_theme!(lightposition=:eyeposition, ambient=RGBf(0.5, 0.5, 0.5))` diff --git a/src/Makie.jl b/src/Makie.jl index 6d6a6452068..1137e94148d 100644 --- a/src/Makie.jl +++ b/src/Makie.jl @@ -198,7 +198,7 @@ include("layouting/text_layouting.jl") include("layouting/boundingbox.jl") include("layouting/text_boundingbox.jl") -# Declaritive SpecApi +# Declarative SpecApi include("specapi.jl") # more default recipes diff --git a/src/basic_recipes/barplot.jl b/src/basic_recipes/barplot.jl index 4cf1d69f7e5..a93aa92d28a 100644 --- a/src/basic_recipes/barplot.jl +++ b/src/basic_recipes/barplot.jl @@ -21,7 +21,7 @@ function bar_default_fillto(tf, ys, offset, in_y_direction) return ys, offset end -# `fillto` is related to `y-axis` transofrmation only, thus we expect `tf::Tuple` +# `fillto` is related to `y-axis` transformation only, thus we expect `tf::Tuple` function bar_default_fillto(tf::Tuple, ys, offset, in_y_direction) _logT = Union{typeof(log), typeof(log2), typeof(log10), Base.Fix1{typeof(log), <: Real}} if in_y_direction && tf[2] isa _logT || (!in_y_direction && tf[1] isa _logT) diff --git a/src/basic_recipes/datashader.jl b/src/basic_recipes/datashader.jl index cffd2b098d6..321d1d75819 100644 --- a/src/basic_recipes/datashader.jl +++ b/src/basic_recipes/datashader.jl @@ -15,10 +15,10 @@ abstract type AggOp end using Makie canvas = Canvas(-1, 1, -1, 1; op=AggCount(), resolution=(800, 800)) aggregate!(canvas, points; point_transform=reverse, method=AggThreads()) -aggregated_values = get_aggregation(canvas; operation=equalize_histogram, local_operation=identiy) -# Recipes are defined for canvas as well and incorperate the `get_aggregation`, but `aggregate!` must be called manually. -image!(canvas; operation=equalize_histogram, local_operation=identiy, colormap=:viridis, colorrange=(0, 20)) -surface!(canvas; operation=equalize_histogram, local_operation=identiy) +aggregated_values = get_aggregation(canvas; operation=equalize_histogram, local_operation=identity) +# Recipes are defined for canvas as well and incorporate the `get_aggregation`, but `aggregate!` must be called manually. +image!(canvas; operation=equalize_histogram, local_operation=identity, colormap=:viridis, colorrange=(0, 20)) +surface!(canvas; operation=equalize_histogram, local_operation=identity) ``` """ mutable struct Canvas @@ -291,7 +291,7 @@ For best performance, use `method=Makie.AggThreads()` and make sure to start jul @recipe DataShader (points,) begin """ Can be `AggCount()`, `AggAny()` or `AggMean()`. - Be sure, to use the correct element type e.g. `AggCount{Float32}()`, which needs to accomodate the output of `local_operation`. + Be sure, to use the correct element type e.g. `AggCount{Float32}()`, which needs to accommodate the output of `local_operation`. User-extensible by overloading: ```julia struct MyAgg{T} <: Makie.AggOp end @@ -500,7 +500,7 @@ function legendelements(plot::FakePlot, legend) return [PolyElement(; color=plot.attributes.color, strokecolor=legend.polystrokecolor, strokewidth=legend.polystrokewidth)] end -# Sadly we must define the colorbar here and cant use the default fallback, +# Sadly we must define the colorbar here and can't use the default fallback, # Since the Image plot will only see the scaled data, and since its hard to make Colorbar support the equalize_histogram # transform, we just create the colorbar form the raw data. # TODO, should we merge the local/global op with colorscale? diff --git a/src/basic_recipes/raincloud.jl b/src/basic_recipes/raincloud.jl index 767d6a32264..a450ae0c750 100644 --- a/src/basic_recipes/raincloud.jl +++ b/src/basic_recipes/raincloud.jl @@ -190,7 +190,7 @@ end function ungroup_labels(category_labels, data_array) if eltype(data_array) <: AbstractVector - @warn "Using a nested array for raincloud is deprected. Read raincloud's documentation and update your usage accordingly." + @warn "Using a nested array for raincloud is deprecated. Read raincloud's documentation and update your usage accordingly." data_array_ = reduce(vcat, data_array) category_labels_ = similar(category_labels, length(data_array_)) ix = 0 diff --git a/src/basic_recipes/text.jl b/src/basic_recipes/text.jl index 2c38861e438..a033fadeaef 100644 --- a/src/basic_recipes/text.jl +++ b/src/basic_recipes/text.jl @@ -189,7 +189,7 @@ function plot!(plot::Text{<:Tuple{<:AbstractArray{<:Tuple{<:Any, <:Point}}}}) pos_unequal = positions.val != poss strings_unequal && (strings.val = strs) pos_unequal && (positions.val = poss) - # Check for equality very imortant, otherwise we get an infinite loop + # Check for equality very important, otherwise we get an infinite loop strings_unequal && notify(strings) pos_unequal && notify(positions) diff --git a/src/basic_recipes/timeseries.jl b/src/basic_recipes/timeseries.jl index cb7bd8ba3bb..7409047fac5 100644 --- a/src/basic_recipes/timeseries.jl +++ b/src/basic_recipes/timeseries.jl @@ -10,7 +10,7 @@ scene = timeseries(signal) display(scene) # @async is optional, but helps to continue evaluating more code @async while isopen(scene) - # aquire data from e.g. a sensor: + # acquire data from e.g. a sensor: data = rand() # update the signal signal[] = data diff --git a/src/bezier.jl b/src/bezier.jl index 20107e0bdbf..03d74c32e34 100644 --- a/src/bezier.jl +++ b/src/bezier.jl @@ -642,7 +642,7 @@ function render_path(path, bitmap_size_px = 256) # freetype has no ClosePath and EllipticalArc, so those need to be replaced path_replaced = replace_nonfreetype_commands(path) - # Minimal size that becomes integer when mutliplying by 64 (target size for + # Minimal size that becomes integer when multiplying by 64 (target size for # atlas). This adds padding to avoid blurring/scaling factors from rounding # during sdf generation path_size = widths(bbox(path)) / maximum(widths(bbox(path))) diff --git a/src/configuration.yaml b/src/configuration.yaml index 5a7f32fec1c..ef9fb5f065b 100644 --- a/src/configuration.yaml +++ b/src/configuration.yaml @@ -18,7 +18,7 @@ theme: SSAO: enable: false - bias: 0.025 # z threshhold for occlusion + bias: 0.025 # z threshold for occlusion radius: 0.5 # range of sample positions (in world space) blur: 2 # A (2blur+1) by (2blur+1) range is used for blurring N_samples: 64 # number of samples (requires shader reload) diff --git a/src/conversions.jl b/src/conversions.jl index 81fdad94dcc..cfdd000a1ae 100644 --- a/src/conversions.jl +++ b/src/conversions.jl @@ -36,7 +36,7 @@ end # if no specific conversion is defined, we don't convert convert_single_argument(@nospecialize(x)) = x -# replace missings with NaNs +# replace `missing`s with `NaN`s function convert_single_argument(a::AbstractArray{<:Union{Missing, <:Real}}) return float_convert(a) end @@ -195,7 +195,7 @@ end function convert_arguments(::Type{<: Lines}, rect::Rect3{T}) where {T} PT = Point3{float_type(T)} points = unique(decompose(PT, rect)) - push!(points, PT(NaN)) # use to seperate linesegments + push!(points, PT(NaN)) # use to separate linesegments return (points[[1, 2, 3, 4, 1, 5, 6, 2, 9, 6, 8, 3, 9, 5, 7, 4, 9, 7, 8]],) end """ @@ -406,7 +406,7 @@ function convert_arguments(::CellGrid, x::EndPointsLike, y::EndPointsLike, Ty = typeof(ye[1]) # heatmaps are centered on the edges, so we need to adjust the range # This is done in conversions, since it's also how we calculate the boundingbox (heatmapplot.x, heatmap.y) - # We need the endpoint type here, since convert_arguments((0, 1), (0, 1), z), whcih only substracts the step + # We need the endpoint type here, since convert_arguments((0, 1), (0, 1), z), which only subtracts the step # Will end in a stackoverflow, since convert_arguments gets called every time the `args_in != args_out`. # If we return a different type with no conversion overload, it stops that recursion return (EndPoints{Tx}(xe[1] - xstep, xe[2] + xstep), EndPoints{Ty}(ye[1] - ystep, ye[2] + ystep), el32convert(z)) @@ -1136,7 +1136,7 @@ function line_diff_pattern(ls::Symbol, gaps::GapType = :normal) else error( """ - Unkown line style: $ls. Available linestyles are: + Unknown line style: $ls. Available linestyles are: :solid, :dash, :dot, :dashdot, :dashdotdot or a sequence of numbers enumerating the next transparent/opaque region. This sequence of numbers must be cumulative; 1 unit corresponds to 1 line width. @@ -1463,7 +1463,7 @@ function available_gradients() end -to_colormap(cm, categories::Integer) = error("`to_colormap(cm, categories)` is deprecated. Use `Makie.categorical_colors(cm, categories)` for categorical colors, and `resample_cmap(cmap, ncolors)` for continous resampling.") +to_colormap(cm, categories::Integer) = error("`to_colormap(cm, categories)` is deprecated. Use `Makie.categorical_colors(cm, categories)` for categorical colors, and `resample_cmap(cmap, ncolors)` for continuous resampling.") """ categorical_colors(colormaplike, categories::Integer) diff --git a/src/dim-converts/categorical-integration.jl b/src/dim-converts/categorical-integration.jl index 126aa859231..8d9d7936e74 100644 --- a/src/dim-converts/categorical-integration.jl +++ b/src/dim-converts/categorical-integration.jl @@ -14,7 +14,7 @@ scatter(1:4, Categorical(["a", "b", "c", "a"])) ``` ```julia -# Explicitely set them for other types: +# Explicitly set them for other types: struct Named value end diff --git a/src/dim-converts/dates-integration.jl b/src/dim-converts/dates-integration.jl index f7b6ac54189..b8d51646481 100644 --- a/src/dim-converts/dates-integration.jl +++ b/src/dim-converts/dates-integration.jl @@ -36,7 +36,7 @@ date_time_range = range(date_time, step=Week(5), length=10) # Automatically chose xticks as DateTeimeTicks: scatter(date_time_range, 1:10) -# explicitely chose DateTimeConversion and use it to plot unitful values into it and display in the `Time` format: +# explicitly chose DateTimeConversion and use it to plot unitful values into it and display in the `Time` format: using Makie.Unitful conversion = Makie.DateTimeConversion(Time) scatter(1:4, (1:4) .* u"s", axis=(dim2_conversion=conversion,)) diff --git a/src/dim-converts/dim-converts.jl b/src/dim-converts/dim-converts.jl index d4c32829311..bedb297883d 100644 --- a/src/dim-converts/dim-converts.jl +++ b/src/dim-converts/dim-converts.jl @@ -75,7 +75,7 @@ end # Recursively gets the dim convert from the plot -# This needs to be recursive to allow recipes to use dim converst +# This needs to be recursive to allow recipes to use dim convert # TODO, should a recipe always set the dim convert to it's parent? get_conversions(any) = nothing diff --git a/src/display.jl b/src/display.jl index b0d33f21979..c5ce72610de 100644 --- a/src/display.jl +++ b/src/display.jl @@ -22,7 +22,7 @@ end """ push_screen!(scene::Scene, screen::MakieScreen) -Adds a screen to the scene and registeres a clean up event when screen closes. +Adds a screen to the scene and registered a clean up event when screen closes. Also, makes sure that always just one screen is active for on scene. """ function push_screen!(scene::Scene, screen::T) where {T<:MakieScreen} @@ -140,7 +140,7 @@ function Base.display(figlike::FigureLike; backend=current_backend(), """) end - # We show inline if explicitely requested or if automatic and we can actually show something inline! + # We show inline if explicitly requested or if automatic and we can actually show something inline! scene = get_scene(figlike) if (inline === true || inline === automatic) && can_show_inline(backend) # We can't forward the screenconfig to show, but show uses the current screen if there is any @@ -187,7 +187,7 @@ end # Since VSCode doesn't call any display/show method for Figurelike if we return # `showable(mime, fig) == false`, we need to return `showable(mime, figlike) == true` # For some vscode displayable mime, even for `Makie.inline!(false)` when we want to display in our own window. -# Only diagnostic can be used for this, since other mimes expect something to be shown afterall and +# Only diagnostic can be used for this, since other mimes expect something to be shown after all and # therefore will look broken in the plotpane if we dont print anything to the IO. # I tried `throw(MethodError(...))` as well, but with plotpane enabled + showable == true, # VScode doesn't catch that method error. diff --git a/src/documentation/documentation.jl b/src/documentation/documentation.jl index 31001416569..f9879545d95 100644 --- a/src/documentation/documentation.jl +++ b/src/documentation/documentation.jl @@ -172,7 +172,7 @@ end """ to_func(Typ) -Maps the input of a Type name to its cooresponding function. +Maps the input of a Type name to its corresponding function. """ function to_func(Typ::Type{<: AbstractPlot{F}}) where F F @@ -184,7 +184,7 @@ to_func(func::Function) = func """ to_type(func) -Maps the input of a function name to its cooresponding Type. +Maps the input of a function name to its corresponding Type. """ to_type(func::Function) = Plot{func} diff --git a/src/ffmpeg-util.jl b/src/ffmpeg-util.jl index 2bb14ffad61..9a9aea2d63f 100644 --- a/src/ffmpeg-util.jl +++ b/src/ffmpeg-util.jl @@ -50,7 +50,7 @@ struct VideoStreamOptions pixel_format, loop, loglevel::String, input::String, rawvideo::Bool=true) if !isa(framerate, Integer) - @warn "The given framefrate is not a subtype of `Integer`, and will be rounded to the nearest integer. To supress this warning, provide an integer as the framerate." + @warn "The given framefrate is not a subtype of `Integer`, and will be rounded to the nearest integer. To suppress this warning, provide an integer as the framerate." framerate = round(Int, framerate) end diff --git a/src/float32-scaling.jl b/src/float32-scaling.jl index deb4e5e0464..0b769fbe9b5 100644 --- a/src/float32-scaling.jl +++ b/src/float32-scaling.jl @@ -107,7 +107,7 @@ function patch_model(@nospecialize(plot), f32c::Float32Convert, model::Observabl onany(plot, f32c.scaling, model, update = true) do f32c, model # Neutral f32c can mean that data and model cancel each other and we - # still have Float32 preicsion issues inbetween. + # still have Float32 preicsion issues in between. # works with rotation component as well, but drops signs on scale trans, scale = decompose_translation_scale_matrix(model) @@ -122,7 +122,7 @@ function patch_model(@nospecialize(plot), f32c::Float32Convert, model::Observabl elseif is_float_safe(scale, trans) && is_rot_free # model can be applied on GPU and we can pull f32c through the # model matrix. This can be merged with the option below, but - # keeping them separate improves compatability with transform_marker + # keeping them separate improves compatibility with transform_marker scale = Vec3d(model[1, 1], model[2, 2], model[3, 3]) # existing scale is missing signs f32c_obs[] = Makie.LinearScaling( f32c.scale, ((f32c.scale .- 1) .* trans .+ f32c.offset) ./ scale @@ -195,7 +195,7 @@ conversion applied to the given limits results in a range not representable with Float32 to high enough precision, the conversion will update. After the update update the converted range will be -1 .. 1. -The function returns true if an update has occured. If `Nothing` is passed, the +The function returns true if an update has occurred. If `Nothing` is passed, the function always returns false. """ function update_limits!(c::Float32Convert, mini::VecTypes{3, Float64}, maxi::VecTypes{3, Float64}) diff --git a/src/interaction/inspector.jl b/src/interaction/inspector.jl index ba8a6901728..f5e166ef393 100644 --- a/src/interaction/inspector.jl +++ b/src/interaction/inspector.jl @@ -286,11 +286,11 @@ function DataInspector(scene::Scene; priority = 100, kwargs...) on_hover(inspector) end end - listners = onany(e.mouseposition, e.scroll) do _, _ + listeners = onany(e.mouseposition, e.scroll) do _, _ empty_channel!(channel) # remove queued up hover requests put!(channel, nothing) end - append!(inspector.obsfuncs, listners) + append!(inspector.obsfuncs, listeners) on(base_attrib.enable_indicators) do enabled if !enabled yield() diff --git a/src/jl_rasterizer/bmp.jl b/src/jl_rasterizer/bmp.jl index bf35407b4d3..e18a6d27f77 100644 --- a/src/jl_rasterizer/bmp.jl +++ b/src/jl_rasterizer/bmp.jl @@ -24,8 +24,8 @@ function writebmp(io, img) write(io, UInt32(0)) # image bits size write(io, Int32(0)) # horz resoluition in pixel / m write(io, Int32(0)) # vert resolutions (0x03C3 = 96 dpi, 0x0B13 = 72 dpi) - write(io, Int32(0)) #colors in pallete - write(io, Int32(0)) #important colors + write(io, Int32(0)) # colors in palette + write(io, Int32(0)) # important colors for i in h:-1:1 for j in 1:w diff --git a/src/makielayout/blocks/colorbar.jl b/src/makielayout/blocks/colorbar.jl index 02075a67272..9dc1be7ab0c 100644 --- a/src/makielayout/blocks/colorbar.jl +++ b/src/makielayout/blocks/colorbar.jl @@ -82,7 +82,7 @@ function extract_colormap_recursive(@nospecialize(plot::T)) where {T <: Abstract # Prefer ColorMapping if in doubt! cmaps = filter(x-> x isa ColorMapping, colormaps) length(cmaps) == 1 && return cmaps[1] - error("Multiple colormaps found for plot $(plot), please specify which one to use manually. Please overload `Makie.extract_colormap(::$(T))` to allow for the automatical creation of a Colorbar.") + error("Multiple colormaps found for plot $(plot), please specify which one to use manually. Please overload `Makie.extract_colormap(::$(T))` to allow for the automatic creation of a Colorbar.") end end end @@ -93,7 +93,7 @@ function Colorbar(fig_or_scene, plot::AbstractPlot; kwargs...) func = plotfunc(plot) if isnothing(cmap) error("Neither $(func) nor any of its children use a colormap. Cannot create a Colorbar from this plot, please create it manually. - If this is a recipe, one needs to overload `Makie.extract_colormap(::$(Plot{func}))` to allow for the automatical creation of a Colorbar.") + If this is a recipe, one needs to overload `Makie.extract_colormap(::$(Plot{func}))` to allow for the automatic creation of a Colorbar.") end if !(cmap isa ColorMapping) error("extract_colormap(::$(Plot{func})) returned an invalid value: $cmap. Needs to return either a `Makie.ColorMapping`.") @@ -222,9 +222,9 @@ function initialize_block!(cb::Colorbar) update_xyrange(barbox[], cb.vertical[], colors[], cmap.scale[], cmap.color_mapping_type[]) onany(update_xyrange, blockscene, barbox, cb.vertical, colors, cmap.scale, cmap.color_mapping_type) - # for continous colormaps we sample a 1d image + # for continuous colormaps we sample a 1d image # to avoid white lines when rendering vector graphics - continous_pixels = lift(blockscene, cb.vertical, colors, + continuous_pixels = lift(blockscene, cb.vertical, colors, cmap.color_mapping_type) do v, colors, mapping_type if mapping_type !== Makie.categorical colors = (colors[1:end-1] .+ colors[2:end]) ./2 @@ -235,26 +235,26 @@ function initialize_block!(cb::Colorbar) # TODO, implement interpolate = true for irregular grics in CairoMakie # Then, we can just use heatmap! and don't need the image plot! show_cats = Observable(false; ignore_equal_values=true) - show_continous = Observable(false; ignore_equal_values=true) + show_continuous = Observable(false; ignore_equal_values=true) on(blockscene, cmap.color_mapping_type; update=true) do type if type === continuous - show_continous[] = true + show_continuous[] = true show_cats[] = false else - show_continous[] = false + show_continuous[] = false show_cats[] = true end end heatmap!(blockscene, - xrange, yrange, continous_pixels; + xrange, yrange, continuous_pixels; colormap=colormap, visible=show_cats, inspectable=false ) image!(blockscene, - lift(extrema, xrange), lift(extrema, yrange), continous_pixels; + lift(extrema, xrange), lift(extrema, yrange), continuous_pixels; colormap = colormap, - visible = show_continous, + visible = show_continuous, inspectable = false ) diff --git a/src/makielayout/blocks/polaraxis.jl b/src/makielayout/blocks/polaraxis.jl index 13ce889dec0..efc6c2d3dcc 100644 --- a/src/makielayout/blocks/polaraxis.jl +++ b/src/makielayout/blocks/polaraxis.jl @@ -1,5 +1,5 @@ ################################################################################ -### Main Block Intialization +### Main Block Initialization ################################################################################ function initialize_block!(po::PolarAxis; palette=nothing) diff --git a/src/specapi.jl b/src/specapi.jl index 2f5e1a6e08d..8f4bf7dd11e 100644 --- a/src/specapi.jl +++ b/src/specapi.jl @@ -288,7 +288,7 @@ end function distance_score(a::BlockSpec, b::BlockSpec, scores_dict) a === b && return 0.0 - (a.type !== b.type) && return 100.0 # Cant update when types dont match + (a.type !== b.type) && return 100.0 # Can't update when types dont match get!(scores_dict, (a, b)) do scores = Float64[ distance_score(a.kwargs, b.kwargs, scores_dict), @@ -448,7 +448,7 @@ function update_plot!(obs_to_notify, plot::AbstractPlot, oldspec::PlotSpec, spec end end # Cycling needs to be handled separately sadly, - # since they're implicitely mutating attributes, e.g. if I re-use a plot + # since they're implicitly mutating attributes, e.g. if I re-use a plot # that has been on cycling position 2, and now I re-use it for the first plot in the list # it will need to change to the color of cycling position 1 if haskey(plot, :cycle) diff --git a/src/stats/violin.jl b/src/stats/violin.jl index 2e4f418223c..b42a61e7161 100644 --- a/src/stats/violin.jl +++ b/src/stats/violin.jl @@ -59,7 +59,7 @@ function plot!(plot::Violin) dodge, n_dodge, gap, dodge_gap, orientation x̂, violinwidth = compute_x_and_width(x, width, gap, dodge, n_dodge, dodge_gap) - # for horizontal violin just flip all componentes + # for horizontal violin just flip all components point_func = Point2f if orientation === :horizontal point_func = flip_xy ∘ point_func diff --git a/src/theming.jl b/src/theming.jl index 00841657e68..a4aebd95c33 100644 --- a/src/theming.jl +++ b/src/theming.jl @@ -9,7 +9,7 @@ function wong_colors(alpha = 1.0) RGB(0/255, 158/255, 115/255), # green RGB(204/255, 121/255, 167/255), # reddish purple RGB(86/255, 180/255, 233/255), # sky blue - RGB(213/255, 94/255, 0/255), # vermillion + RGB(213/255, 94/255, 0/255), # vermilion RGB(240/255, 228/255, 66/255), # yellow ] return RGBAf.(colors, alpha) @@ -64,7 +64,7 @@ const MAKIE_DEFAULT_THEME = Attributes( limits = automatic, SSAO = Attributes( # enable = false, - bias = 0.025f0, # z threshhold for occlusion + bias = 0.025f0, # z threshold for occlusion radius = 0.5f0, # range of sample positions (in world space) blur = Int32(2), # A (2blur+1) by (2blur+1) range is used for blurring # N_samples = 64, # number of samples (requires shader reload) @@ -72,7 +72,7 @@ const MAKIE_DEFAULT_THEME = Attributes( inspectable = true, clip_planes = Vector{Plane3f}(), - # Vec is equvalent to 36° right/east, 39° up/north from camera position + # Vec is equivalent to 36° right/east, 39° up/north from camera position # The order here is Vec3f(right of, up from, towards) viewer/camera light_direction = Vec3f(-0.45679495, -0.6293204, -0.6287243), camera_relative_light = true, # Only applies to default DirectionalLight diff --git a/src/types.jl b/src/types.jl index b99c5d56e60..7160cdc7b4e 100644 --- a/src/types.jl +++ b/src/types.jl @@ -21,7 +21,7 @@ include("interaction/iodevices.jl") Identifies the source of a tick: - `BackendTick`: A tick used for backend purposes which is not present in `event.tick`. -- `UnknownTickState`: A tick from an uncategorized source (e.g. intialization of Events). +- `UnknownTickState`: A tick from an uncategorized source (e.g. initialization of Events). - `PausedRenderTick`: A tick from a paused renderloop. - `SkippedRenderTick`: A tick from a running renderloop where the previous image was reused. - `RegularRenderTick`: A tick from a running renderloop where a new image was produced. diff --git a/src/utilities/utilities.jl b/src/utilities/utilities.jl index ee44ed34814..52e9cf7870a 100644 --- a/src/utilities/utilities.jl +++ b/src/utilities/utilities.jl @@ -298,7 +298,7 @@ function merged_get!(defaults::Function, key, scene::SceneLike, input::Attribute d = defaults() if haskey(theme(scene), key) # we need to merge theme(scene) with the defaults, because it might be an incomplete theme - # TODO have a mark that says "theme uncomplete" and only then get the defaults + # TODO have a mark that says "theme incomplete" and only then get the defaults d = merge!(to_value(theme(scene, key)), d) end return merge!(input, d) @@ -427,7 +427,7 @@ function nan_aware_normals(vertices::AbstractVector{<:GeometryBasics.PointMeta{D end function surface2mesh(xs, ys, zs::AbstractMatrix, transform_func = identity, space = :data) - # crate a `Matrix{Point3}` + # create a `Matrix{Point3}` # ps = matrix_grid(identity, xs, ys, zs) ps = matrix_grid(p -> apply_transform(transform_func, p, space), xs, ys, zs) # create valid tessellations (triangulations) for the mesh diff --git a/test/Plane.jl b/test/Plane.jl index f3cac789700..3b528b5344f 100644 --- a/test/Plane.jl +++ b/test/Plane.jl @@ -86,7 +86,7 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d @test eachindex(ps) == Makie.unclipped_indices([plane], ps, :other) end - @testset "Tranformations" begin + @testset "Transformations" begin # Test apply_transform() plane = Plane(Point3f(1), Vec3f(0,0,1)) v = normalize(2f0 .* rand(Vec3f) .- 1) diff --git a/test/PolarAxis.jl b/test/PolarAxis.jl index c05b24c0289..dcfc45535a3 100644 --- a/test/PolarAxis.jl +++ b/test/PolarAxis.jl @@ -13,7 +13,7 @@ ) rticklabelplot = po.overlay.plots[8].plots[1] - # Mostly for verfication that we got the right plot + # Mostly for verification that we got the right plot @test po.overlay.plots[8][1][] == [("0.0", Point2f(0.0, 0.0)), ("2.5", Point2f(0.25, 0.0)), ("5.0", Point2f(0.5, 0.0)), ("7.5", Point2f(0.75, 0.0)), ("10.0", Point2f(1.0, 0.0))] # automatic diff --git a/test/convert_arguments.jl b/test/convert_arguments.jl index 77e9e525815..a075c27b133 100644 --- a/test/convert_arguments.jl +++ b/test/convert_arguments.jl @@ -68,7 +68,7 @@ end @test_throws ArgumentError heatmap(1im) end -# custom vector type to ensure that the conversion can be overriden for vectors +# custom vector type to ensure that the conversion can be overridden for vectors struct MyConvVector <: AbstractVector{Float64} end Makie.convert_arguments(::PointBased, ::MyConvVector) = ([Point(10, 20)],) diff --git a/test/events.jl b/test/events.jl index 3e5ac1b2392..524d029e69e 100644 --- a/test/events.jl +++ b/test/events.jl @@ -230,7 +230,7 @@ Base.:(==)(l::Or, r::Or) = l.left == r.left && l.right == r.right - # Reset state so this is indepentent from the last checks + # Reset state so this is independent from the last checks scene = Scene(size=(800, 600)); e = events(scene) cam3d!(scene, fixed_axis=true, cad=false, zoom_shift_lookat=false) diff --git a/test/float32convert.jl b/test/float32convert.jl index cd2408c8509..d01ace2079d 100644 --- a/test/float32convert.jl +++ b/test/float32convert.jl @@ -16,7 +16,7 @@ using Makie: Mat4f, Vec2d, Vec3d, Point2d, Point3d, Point4d f32min = Float64(floatmin(Float32)) * f32c.resolution f32eps = Float64(eps(Float32)) * f32c.resolution - @testset "Intialization" begin + @testset "Initialization" begin @test f32c.scaling[] == unit_scaling @test f32c.resolution == 1f4 # this may be subject to change end diff --git a/test/ray_casting.jl b/test/ray_casting.jl index 1d159fc0e75..2d9d39ae11f 100644 --- a/test/ray_casting.jl +++ b/test/ray_casting.jl @@ -276,7 +276,7 @@ end end - # Optional - show selected positon + # Optional - show selected position # This may change the camera, so don't use it for test values # for apply_transform = false, add `transformation = transform` scatter!(scene, pos, color = :red, strokewidth = 1.0, strokecolor = :yellow, depth_shift = -1f-1) diff --git a/test/scenes.jl b/test/scenes.jl index a2c26e96b92..1a7876bcca7 100644 --- a/test/scenes.jl +++ b/test/scenes.jl @@ -4,8 +4,8 @@ @testset "getproperty(scene, :$field)" for field in fieldnames(Scene) @test getproperty(scene, field) !== missing # well, just don't error end - @test theme(nothing, :nonexistant, default=1) == 1 - @test theme(scene, :nonexistant, default=1) == 1 + @test theme(nothing, :nonexistent, default=1) == 1 + @test theme(scene, :nonexistent, default=1) == 1 end @testset "Lighting" begin From 0a67ffc71bfb15ef5d187d23aaaabc999cc0c6c4 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 1 Nov 2024 13:50:28 +0100 Subject: [PATCH 08/17] Improve closeall and renderloop task handling (#4538) * stop_renderloop needs atomic + cleanup * correct order --- GLMakie/src/screen.jl | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/GLMakie/src/screen.jl b/GLMakie/src/screen.jl index 31bd584a089..a6782635144 100644 --- a/GLMakie/src/screen.jl +++ b/GLMakie/src/screen.jl @@ -164,7 +164,7 @@ mutable struct Screen{GLWindow} <: MakieScreen shader_cache::GLAbstraction.ShaderCache framebuffer::GLFramebuffer config::Union{Nothing, ScreenConfig} - stop_renderloop::Bool + stop_renderloop::Threads.Atomic{Bool} rendertask::Union{Task, Nothing} timer::BudgetedTimer px_per_unit::Observable{Float32} @@ -207,7 +207,7 @@ mutable struct Screen{GLWindow} <: MakieScreen s = size(framebuffer) screen = new{GLWindow}( glscreen, owns_glscreen, shader_cache, framebuffer, - config, stop_renderloop, rendertask, BudgetedTimer(1.0 / 30.0), + config, Threads.Atomic{Bool}(stop_renderloop), rendertask, BudgetedTimer(1.0 / 30.0), Observable(0f0), screen2scene, screens, renderlist, postprocessors, cache, cache2plot, Matrix{RGB{N0f8}}(undef, s), Observable(Makie.UnknownTickState), @@ -661,11 +661,12 @@ Doesn't destroy the screen and instead frees it to be re-used again, if `reuse=t function Base.close(screen::Screen; reuse=true) @debug("Close screen!") set_screen_visibility!(screen, false) - stop_renderloop!(screen; close_after_renderloop=false) if screen.window_open[] # otherwise we trigger an infinite loop of closing screen.window_open[] = false end empty!(screen) + stop_renderloop!(screen; close_after_renderloop=false) + if reuse && screen.reuse @debug("reusing screen!") push!(SCREEN_REUSE_POOL, screen) @@ -684,20 +685,13 @@ function closeall(; empty_shader=true) empty!(LOADED_SHADERS) WARN_ON_LOAD[] = false end - while !isempty(SCREEN_REUSE_POOL) - screen = pop!(SCREEN_REUSE_POOL) - delete!(ALL_SCREENS, screen) - destroy!(screen) - end - if !isempty(SINGLETON_SCREEN) - screen = pop!(SINGLETON_SCREEN) - delete!(ALL_SCREENS, screen) - destroy!(screen) - end + while !isempty(ALL_SCREENS) screen = pop!(ALL_SCREENS) destroy!(screen) end + empty!(SINGLETON_SCREEN) + empty!(SCREEN_REUSE_POOL) return end @@ -815,7 +809,7 @@ end Makie.to_native(x::Screen) = x.glscreen function renderloop_running(screen::Screen) - return !screen.stop_renderloop && !isnothing(screen.rendertask) && !istaskdone(screen.rendertask) + return !screen.stop_renderloop[] && !isnothing(screen.rendertask) && !istaskdone(screen.rendertask) end function start_renderloop!(screen::Screen) @@ -823,7 +817,7 @@ function start_renderloop!(screen::Screen) screen.config.pause_renderloop = false return else - screen.stop_renderloop = false + screen.stop_renderloop[] = false task = @async screen.config.renderloop(screen) yield() if istaskstarted(task) @@ -844,7 +838,7 @@ function stop_renderloop!(screen::Screen; close_after_renderloop=screen.close_af # don't double close when stopping renderloop c = screen.close_after_renderloop screen.close_after_renderloop = close_after_renderloop - screen.stop_renderloop = true + screen.stop_renderloop[] = true screen.close_after_renderloop = c # stop_renderloop! may be called inside renderloop as part of close @@ -890,7 +884,7 @@ scalechangeobs(screen) = scalefactor -> scalechangeobs(screen, scalefactor) function vsynced_renderloop(screen) - while isopen(screen) && !screen.stop_renderloop + while isopen(screen) && !screen.stop_renderloop[] if screen.config.pause_renderloop pollevents(screen, Makie.PausedRenderTick); sleep(0.1) continue @@ -905,7 +899,7 @@ end function fps_renderloop(screen::Screen) reset!(screen.timer, 1.0 / screen.config.framerate) - while isopen(screen) && !screen.stop_renderloop + while isopen(screen) && !screen.stop_renderloop[] if screen.config.pause_renderloop pollevents(screen, Makie.PausedRenderTick) else @@ -935,7 +929,7 @@ function on_demand_renderloop(screen::Screen) tick_state = Makie.UnknownTickState # last_time = time_ns() reset!(screen.timer, 1.0 / screen.config.framerate) - while isopen(screen) && !screen.stop_renderloop + while isopen(screen) && !screen.stop_renderloop[] pollevents(screen, tick_state) # GLFW poll if !screen.config.pause_renderloop && requires_update(screen) @@ -953,7 +947,7 @@ function on_demand_renderloop(screen::Screen) # push!(time_record, 1e-9 * (t - last_time)) # last_time = t end - cause = screen.stop_renderloop ? "stopped renderloop" : "closing window" + cause = screen.stop_renderloop[] ? "stopped renderloop" : "closing window" @debug("Leaving renderloop, cause: $(cause)") end From 695df644c7b6c653311effae185d55c62f6e6655 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 1 Nov 2024 13:47:32 -0700 Subject: [PATCH 09/17] Add attributes to the Raincloud definition (#4517) * Add attributes to the Raincloud definition * Update CHANGELOG.md --------- Co-authored-by: Simon --- CHANGELOG.md | 1 + src/basic_recipes/raincloud.jl | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3e4de0309d..3b5e2b8f3a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] - Added `subsup` and `left_subsup` functions that offer stacked sub- and superscripts for `rich` text which means this style can be used with arbitrary fonts and is not limited to fonts supported by MathTeXEngine.jl [#4489](https://github.com/MakieOrg/Makie.jl/pull/4489). +- Added the `jitter_width` and `side_nudge` attributes to the `raincloud` plot definition, so that they can be used as kwargs [#4517]https://github.com/MakieOrg/Makie.jl/pull/4517) - Expand PlotList plots to expose their child plots to the legend interface, allowing `axislegend`show plots within PlotSpecs as individual entries. [#4546](https://github.com/MakieOrg/Makie.jl/pull/4546) - Implement S.Colorbar(plotspec) [#4520](https://github.com/MakieOrg/Makie.jl/pull/4520). diff --git a/src/basic_recipes/raincloud.jl b/src/basic_recipes/raincloud.jl index a450ae0c750..bf5fb74116e 100644 --- a/src/basic_recipes/raincloud.jl +++ b/src/basic_recipes/raincloud.jl @@ -29,12 +29,6 @@ between each. # Keywords -## Violin/Histogram Plot Specific Keywords - -## Scatter Plot Specific Keywords -- `side_nudge`: Default value is 0.02 if `plot_boxplots` is true, otherwise `0.075` default. -- `jitter_width=0.05`: Determines the width of the scatter-plot bar in category x-axis - absolute terms. """ @recipe RainClouds (category_labels, data_array) begin """ @@ -116,6 +110,14 @@ between each. If `clouds=hist`, this passes down the number of bins to the histogram call. """ hist_bins = 30 + """ + Scatter plot specific. Default value is 0.02 if `plot_boxplots` is true, otherwise `0.075` default. + """ + side_nudge = automatic + """ + Determines the width of the scatter-plot bar in category x-axis absolute terms. + """ + jitter_width = 0.05 """ A single color, or a vector of colors, one for each point. @@ -246,12 +248,11 @@ function plot!(plot::RainClouds) # Scatter Plot defaults dependent on if there is a boxplot side_scatter_nudge_default = plot_boxplots ? 0.2 : 0.075 - jitter_width_default = 0.05 # Scatter Plot Settings - side_scatter_nudge = to_value(get(plot, :side_nudge, side_scatter_nudge_default)) + side_scatter_nudge = plot.side_nudge[] isa Makie.Automatic ? side_scatter_nudge_default : plot.side_nudge[] side_scatter_nudge < 0 && ArgumentError("`side_nudge` should be positive. Change `side` to :left, :right if you wish.") - jitter_width = abs(to_value(get(plot, :jitter_width, jitter_width_default))) + jitter_width = plot.jitter_width[] jitter_width < 0 && ArgumentError("`jitter_width` should be positive.") markersize = plot.markersize[] From 28862f9cc61161232e9fa9206503b7dc73f0623e Mon Sep 17 00:00:00 2001 From: damianodegaspari <144324904+damianodegaspari@users.noreply.github.com> Date: Sat, 2 Nov 2024 14:19:08 +0100 Subject: [PATCH 10/17] Spelling in the documentation - Update types.jl (#4559) --- src/makielayout/types.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/makielayout/types.jl b/src/makielayout/types.jl index 4da455bf423..83321578634 100644 --- a/src/makielayout/types.jl +++ b/src/makielayout/types.jl @@ -1829,7 +1829,7 @@ end xzpanelvisible = true "The limits that the axis tries to set given other constraints like aspect. Don't set this directly, use `xlims!`, `ylims!` or `limits!` instead." targetlimits = Rect3f(Vec3f(0, 0, 0), Vec3f(1, 1, 1)) - "The limits that the user has manually set. They are reinstated when calling `reset_limits!` and are set to nothing by `autolimits!`. Can be either a tuple (xlow, xhigh, ylow, high, zlow, zhigh) or a tuple (nothing_or_xlims, nothing_or_ylims, nothing_or_zlims). Are set by `xlims!`, `ylims!`, `zlims!` and `limits!`." + "The limits that the user has manually set. They are reinstated when calling `reset_limits!` and are set to nothing by `autolimits!`. Can be either a tuple (xlow, xhigh, ylow, yhigh, zlow, zhigh) or a tuple (nothing_or_xlims, nothing_or_ylims, nothing_or_zlims). Are set by `xlims!`, `ylims!`, `zlims!` and `limits!`." limits = (nothing, nothing, nothing) "The relative margins added to the autolimits in x direction." xautolimitmargin = (0.05, 0.05) From 2b814f463545ff3540ccda4b3b6cd6137964a200 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 4 Nov 2024 13:38:24 +0100 Subject: [PATCH 11/17] Make all backends support Screen() constructor (#4561) * make all backends support Screen() constructor * add changelog --- CHANGELOG.md | 1 + CairoMakie/src/display.jl | 6 ++++-- CairoMakie/src/screen.jl | 33 ++++++++++++++++++++++++++++++++- WGLMakie/src/display.jl | 6 ++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b5e2b8f3a4..c831b8148e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- Added empty constructor to all backends for `Screen` allowing `display(Makie.current_backend().Screen(), fig)` [#4561](https://github.com/MakieOrg/Makie.jl/pull/4561). - Added `subsup` and `left_subsup` functions that offer stacked sub- and superscripts for `rich` text which means this style can be used with arbitrary fonts and is not limited to fonts supported by MathTeXEngine.jl [#4489](https://github.com/MakieOrg/Makie.jl/pull/4489). - Added the `jitter_width` and `side_nudge` attributes to the `raincloud` plot definition, so that they can be used as kwargs [#4517]https://github.com/MakieOrg/Makie.jl/pull/4517) - Expand PlotList plots to expose their child plots to the legend interface, allowing `axislegend`show plots within PlotSpecs as individual entries. [#4546](https://github.com/MakieOrg/Makie.jl/pull/4546) diff --git a/CairoMakie/src/display.jl b/CairoMakie/src/display.jl index 65a2671b37c..813b53b7b10 100644 --- a/CairoMakie/src/display.jl +++ b/CairoMakie/src/display.jl @@ -37,7 +37,9 @@ function Base.display(screen::Screen, scene::Scene; connect=false) return screen end -function Base.display(screen::Screen{IMAGE}, scene::Scene; connect=false) +function Base.display(screen::Screen{IMAGE}, scene::Scene; connect=false, screen_config...) + config = Makie.merge_screen_config(ScreenConfig, Dict{Symbol,Any}(screen_config)) + screen = Makie.apply_screen_config!(screen, config, scene) path = joinpath(mktempdir(), "display.png") Makie.push_screen!(scene, screen) cairo_draw(screen, scene) @@ -80,7 +82,7 @@ function Makie.backend_show(screen::Screen{SVG}, io::IO, ::MIME"image/svg+xml", # xlink:href="someid" (but not xlink:href="data:someothercontent" which is how image data is attached) # url(#someid) svg = replace(svg, r"((?:(?:id|xlink:href)=\"(?!data:)[^\"]+)|url\(#[^)]+)" => SubstitutionString("\\1-$salt")) - + print(io, svg) return screen end diff --git a/CairoMakie/src/screen.jl b/CairoMakie/src/screen.jl index b2fc618fa0b..b6759fee88d 100644 --- a/CairoMakie/src/screen.jl +++ b/CairoMakie/src/screen.jl @@ -175,6 +175,31 @@ mutable struct Screen{SurfaceRenderType} <: Makie.MakieScreen antialias::Int # cairo_antialias_t visible::Bool config::ScreenConfig + + function Screen() + return new{IMAGE}() + end + function Screen{SurfaceRenderType}( + scene::Scene, + surface::Cairo.CairoSurface, + context::Cairo.CairoContext, + device_scaling_factor::Float64, + antialias::Int, + visible::Bool, + config::ScreenConfig + ) where {SurfaceRenderType} + + return new{SurfaceRenderType}( + scene, + surface, + context, + device_scaling_factor, + antialias, + visible, + config, + ) + end + end function Base.empty!(screen::Screen) @@ -192,6 +217,7 @@ end Base.close(screen::Screen) = empty!(screen) function destroy!(screen::Screen) + isdefined(screen, :surface) || return Cairo.destroy(screen.surface) Cairo.destroy(screen.context) end @@ -200,7 +226,10 @@ function Base.isopen(screen::Screen) return !(screen.surface.ptr == C_NULL || screen.context.ptr == C_NULL) end -Base.size(screen::Screen) = round.(Int, (screen.surface.width, screen.surface.height)) +function Base.size(screen::Screen) + isdefined(screen, :surface) || return (0, 0) + round.(Int, (screen.surface.width, screen.surface.height)) +end # we render the scene directly, since we have # no screen dependent state like in e.g. opengl Base.insert!(screen::Screen, scene::Scene, plot) = nothing @@ -267,6 +296,7 @@ function Makie.apply_screen_config!( destroy!(old_screen) end apply_config!(screen, config) + screen.scene = scene return screen end @@ -275,6 +305,7 @@ function Makie.apply_screen_config!(screen::Screen, config::ScreenConfig, scene: Makie.apply_screen_config!(screen, config, scene, nothing, MIME"image/png"()) end + function Screen(scene::Scene; screen_config...) config = Makie.merge_screen_config(ScreenConfig, Dict{Symbol, Any}(screen_config)) return Screen(scene, config) diff --git a/WGLMakie/src/display.jl b/WGLMakie/src/display.jl index 16ebcb93588..73717f390f9 100644 --- a/WGLMakie/src/display.jl +++ b/WGLMakie/src/display.jl @@ -66,6 +66,12 @@ mutable struct Screen <: Makie.MakieScreen end end +function Screen(; config...) + config = Makie.merge_screen_config(ScreenConfig, Dict{Symbol,Any}(config)) + return Screen(nothing, config) +end + + function scene_already_displayed(screen::Screen, scene=screen.scene) scene === nothing && return false screen.scene === scene || return false From 73750724173e5a8ae297d1229621f188b16dfa68 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com> Date: Mon, 4 Nov 2024 14:26:45 +0100 Subject: [PATCH 12/17] IOCapture VideoStream hang fix (#4562) * avoid hang when closing over VideoStream with IOCapture.capture * add test * add changelog entry * fix testset position relative to backend activation --- CHANGELOG.md | 1 + src/ffmpeg-util.jl | 4 +++- test/Project.toml | 1 + test/record.jl | 9 +++++++++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c831b8148e6..2af1bc5031e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Added the `jitter_width` and `side_nudge` attributes to the `raincloud` plot definition, so that they can be used as kwargs [#4517]https://github.com/MakieOrg/Makie.jl/pull/4517) - Expand PlotList plots to expose their child plots to the legend interface, allowing `axislegend`show plots within PlotSpecs as individual entries. [#4546](https://github.com/MakieOrg/Makie.jl/pull/4546) - Implement S.Colorbar(plotspec) [#4520](https://github.com/MakieOrg/Makie.jl/pull/4520). +- Fixed a hang when `Record` was created inside a closure passed to `IOCapture.capture` [#4562](https://github.com/MakieOrg/Makie.jl/pull/4562). ## [0.21.15] - 2024-10-25 diff --git a/src/ffmpeg-util.jl b/src/ffmpeg-util.jl index 9a9aea2d63f..6cd0341069d 100644 --- a/src/ffmpeg-util.jl +++ b/src/ffmpeg-util.jl @@ -266,7 +266,9 @@ function VideoStream(fig::FigureLike; buffer = Matrix{RGB{N0f8}}(undef, xdim, ydim) vso = VideoStreamOptions(format, framerate, compression, profile, pixel_format, loop, loglevel, "pipe:0", true) cmd = to_ffmpeg_cmd(vso, xdim, ydim) - process = open(`$(FFMPEG_jll.ffmpeg()) $cmd $path`, "w") + # a plain `open` without the `pipeline` causes hangs when IOCapture.capture closes over a function that creates + # a `VideoStream` without closing the process explicitly, such as when returning `Record` in a cell in Documenter or quarto + process = open(pipeline(`$(FFMPEG_jll.ffmpeg()) $cmd $path`; stdout = devnull, stderr = devnull), "w") tick_controller = TickController(fig, 1.0 / vso.framerate, filter_ticks) result = VideoStream(process.in, process, screen, tick_controller, buffer, abspath(path), vso) finalizer(result) do x diff --git a/test/Project.toml b/test/Project.toml index 42c77e8b94a..2ccea4ae5ba 100644 --- a/test/Project.toml +++ b/test/Project.toml @@ -2,6 +2,7 @@ CategoricalArrays = "324d7699-5711-5eae-9e2f-1d82baa6b597" Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f" GeometryBasics = "5c1252a2-5f33-56bf-86c9-59e7332b4326" +IOCapture = "b5f81e59-6552-4d32-b1f0-c071b021bf89" InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240" KernelDensity = "5ab0869b-81aa-558d-bb23-cbf5423bbe9b" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" diff --git a/test/record.jl b/test/record.jl index 9e2310a28ed..9d2a92d298b 100644 --- a/test/record.jl +++ b/test/record.jl @@ -1,4 +1,5 @@ using Logging +using IOCapture: IOCapture module VideoBackend using Makie @@ -77,4 +78,12 @@ mktempdir() do tempdir end end end + +@testset "No hang when closing IOCapture.capture over VideoStream" begin + @test_nowarn IOCapture.capture() do + f = Figure() + Makie.VideoStream(f) + end +end + Makie.set_active_backend!(missing) From 0f4b02d1fc9febf6ce0333d81cf5358046a860e0 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 4 Nov 2024 15:06:58 +0100 Subject: [PATCH 13/17] Allow plots to move between scenes in SpecApi (#4478) * refactor JS Plot object to be movable * allow to move plots between scenes * correctly close channel * re-use plots, that got moved between axes * remove lock * fix specapi * fix makie tests * fix CairoMakie * Update CHANGELOG.md * try showing more infos * implement moveto! correctly for GLMakie and test for all backends * test & fix move_to! * add another test * make move_to optional * revert GLMakie changes * revert more changes * fix WGLMakie move_to * rename test * improve tests * clean up test --- CHANGELOG.md | 1 + MakieCore/src/recipes.jl | 4 +- ReferenceTests/src/tests/examples2d.jl | 42 +- ReferenceTests/src/tests/specapi.jl | 35 +- ReferenceTests/src/tests/updating.jl | 70 ++- ReferenceUpdater/src/local_server.jl | 10 +- WGLMakie/src/Serialization.js | 297 +++++++----- WGLMakie/src/display.jl | 7 +- WGLMakie/src/picking.jl | 6 +- WGLMakie/src/serialization.jl | 26 +- WGLMakie/src/three_plot.jl | 17 + WGLMakie/src/wglmakie.bundled.js | 598 ++++++++++++++----------- WGLMakie/test/runtests.jl | 8 +- src/interaction/inspector.jl | 63 +-- src/layouting/transformation.jl | 2 + src/scenes.jl | 44 +- src/specapi.jl | 102 +++-- test/specapi.jl | 6 +- 18 files changed, 844 insertions(+), 494 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2af1bc5031e..74cebd149f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- Allow plots to move between scenes in SpecApi [#4132](https://github.com/MakieOrg/Makie.jl/pull/4132). - Added empty constructor to all backends for `Screen` allowing `display(Makie.current_backend().Screen(), fig)` [#4561](https://github.com/MakieOrg/Makie.jl/pull/4561). - Added `subsup` and `left_subsup` functions that offer stacked sub- and superscripts for `rich` text which means this style can be used with arbitrary fonts and is not limited to fonts supported by MathTeXEngine.jl [#4489](https://github.com/MakieOrg/Makie.jl/pull/4489). - Added the `jitter_width` and `side_nudge` attributes to the `raincloud` plot definition, so that they can be used as kwargs [#4517]https://github.com/MakieOrg/Makie.jl/pull/4517) diff --git a/MakieCore/src/recipes.jl b/MakieCore/src/recipes.jl index 6e8da443ba6..6d87c95a565 100644 --- a/MakieCore/src/recipes.jl +++ b/MakieCore/src/recipes.jl @@ -435,7 +435,7 @@ function default_theme(scene, T::Type{<: Plot}) end return attr end - + function extract_docstring(str) if VERSION >= v"1.11" && str isa Base.Docs.DocStr return only(str.text::Core.SimpleVector) @@ -502,7 +502,7 @@ function create_recipe_expr(Tsym, args, attrblock) end function ($funcname!)(args...; kw...) kwdict = Dict{Symbol, Any}(kw) - _create_plot!($funcname, kwdict, args...) + _create_plot!($funcname, kwdict, args...) end $(arg_type_func) diff --git a/ReferenceTests/src/tests/examples2d.jl b/ReferenceTests/src/tests/examples2d.jl index 94414126528..89ef03ce26a 100644 --- a/ReferenceTests/src/tests/examples2d.jl +++ b/ReferenceTests/src/tests/examples2d.jl @@ -1498,14 +1498,14 @@ end @reference_test "Violin" begin fig = Figure() - + categories = vcat(fill(1, 300), fill(2, 300), fill(3, 300)) values = vcat(RNG.randn(300), (1.5 .* RNG.rand(300)).^2, -(1.5 .* RNG.rand(300)).^2) violin(fig[1, 1], categories, values) dodge = RNG.rand(1:2, 900) - violin(fig[1, 2], categories, values, dodge = dodge, - color = map(d->d==1 ? :yellow : :orange, dodge), + violin(fig[1, 2], categories, values, dodge = dodge, + color = map(d->d==1 ? :yellow : :orange, dodge), strokewidth = 2, strokecolor = :black, gap = 0.1, dodge_gap = 0.5 ) @@ -1513,7 +1513,7 @@ end color = :gray, side = :left ) - violin!(categories, values, orientation = :horizontal, + violin!(categories, values, orientation = :horizontal, color = :yellow, side = :right, strokewidth = 2, strokecolor = :black, weights = abs.(values) ) @@ -1607,7 +1607,7 @@ end @reference_test "boxplot" begin fig = Figure() - + categories = vcat(fill(1, 300), fill(2, 300), fill(3, 300)) values = RNG.randn(900) .+ range(-1, 1, length=900) boxplot(fig[1, 1], categories, values) @@ -1646,16 +1646,16 @@ end @reference_test "crossbar" begin fig = Figure() - + xs = [1, 1, 2, 2, 3, 3] ys = RNG.rand(6) ymins = ys .- 1 ymaxs = ys .+ 1 dodge = [1, 2, 1, 2, 1, 2] - + crossbar(fig[1, 1], xs, ys, ymins, ymaxs, dodge = dodge, show_notch = true) - - crossbar(fig[1, 2], xs, ys, ymins, ymaxs, + + crossbar(fig[1, 2], xs, ys, ymins, ymaxs, dodge = dodge, dodge_gap = 0.25, gap = 0.05, midlinecolor = :blue, midlinewidth = 5, @@ -1679,7 +1679,7 @@ end w = @. x^2 * (1 - x)^2 ecdfplot(f[1, 2], x) ecdfplot!(x; weights = w, color=:orange) - + f end @@ -1710,18 +1710,18 @@ end data[201:500] .-= 3 data[501:end] .= 3 .* abs.(data[501:end]) .- 3 labels = vcat(fill("red", 500), fill("green", 500)) - + fig = Figure() rainclouds(fig[1, 1], labels, data, plot_boxplots = false, cloud_width = 2.0, markersize = 5.0) rainclouds(fig[1, 2], labels, data, color = labels, orientation = :horizontal, cloud_width = 2.0) - rainclouds(fig[2, 1], labels, data, clouds = hist, hist_bins = 30, boxplot_nudge = 0.1, + rainclouds(fig[2, 1], labels, data, clouds = hist, hist_bins = 30, boxplot_nudge = 0.1, center_boxplot = false, boxplot_width = 0.2, whiskerwidth = 1.0, strokewidth = 3.0) rainclouds(fig[2, 2], labels, data, color = labels, side = :right, violin_limits = extrema) fig end -@reference_test "series" begin +@reference_test "series" begin fig = Figure() data = cumsum(RNG.randn(4, 21), dims = 2) @@ -1729,7 +1729,7 @@ end linewidth = 4, linestyle = :dot, markersize = 15, solid_color = :black) axislegend(ax, position = :lt) - ax, sp = series(fig[2, 1], data, labels=["label $i" for i in 1:4], markersize = 10.0, + ax, sp = series(fig[2, 1], data, labels=["label $i" for i in 1:4], markersize = 10.0, marker = Circle, markercolor = :transparent, strokewidth = 2.0, strokecolor = :black) axislegend(ax, position = :lt) @@ -1741,11 +1741,11 @@ end xs = LinRange(0, 4pi, 21) ys = sin.(xs) - + stairs(f[1, 1], xs, ys) stairs(f[2, 1], xs, ys; step=:post, color=:blue, linestyle=:dash) stairs(f[3, 1], xs, ys; step=:center, color=:red, linestyle=:dot) - + f end @@ -1760,7 +1760,7 @@ end stemcolor = :red, color = :orange, markersize = 15, strokecolor = :red, strokewidth = 3, trunklinestyle = :dash, stemlinestyle = :dashdot) - + stem(f[2, 1], xs, sin.(xs), offset = LinRange(-0.5, 0.5, 30), color = LinRange(0, 1, 30), colorrange = (0, 0.5), @@ -1782,21 +1782,21 @@ end fig = Figure() waterfall(fig[1, 1], y) - waterfall(fig[1, 2], y, show_direction = true, marker_pos = :cross, + waterfall(fig[1, 2], y, show_direction = true, marker_pos = :cross, marker_neg = :hline, direction_color = :yellow) colors = Makie.wong_colors() x = repeat(1:2, inner=5) group = repeat(1:5, outer=2) - waterfall(fig[2, 1], x, y, dodge = group, color = colors[group], + waterfall(fig[2, 1], x, y, dodge = group, color = colors[group], show_direction = true, show_final = true, final_color=(colors[6], 1//3), dodge_gap = 0.1, gap = 0.05) x = repeat(1:5, outer=2) group = repeat(1:2, inner=5) - - waterfall(fig[2, 2], x, y, dodge = group, color = colors[group], + + waterfall(fig[2, 2], x, y, dodge = group, color = colors[group], show_direction = true, stack = :x, show_final = true) fig diff --git a/ReferenceTests/src/tests/specapi.jl b/ReferenceTests/src/tests/specapi.jl index ab5847915d9..f6bd6fb8651 100644 --- a/ReferenceTests/src/tests/specapi.jl +++ b/ReferenceTests/src/tests/specapi.jl @@ -117,12 +117,39 @@ end st end +AxNoTicks(;kw...) = S.Axis(; xticksvisible=false, + yticksvisible=false, yticklabelsvisible=false, + xticklabelsvisible=false, kw...) + +@reference_test "Moving Plots in SpecApi" begin + pl1 = S.Heatmap((1, 4), (1, 4), Makie.peaks(50)) + pl2 = S.Scatter(1:4; color=1:4, markersize=30, strokewidth=1, strokecolor=:black) + ax1 = AxNoTicks(; plots=[pl1, pl2]) + grid = S.GridLayout(AxNoTicks()) + f, _, pl = plot(S.GridLayout([ax1 grid]; colgaps=Fixed(4)); figure=(; figure_padding=2, size=(500, 100))) + cb1 = copy(colorbuffer(f)) + + pl1 = S.Heatmap((1, 4), (1, 4), Makie.peaks(50); colormap=:inferno) + ax1 = AxNoTicks() + grid = S.GridLayout(AxNoTicks(; plots=[pl1, pl2])) + pl[1] = S.GridLayout([ax1 grid]; colgaps=Fixed(4)) + cb2 = copy(colorbuffer(f)) + + pl1 = S.Heatmap((1, 4), (1, 4), Makie.peaks(50)) + ax1 = AxNoTicks(; plots=[pl1]) + ax2 = S.GridLayout(AxNoTicks(; plots=[pl2])) + pl[1] = S.GridLayout([ax1 ax2]; colgaps=Fixed(4)) + cb3 = copy(colorbuffer(f)) + + imgs = hcat(rotr90.((cb1, cb2, cb3))...) + s = Scene(; size=size(imgs)) + image!(s, imgs; space=:pixel) + s +end + function to_plot(plots) axes = map(permutedims(plots)) do plot - ax = S.Axis(; - plots=[plot], xticksvisible=false, - yticksvisible=false, yticklabelsvisible=false, - xticklabelsvisible=false) + ax = AxNoTicks(; plots=[plot]) return S.GridLayout([ax S.Colorbar(plot)]) end return S.GridLayout(axes) diff --git a/ReferenceTests/src/tests/updating.jl b/ReferenceTests/src/tests/updating.jl index b80a3b2ef12..1482da75cc3 100644 --- a/ReferenceTests/src/tests/updating.jl +++ b/ReferenceTests/src/tests/updating.jl @@ -175,7 +175,7 @@ end end @reference_test "event ticks in record" begin - # Checks whether record calculates and triggers event.tick by drawing a + # Checks whether record calculates and triggers event.tick by drawing a # Point at y = 1 for each frame where it does. The animation is irrelevant # here, so we can just check the final image. # The first point maybe at 0 depending on when the backend sets up it's @@ -192,6 +192,68 @@ end f end +@reference_test "Moving plots with move_to" begin + f, ax, pl1 = scatter(5:-1:1; markersize=20, axis=(; title="Axis 1")) + pl2 = poly!(ax, Rect2f(10, 10, 100, 100); color=:green, space=:pixel) + pl3 = scatter!(ax, 1:5; color=Float64[1:5;], markersize=0.5, colorrange=(1, 5), lowclip=:black, + highclip=:red, markerspace=:data) + pl4 = poly!(ax, Rect2f(0, 0, 1, 1); color=(:green, 0.5), strokewidth=2, strokecolor=:black) + f + img1 = copy(colorbuffer(f; px_per_unit=1)) + plots = [pl1, pl2, pl3, pl4] + get_listener_lengths() = map(plots) do x + arg_l = length(x[1].listeners) + attr_l = length(x.color.listeners) + return [arg_l, attr_l] + end + listener_lengths_1 = get_listener_lengths() + + ax2 = Axis(f[1, 2]; title="Axis 2") + ls = LScene(f[2, :]; show_axis=false) + scene = Makie.camrelative(ls.scene) + + Makie.move_to!(pl2, ax2.scene) + Makie.move_to!(pl3, ax2.scene) + Makie.move_to!(pl4, scene) + # Make sure updating still works for color + pl3.color = [-1, 2, 3, 4, 7] + pl3.colormap = :inferno + pl3.markersize = 1 + + @test listener_lengths_1 == get_listener_lengths() + + img2 = copy(colorbuffer(f; px_per_unit=1)) + @test length(ax.scene.plots) == 1 + @test ax.scene.plots[1] === pl1 + @test length(ax2.scene.plots) == 2 + @test pl2 in ax2.scene.plots + @test pl3 in ax2.scene.plots + @test pl4 in scene.plots + @test length(scene) == 1 + + # Move everything back + pl3.color = Float64[1:5;] + pl3.colormap = :viridis + pl3.markersize = 0.5 + Makie.move_to!(pl1, ax.scene) + Makie.move_to!(pl2, ax.scene) + Makie.move_to!(pl3, ax.scene) + Makie.move_to!(pl4, ax.scene) + # Make it easier to see similarity to first plot, by removing new scenes + delete!(ls) + delete!(ax2) + trim!(f.layout) + + img3 = copy(colorbuffer(f; px_per_unit=1)) + + @test listener_lengths_1 == get_listener_lengths() + + imgs = hcat(rotr90.((img1, img2, img3))...) + s = Scene(; size=size(imgs)) + image!(s, imgs; space=:pixel) + s +end + @reference_test "updating surface size" begin X = Observable(-5:5) Y = Observable(-5:5) @@ -200,8 +262,8 @@ end f = Figure(size = (800, 400)) surface(f[1, 1], X, Y, Z) surface(f[1, 2], map(collect, X), map(collect, Y), Z) - surface(f[1, 3], - map((X, Y) -> [x for x in X, y in Y], X, Y), + surface(f[1, 3], + map((X, Y) -> [x for x in X, y in Y], X, Y), map((X, Y) -> [y for x in X, y in Y], X, Y), Z) st = Stepper(f) Makie.step!(st) @@ -215,4 +277,4 @@ end Z.val = [0.01 * x*x * y*y for x in X.val, y in Y.val] notify(Z) Makie.step!(st) -end \ No newline at end of file +end diff --git a/ReferenceUpdater/src/local_server.jl b/ReferenceUpdater/src/local_server.jl index 668af3fd6d9..192ce6a9d65 100644 --- a/ReferenceUpdater/src/local_server.jl +++ b/ReferenceUpdater/src/local_server.jl @@ -28,7 +28,7 @@ function serve_update_page_from_dir(folder) @info "Downloading latest reference folder for $tag" tempdir = download_refimages(tag) - + @info "Updating files in $tempdir" for image in images_to_update @@ -253,7 +253,7 @@ end function group_files(path, input_filename, output_filename) isfile(joinpath(path, output_filename)) && return - + # Group files in new_files/missing_files into a table like layout: # GLMakie CairoMakie WGLMakie @@ -261,12 +261,12 @@ function group_files(path, input_filename, output_filename) data = Dict{String, Vector{Bool}}() open(joinpath(path, input_filename), "r") do file for filepath in eachline(file) - pieces = split(filepath, '/') + pieces = splitpath(filepath) backend = pieces[1] if !(backend in ("GLMakie", "CairoMakie", "WGLMakie")) - error("Failed to parse backend in \"$line\", got \"$backend\"") + error("Failed to parse backend in \"$pieces\", got \"$backend\"") end - + filename = join(pieces[2:end], '/') exists = get!(data, filename, [false, false, false]) diff --git a/WGLMakie/src/Serialization.js b/WGLMakie/src/Serialization.js index 09de2ff7e8f..3c3a876fdc0 100644 --- a/WGLMakie/src/Serialization.js +++ b/WGLMakie/src/Serialization.js @@ -2,6 +2,144 @@ import * as THREE from "./THREE.js"; import * as Camera from "./Camera.js"; import { create_line, create_linesegments } from "./Lines.js"; + +/** + * Updates the value of a given uniform with a new value. + * + * @param {THREE.Uniform} uniform - The uniform to update. + * @param {Object|Array} new_value - The new value to set for the uniform. If the uniform is a texture, this should be an array containing the size and texture data. + */ +function update_uniform(uniform, new_value) { + if (uniform.value.isTexture) { + const im_data = uniform.value.image; + const [size, tex_data] = new_value; + if (tex_data.length == im_data.data.length) { + im_data.data.set(tex_data); + } else { + const old_texture = uniform.value; + uniform.value = re_create_texture(old_texture, tex_data, size); + old_texture.dispose(); + } + uniform.value.needsUpdate = true; + } else { + if (is_three_fixed_array(uniform.value)) { + uniform.value.fromArray(new_value); + } else { + uniform.value = new_value; + } + } +} + + +class Plot { + mesh = undefined; + parent = undefined; + uuid = ""; + name = ""; + is_instanced = false; + geometry_needs_recreation = false; + plot_data = {}; + + constructor(scene, data) { + + this.plot_data = data; + + connect_plot(scene, this); + + if (data.plot_type === "lines") { + this.mesh = create_line(scene, this.plot_data); + } else if (data.plot_type === "linesegments") { + this.mesh = create_linesegments(scene, this.plot_data); + } else if ("instance_attributes" in data) { + this.is_instanced = true + this.mesh = create_instanced_mesh(scene, this.plot_data); + } else { + this.mesh = create_mesh(scene, this.plot_data); + } + + this.name = data.name; + this.uuid = data.uuid; + this.mesh.plot_uuid = data.uuid; + + this.mesh.frustumCulled = false; + this.mesh.matrixAutoUpdate = false; + this.mesh.renderOrder = data.zvalue; + + + data.uniform_updater.on(([name, data]) => { + this.update_uniform(name, data); + }); + + if ( + !(data.plot_type === "lines" || data.plot_type === "linesegments") + ) { + connect_attributes(this.mesh, data.attribute_updater); + } + this.parent = scene; + // Give mesh a reference to the plot object. + this.mesh.plot_object = this; + this.mesh.visible = data.visible.value; + data.visible.on(v=> { + this.mesh.visible = v; + }); + + } + + move_to(scene) { + if (scene === this.parent) { + return + } + this.parent.remove(this.mesh) + connect_plot(scene, this) + scene.add(this.mesh) + this.parent = scene + return + } + + update(attributes) { + attributes.keys().forEach(key=> { + const value = attributes[key] + if (value.type == "uniform") { + this.update_uniform(key, value.data); + } else if (value.type === "geometry") { + this.update_geometries(value.data) + } else if (value.type === "faces") { + this.update_faces(value.data) + } + }) + // For e.g. when we need to re-create the geometry + this.apply_updates() + } + + update_uniform(name, new_data) { + const uniform = this.mesh.material.uniforms[name]; + if (!uniform) { + throw new Error(`Uniform ${name} doesn't exist in Plot: ${this.name}`) + } + update_uniform(uniform, new_data); + } + + update_geometry(name, new_data) { + buffer = this.mesh.geometry.attributes[name]; + if (!buffer) { + throw new Error(`Buffer ${name} doesn't exist in Plot: ${this.name}`) + } + const old_length = buffer.count + if (new_data.length <= old_length) { + buffer.set(new_data.data); + buffer.needsUpdate = true; + } else { + // if we have a larger size we need resizing + recreation of the buffer geometry + buffer.to_update = new_data.data; + this.geometry_needs_recreation = true; + } + } + + update_faces(face_data) { + this.mesh.geometry.setIndex(new THREE.BufferAttribute(face_data, 1)); + } +} + // global scene cache to look them up for dynamic operations in Makie // e.g. insert!(scene, plot) / delete!(scene, plot) const scene_cache = {}; @@ -139,130 +277,71 @@ export function deserialize_uniforms(scene, data) { return result; } -export function deserialize_plot(scene, data) { - let mesh; - const update_visible = (v) => { - mesh.visible = v; - // don't return anything, since that will disable on_update callback - return; - }; - if (data.plot_type === "lines") { - mesh = create_line(scene, data); - } else if (data.plot_type === "linesegments") { - mesh = create_linesegments(scene, data); - } else if ("instance_attributes" in data) { - mesh = create_instanced_mesh(scene, data); - } else { - mesh = create_mesh(scene, data); - } - mesh.name = data.name; - mesh.frustumCulled = false; - mesh.matrixAutoUpdate = false; - mesh.plot_uuid = data.uuid; - mesh.renderOrder = data.zvalue; - update_visible(data.visible.value); - data.visible.on(update_visible); - connect_uniforms(mesh, data.uniform_updater); - if (!(data.plot_type === "lines" || data.plot_type === "linesegments")) { - connect_attributes(mesh, data.attribute_updater); - } - return mesh; -} - const ON_NEXT_INSERT = new Set(); export function on_next_insert(f) { ON_NEXT_INSERT.add(f); } -export function add_plot(scene, plot_data) { +/** + * Connects a plot to a scene by setting up the necessary camera uniforms. + * + * @param {THREE.Scene} scene - The scene object containing the camera and screen information. + * @param {Plot} plot - The plot object to be connected to the scene. + */ +function connect_plot(scene, plot) { // fill in the camera uniforms, that we don't sent in serialization per plot const cam = scene.wgl_camera; const identity = new THREE.Uniform(new THREE.Matrix4()); - if (plot_data.cam_space == "data") { - plot_data.uniforms.view = cam.view; - plot_data.uniforms.projection = cam.projection; - plot_data.uniforms.projectionview = cam.projectionview; - plot_data.uniforms.eyeposition = cam.eyeposition; - } else if (plot_data.cam_space == "pixel") { - plot_data.uniforms.view = identity; - plot_data.uniforms.projection = cam.pixel_space; - plot_data.uniforms.projectionview = cam.pixel_space; - } else if (plot_data.cam_space == "relative") { - plot_data.uniforms.view = identity; - plot_data.uniforms.projection = cam.relative_space; - plot_data.uniforms.projectionview = cam.relative_space; - } else { + const uniforms = plot.mesh ? plot.mesh.material.uniforms : plot.plot_data.uniforms; + const space = plot.plot_data.cam_space; + if (space == "data") { + uniforms.view = cam.view; + uniforms.projection = cam.projection; + uniforms.projectionview = cam.projectionview; + uniforms.eyeposition = cam.eyeposition; + } else if (space == "pixel") { + uniforms.view = identity; + uniforms.projection = cam.pixel_space; + uniforms.projectionview = cam.pixel_space; + } else if (space == "relative") { + uniforms.view = identity; + uniforms.projection = cam.relative_space; + uniforms.projectionview = cam.relative_space; + } else if (space == "clip") { // clip space - plot_data.uniforms.view = identity; - plot_data.uniforms.projection = identity; - plot_data.uniforms.projectionview = identity; + uniforms.view = identity; + uniforms.projection = identity; + uniforms.projectionview = identity; + } else { + throw new Error(`Space ${space} not supported!`) } const { px_per_unit } = scene.screen; - plot_data.uniforms.resolution = cam.resolution; - plot_data.uniforms.px_per_unit = new THREE.Uniform(px_per_unit); + uniforms.resolution = cam.resolution; + uniforms.px_per_unit = new THREE.Uniform(px_per_unit); - if (plot_data.uniforms.preprojection) { - const { space, markerspace } = plot_data; - plot_data.uniforms.preprojection = cam.preprojection_matrix( + if (plot.plot_data.uniforms.preprojection) { + const { space, markerspace } = plot.plot_data; + uniforms.preprojection = cam.preprojection_matrix( space.value, markerspace.value ); } - if (scene.camera_relative_light) { - plot_data.uniforms.light_direction = cam.light_direction; - scene.light_direction.on((value) => { - cam.update_light_dir(value); - }); - } else { - // TODO how update? - const light_dir = new THREE.Vector3().fromArray( - scene.light_direction.value - ); - plot_data.uniforms.light_direction = new THREE.Uniform(light_dir); - scene.light_direction.on((value) => { - plot_data.uniforms.light_direction.value.fromArray(value); - }); - } + uniforms.light_direction = scene.light_direction; +} + - const p = deserialize_plot(scene, plot_data); - plot_cache[p.plot_uuid] = p; - scene.add(p); +export function add_plot(scene, plot_data) { + // fill in the camera uniforms, that we don't sent in serialization per plot + const p = new Plot(scene, plot_data); + plot_cache[p.uuid] = p.mesh; + scene.add(p.mesh); // execute all next insert callbacks const next_insert = new Set(ON_NEXT_INSERT); // copy next_insert.forEach((f) => f()); } -function connect_uniforms(mesh, updater) { - updater.on(([name, data]) => { - // this is the initial value, which shouldn't end up getting updated - - // TODO, figure out why this gets pushed!! - if (name === "none") { - return; - } - const uniform = mesh.material.uniforms[name]; - if (uniform.value.isTexture) { - const im_data = uniform.value.image; - const [size, tex_data] = data; - if (tex_data.length == im_data.data.length) { - im_data.data.set(tex_data); - } else { - const old_texture = uniform.value; - uniform.value = re_create_texture(old_texture, tex_data, size); - old_texture.dispose(); - } - uniform.value.needsUpdate = true; - } else { - if (is_three_fixed_array(uniform.value)) { - uniform.value.fromArray(data); - } else { - uniform.value = data; - } - } - }); -} - function convert_RGB_to_RGBA(rgbArray) { const length = rgbArray.length; const rgbaArray = new rgbArray.constructor((length / 3) * 4); @@ -365,6 +444,8 @@ function re_create_texture(old_texture, buffer, size) { } return tex; } + + function BufferAttribute(buffer) { const jsbuff = new THREE.BufferAttribute(buffer.flat, buffer.type_length); jsbuff.setUsage(THREE.DynamicDrawUsage); @@ -577,8 +658,6 @@ export function deserialize_scene(data, screen) { scene.backgroundcolor_alpha = data.backgroundcolor_alpha; scene.clearscene = data.clearscene; scene.visible = data.visible; - scene.camera_relative_light = data.camera_relative_light; - scene.light_direction = data.light_direction; const camera = new Camera.MakieCamera(); @@ -607,9 +686,23 @@ export function deserialize_scene(data, screen) { } update_cam(data.camera.value, true); // force update on first call + camera.update_light_dir(data.light_direction.value); data.camera.on(update_cam); + if (data.camera_relative_light) { + scene.light_direction = camera.light_direction; + } else { + const light_dir = new THREE.Vector3().fromArray( + data.light_direction.value + ); + scene.light_direction = new THREE.Uniform(light_dir); + data.light_direction.on((value) => { + plot_data.uniforms.light_direction.value.fromArray(value); + }); + } + + data.plots.forEach((plot_data) => { add_plot(scene, plot_data); }); diff --git a/WGLMakie/src/display.jl b/WGLMakie/src/display.jl index 73717f390f9..cc9be787c70 100644 --- a/WGLMakie/src/display.jl +++ b/WGLMakie/src/display.jl @@ -339,18 +339,23 @@ function insert_scene!(session::Session, screen::Screen, scene::Scene) end function insert_plot!(session::Session, scene::Scene, @nospecialize(plot::Plot)) + @assert !haskey(plot, :__wgl_session) plot_data = serialize_plots(scene, Plot[plot]) plot_sub = Session(session) Bonito.init_session(plot_sub) - plot.__wgl_session = plot_sub js = js""" $(WGL).then(WGL=> { WGL.insert_plot($(js_uuid(scene)), $plot_data); })""" Bonito.evaljs_value(plot_sub, js; timeout=50) + @assert !haskey(plot.attributes, :__wgl_session) + plot.attributes[:__wgl_session] = plot_sub return end +function Base.insert!(screen::Screen, scene::Scene, @nospecialize(plot::PlotList)) + return nothing +end function Base.insert!(screen::Screen, scene::Scene, @nospecialize(plot::Plot)) session = get_screen_session(screen; error="Plot needs to be displayed to insert additional plots") if js_uuid(scene) in screen.displayed_scenes diff --git a/WGLMakie/src/picking.jl b/WGLMakie/src/picking.jl index 93bf7a56d9d..bd8c36b0355 100644 --- a/WGLMakie/src/picking.jl +++ b/WGLMakie/src/picking.jl @@ -56,7 +56,10 @@ function Makie.pick_sorted(scene::Scene, screen::Screen, xy, range) # E.g. if websocket got closed isnothing(session) && return Tuple{Plot,Int}[] selection = Bonito.evaljs_value(session, js""" - Promise.all([$(WGL), $(scene)]).then(([WGL, scene]) => WGL.pick_sorted(scene, $(xy_vec), $(range))) + Promise.all([$(WGL), $(scene)]).then(([WGL, scene]) => { + const picked = WGL.pick_sorted(scene, $(xy_vec), $(range)) + return picked + }) """) isnothing(selection) && return Tuple{Plot,Int}[] lookup = plot_lookup(scene) @@ -68,6 +71,7 @@ function Makie.pick_sorted(scene::Scene, screen::Screen, xy, range) end function Makie.pick(::Scene, screen::Screen, xy) + plot_matrix = pick_native(screen, Rect2i(xy..., 1, 1)) return plot_matrix[1, 1] end diff --git a/WGLMakie/src/serialization.jl b/WGLMakie/src/serialization.jl index 64adb5bda0b..527e361750b 100644 --- a/WGLMakie/src/serialization.jl +++ b/WGLMakie/src/serialization.jl @@ -300,18 +300,20 @@ function serialize_scene(scene::Scene) light_dir = isnothing(dirlight) ? Observable(Vec3f(1)) : dirlight.direction cam_rel = isnothing(dirlight) ? false : dirlight.camera_relative - serialized = Dict(:viewport => pixel_area, - :backgroundcolor => lift(hexcolor, scene, scene.backgroundcolor), - :backgroundcolor_alpha => lift(Colors.alpha, scene, scene.backgroundcolor), - :clearscene => scene.clear, - :camera => serialize_camera(scene), - :light_direction => light_dir, - :camera_relative_light => cam_rel, - :plots => serialize_plots(scene, scene.plots), - :cam3d_state => cam3d_state, - :visible => scene.visible, - :uuid => js_uuid(scene), - :children => children) + serialized = Dict( + :viewport => pixel_area, + :backgroundcolor => lift(hexcolor, scene, scene.backgroundcolor), + :backgroundcolor_alpha => lift(Colors.alpha, scene, scene.backgroundcolor), + :clearscene => scene.clear, + :camera => serialize_camera(scene), + :light_direction => light_dir, + :camera_relative_light => cam_rel, + :plots => serialize_plots(scene, scene.plots), + :cam3d_state => cam3d_state, + :visible => scene.visible, + :uuid => js_uuid(scene), + :children => children + ) return serialized end diff --git a/WGLMakie/src/three_plot.jl b/WGLMakie/src/three_plot.jl index c44683adca5..11ac0e34ab7 100644 --- a/WGLMakie/src/three_plot.jl +++ b/WGLMakie/src/three_plot.jl @@ -72,3 +72,20 @@ function three_display(screen::Screen, session::Session, scene::Scene) connect_scene_events!(screen, scene, comm) return wrapper, done_init end + +Makie.supports_move_to(::Screen) = true + +function Makie.move_to!(screen::Screen, plot::Plot, scene::Scene) + session = get_screen_session(screen) + # Make sure target scene is serialized + insert_scene!(session, screen, scene) + return evaljs(session, js""" + $(scene).then(scene=> { + $(plot).then(meshes=> { + meshes.forEach(m => { + m.plot_object.move_to(scene) + }) + }) + }) + """) +end diff --git a/WGLMakie/src/wglmakie.bundled.js b/WGLMakie/src/wglmakie.bundled.js index a2e349a9981..89a9c13a822 100644 --- a/WGLMakie/src/wglmakie.bundled.js +++ b/WGLMakie/src/wglmakie.bundled.js @@ -21210,7 +21210,26 @@ class MakieCamera { } } } -const scene_cache = {}; +function update_uniform(uniform, new_value) { + if (uniform.value.isTexture) { + const im_data = uniform.value.image; + const [size, tex_data] = new_value; + if (tex_data.length == im_data.data.length) { + im_data.data.set(tex_data); + } else { + const old_texture = uniform.value; + uniform.value = re_create_texture(old_texture, tex_data, size); + old_texture.dispose(); + } + uniform.value.needsUpdate = true; + } else { + if (is_three_fixed_array(uniform.value)) { + uniform.value.fromArray(new_value); + } else { + uniform.value = new_value; + } + } +} function filter_by_key(dict, keys, default_value = false) { const result = {}; keys.forEach((key)=>{ @@ -21223,117 +21242,6 @@ function filter_by_key(dict, keys, default_value = false) { }); return result; } -const plot_cache = {}; -const TEXTURE_ATLAS = [ - undefined -]; -function add_scene(scene_id, three_scene) { - scene_cache[scene_id] = three_scene; -} -function find_scene(scene_id) { - return scene_cache[scene_id]; -} -function delete_scene(scene_id) { - const scene = scene_cache[scene_id]; - if (!scene) { - return; - } - delete_three_scene(scene); - while(scene.children.length > 0){ - scene.remove(scene.children[0]); - } - delete scene_cache[scene_id]; -} -function find_plots(plot_uuids) { - const plots = []; - plot_uuids.forEach((id)=>{ - const plot = plot_cache[id]; - if (plot) { - plots.push(plot); - } - }); - return plots; -} -function delete_scenes(scene_uuids, plot_uuids) { - plot_uuids.forEach((plot_id)=>{ - const plot = plot_cache[plot_id]; - if (plot) { - delete_plot(plot); - } - }); - scene_uuids.forEach((scene_id)=>{ - delete_scene(scene_id); - }); -} -function insert_plot(scene_id, plot_data) { - const scene = find_scene(scene_id); - plot_data.forEach((plot)=>{ - add_plot(scene, plot); - }); -} -function delete_plots(plot_uuids) { - const plots = find_plots(plot_uuids); - plots.forEach(delete_plot); -} -function convert_texture(scene, data) { - const tex = create_texture(scene, data); - tex.needsUpdate = true; - tex.minFilter = mod[data.minFilter]; - tex.magFilter = mod[data.magFilter]; - tex.anisotropy = data.anisotropy; - tex.wrapS = mod[data.wrapS]; - if (data.size.length > 1) { - tex.wrapT = mod[data.wrapT]; - } - if (data.size.length > 2) { - tex.wrapR = mod[data.wrapR]; - } - return tex; -} -function is_three_fixed_array(value) { - return value instanceof mod.Vector2 || value instanceof mod.Vector3 || value instanceof mod.Vector4 || value instanceof mod.Matrix4; -} -function to_uniform(scene, data) { - if (data.type !== undefined) { - if (data.type == "Sampler") { - return convert_texture(scene, data); - } - throw new Error(`Type ${data.type} not known`); - } - if (Array.isArray(data) || ArrayBuffer.isView(data)) { - if (!data.every((x)=>typeof x === "number")) { - return data; - } - if (data.length == 2) { - return new mod.Vector2().fromArray(data); - } - if (data.length == 3) { - return new mod.Vector3().fromArray(data); - } - if (data.length == 4) { - return new mod.Vector4().fromArray(data); - } - if (data.length == 16) { - const mat = new mod.Matrix4(); - mat.fromArray(data); - return mat; - } - } - return data; -} -function deserialize_uniforms(scene, data) { - const result = {}; - for(const name in data){ - const value = data[name]; - if (value instanceof mod.Uniform) { - result[name] = value; - } else { - const ser = to_uniform(scene, value); - result[name] = new mod.Uniform(ser); - } - } - return result; -} function lines_vertex_shader(uniforms, attributes, is_linesegments) { const attribute_decl = attributes_to_type_declaration(attributes); const uniform_decl = uniforms_to_type_declaration(uniforms); @@ -22301,37 +22209,20 @@ function lines_fragment_shader(uniforms, attributes) { } `; } -function create_line_material(scene, uniforms, attributes, is_linesegments) { - const uniforms_des = deserialize_uniforms(scene, uniforms); - const mat = new THREE.RawShaderMaterial({ - uniforms: uniforms_des, - glslVersion: THREE.GLSL3, - vertexShader: lines_vertex_shader(uniforms_des, attributes, is_linesegments), - fragmentShader: lines_fragment_shader(uniforms_des, attributes), - transparent: true, - blending: THREE.CustomBlending, - blendSrc: THREE.SrcAlphaFactor, - blendDst: THREE.OneMinusSrcAlphaFactor, - blendSrcAlpha: THREE.ZeroFactor, - blendDstAlpha: THREE.OneFactor, - blendEquation: THREE.AddEquation - }); - mat.uniforms.object_id = { - value: 1 - }; - return mat; +function create_line(scene, line_data) { + return _create_line(scene, line_data, false); } function attach_interleaved_line_buffer(attr_name, geometry, data, ndim, is_segments, is_position) { const skip_elems = is_segments ? 2 * ndim : ndim; - const buffer = new THREE.InstancedInterleavedBuffer(data, skip_elems, 1); - buffer.count = Math.max(0, is_segments ? Math.floor(buffer.count - 1) : buffer.count - 3); - geometry.setAttribute(attr_name + "_start", new THREE.InterleavedBufferAttribute(buffer, ndim, ndim)); - geometry.setAttribute(attr_name + "_end", new THREE.InterleavedBufferAttribute(buffer, ndim, 2 * ndim)); + const buffer1 = new THREE.InstancedInterleavedBuffer(data, skip_elems, 1); + buffer1.count = Math.max(0, is_segments ? Math.floor(buffer1.count - 1) : buffer1.count - 3); + geometry.setAttribute(attr_name + "_start", new THREE.InterleavedBufferAttribute(buffer1, ndim, ndim)); + geometry.setAttribute(attr_name + "_end", new THREE.InterleavedBufferAttribute(buffer1, ndim, 2 * ndim)); if (is_position) { - geometry.setAttribute(attr_name + "_prev", new THREE.InterleavedBufferAttribute(buffer, ndim, 0)); - geometry.setAttribute(attr_name + "_next", new THREE.InterleavedBufferAttribute(buffer, ndim, 3 * ndim)); + geometry.setAttribute(attr_name + "_prev", new THREE.InterleavedBufferAttribute(buffer1, ndim, 0)); + geometry.setAttribute(attr_name + "_next", new THREE.InterleavedBufferAttribute(buffer1, ndim, 3 * ndim)); } - return buffer; + return buffer1; } function create_line_instance_geometry() { const geometry = new THREE.InstancedBufferGeometry(); @@ -22403,116 +22294,274 @@ function _create_line(scene, line_data, is_segments) { attach_updates(mesh, buffers, line_data.attributes, is_segments); return mesh; } -function create_line(scene, line_data) { - return _create_line(scene, line_data, false); -} function create_linesegments(scene, line_data) { return _create_line(scene, line_data, true); } -function deserialize_plot(scene, data) { - let mesh; - const update_visible = (v)=>{ - mesh.visible = v; +class Plot { + mesh = undefined; + parent = undefined; + uuid = ""; + name = ""; + is_instanced = false; + geometry_needs_recreation = false; + plot_data = {}; + constructor(scene, data){ + this.plot_data = data; + connect_plot(scene, this); + if (data.plot_type === "lines") { + this.mesh = create_line(scene, this.plot_data); + } else if (data.plot_type === "linesegments") { + this.mesh = create_linesegments(scene, this.plot_data); + } else if ("instance_attributes" in data) { + this.is_instanced = true; + this.mesh = create_instanced_mesh(scene, this.plot_data); + } else { + this.mesh = create_mesh(scene, this.plot_data); + } + this.name = data.name; + this.uuid = data.uuid; + this.mesh.plot_uuid = data.uuid; + this.mesh.frustumCulled = false; + this.mesh.matrixAutoUpdate = false; + this.mesh.renderOrder = data.zvalue; + data.uniform_updater.on(([name, data])=>{ + this.update_uniform(name, data); + }); + if (!(data.plot_type === "lines" || data.plot_type === "linesegments")) { + connect_attributes(this.mesh, data.attribute_updater); + } + this.parent = scene; + this.mesh.plot_object = this; + this.mesh.visible = data.visible.value; + data.visible.on((v)=>{ + this.mesh.visible = v; + }); + } + move_to(scene) { + if (scene === this.parent) { + return; + } + this.parent.remove(this.mesh); + connect_plot(scene, this); + scene.add(this.mesh); + this.parent = scene; return; - }; - if (data.plot_type === "lines") { - mesh = create_line(scene, data); - } else if (data.plot_type === "linesegments") { - mesh = create_linesegments(scene, data); - } else if ("instance_attributes" in data) { - mesh = create_instanced_mesh(scene, data); - } else { - mesh = create_mesh(scene, data); - } - mesh.name = data.name; - mesh.frustumCulled = false; - mesh.matrixAutoUpdate = false; - mesh.plot_uuid = data.uuid; - mesh.renderOrder = data.zvalue; - update_visible(data.visible.value); - data.visible.on(update_visible); - connect_uniforms(mesh, data.uniform_updater); - if (!(data.plot_type === "lines" || data.plot_type === "linesegments")) { - connect_attributes(mesh, data.attribute_updater); } - return mesh; + update(attributes) { + attributes.keys().forEach((key)=>{ + const value = attributes[key]; + if (value.type == "uniform") { + this.update_uniform(key, value.data); + } else if (value.type === "geometry") { + this.update_geometries(value.data); + } else if (value.type === "faces") { + this.update_faces(value.data); + } + }); + this.apply_updates(); + } + update_uniform(name, new_data) { + const uniform = this.mesh.material.uniforms[name]; + if (!uniform) { + throw new Error(`Uniform ${name} doesn't exist in Plot: ${this.name}`); + } + update_uniform(uniform, new_data); + } + update_geometry(name, new_data) { + buffer = this.mesh.geometry.attributes[name]; + if (!buffer) { + throw new Error(`Buffer ${name} doesn't exist in Plot: ${this.name}`); + } + const old_length = buffer.count; + if (new_data.length <= old_length) { + buffer.set(new_data.data); + buffer.needsUpdate = true; + } else { + buffer.to_update = new_data.data; + this.geometry_needs_recreation = true; + } + } + update_faces(face_data) { + this.mesh.geometry.setIndex(new mod.BufferAttribute(face_data, 1)); + } +} +const scene_cache = {}; +const plot_cache = {}; +const TEXTURE_ATLAS = [ + undefined +]; +function add_scene(scene_id, three_scene) { + scene_cache[scene_id] = three_scene; +} +function find_scene(scene_id) { + return scene_cache[scene_id]; +} +function delete_scene(scene_id) { + const scene = scene_cache[scene_id]; + if (!scene) { + return; + } + delete_three_scene(scene); + while(scene.children.length > 0){ + scene.remove(scene.children[0]); + } + delete scene_cache[scene_id]; +} +function find_plots(plot_uuids) { + const plots = []; + plot_uuids.forEach((id)=>{ + const plot = plot_cache[id]; + if (plot) { + plots.push(plot); + } + }); + return plots; +} +function delete_scenes(scene_uuids, plot_uuids) { + plot_uuids.forEach((plot_id)=>{ + const plot = plot_cache[plot_id]; + if (plot) { + delete_plot(plot); + } + }); + scene_uuids.forEach((scene_id)=>{ + delete_scene(scene_id); + }); +} +function insert_plot(scene_id, plot_data1) { + const scene = find_scene(scene_id); + plot_data1.forEach((plot)=>{ + add_plot(scene, plot); + }); +} +function delete_plots(plot_uuids) { + const plots = find_plots(plot_uuids); + plots.forEach(delete_plot); +} +function convert_texture(scene, data) { + const tex = create_texture(scene, data); + tex.needsUpdate = true; + tex.minFilter = mod[data.minFilter]; + tex.magFilter = mod[data.magFilter]; + tex.anisotropy = data.anisotropy; + tex.wrapS = mod[data.wrapS]; + if (data.size.length > 1) { + tex.wrapT = mod[data.wrapT]; + } + if (data.size.length > 2) { + tex.wrapR = mod[data.wrapR]; + } + return tex; +} +function is_three_fixed_array(value) { + return value instanceof mod.Vector2 || value instanceof mod.Vector3 || value instanceof mod.Vector4 || value instanceof mod.Matrix4; +} +function to_uniform(scene, data) { + if (data.type !== undefined) { + if (data.type == "Sampler") { + return convert_texture(scene, data); + } + throw new Error(`Type ${data.type} not known`); + } + if (Array.isArray(data) || ArrayBuffer.isView(data)) { + if (!data.every((x)=>typeof x === "number")) { + return data; + } + if (data.length == 2) { + return new mod.Vector2().fromArray(data); + } + if (data.length == 3) { + return new mod.Vector3().fromArray(data); + } + if (data.length == 4) { + return new mod.Vector4().fromArray(data); + } + if (data.length == 16) { + const mat = new mod.Matrix4(); + mat.fromArray(data); + return mat; + } + } + return data; +} +function deserialize_uniforms(scene, data) { + const result = {}; + for(const name in data){ + const value = data[name]; + if (value instanceof mod.Uniform) { + result[name] = value; + } else { + const ser = to_uniform(scene, value); + result[name] = new mod.Uniform(ser); + } + } + return result; +} +function create_line_material(scene, uniforms, attributes, is_linesegments) { + const uniforms_des = deserialize_uniforms(scene, uniforms); + const mat = new THREE.RawShaderMaterial({ + uniforms: uniforms_des, + glslVersion: THREE.GLSL3, + vertexShader: lines_vertex_shader(uniforms_des, attributes, is_linesegments), + fragmentShader: lines_fragment_shader(uniforms_des, attributes), + transparent: true, + blending: THREE.CustomBlending, + blendSrc: THREE.SrcAlphaFactor, + blendDst: THREE.OneMinusSrcAlphaFactor, + blendSrcAlpha: THREE.ZeroFactor, + blendDstAlpha: THREE.OneFactor, + blendEquation: THREE.AddEquation + }); + mat.uniforms.object_id = { + value: 1 + }; + return mat; } const ON_NEXT_INSERT = new Set(); function on_next_insert(f) { ON_NEXT_INSERT.add(f); } -function add_plot(scene, plot_data) { +function connect_plot(scene, plot) { const cam = scene.wgl_camera; const identity = new mod.Uniform(new mod.Matrix4()); - if (plot_data.cam_space == "data") { - plot_data.uniforms.view = cam.view; - plot_data.uniforms.projection = cam.projection; - plot_data.uniforms.projectionview = cam.projectionview; - plot_data.uniforms.eyeposition = cam.eyeposition; - } else if (plot_data.cam_space == "pixel") { - plot_data.uniforms.view = identity; - plot_data.uniforms.projection = cam.pixel_space; - plot_data.uniforms.projectionview = cam.pixel_space; - } else if (plot_data.cam_space == "relative") { - plot_data.uniforms.view = identity; - plot_data.uniforms.projection = cam.relative_space; - plot_data.uniforms.projectionview = cam.relative_space; + const uniforms = plot.mesh ? plot.mesh.material.uniforms : plot.plot_data.uniforms; + const space = plot.plot_data.cam_space; + if (space == "data") { + uniforms.view = cam.view; + uniforms.projection = cam.projection; + uniforms.projectionview = cam.projectionview; + uniforms.eyeposition = cam.eyeposition; + } else if (space == "pixel") { + uniforms.view = identity; + uniforms.projection = cam.pixel_space; + uniforms.projectionview = cam.pixel_space; + } else if (space == "relative") { + uniforms.view = identity; + uniforms.projection = cam.relative_space; + uniforms.projectionview = cam.relative_space; + } else if (space == "clip") { + uniforms.view = identity; + uniforms.projection = identity; + uniforms.projectionview = identity; } else { - plot_data.uniforms.view = identity; - plot_data.uniforms.projection = identity; - plot_data.uniforms.projectionview = identity; + throw new Error(`Space ${space} not supported!`); } const { px_per_unit } = scene.screen; - plot_data.uniforms.resolution = cam.resolution; - plot_data.uniforms.px_per_unit = new mod.Uniform(px_per_unit); - if (plot_data.uniforms.preprojection) { - const { space , markerspace } = plot_data; - plot_data.uniforms.preprojection = cam.preprojection_matrix(space.value, markerspace.value); - } - if (scene.camera_relative_light) { - plot_data.uniforms.light_direction = cam.light_direction; - scene.light_direction.on((value)=>{ - cam.update_light_dir(value); - }); - } else { - const light_dir = new mod.Vector3().fromArray(scene.light_direction.value); - plot_data.uniforms.light_direction = new mod.Uniform(light_dir); - scene.light_direction.on((value)=>{ - plot_data.uniforms.light_direction.value.fromArray(value); - }); - } - const p = deserialize_plot(scene, plot_data); - plot_cache[p.plot_uuid] = p; - scene.add(p); + uniforms.resolution = cam.resolution; + uniforms.px_per_unit = new mod.Uniform(px_per_unit); + if (plot.plot_data.uniforms.preprojection) { + const { space , markerspace } = plot.plot_data; + uniforms.preprojection = cam.preprojection_matrix(space.value, markerspace.value); + } + uniforms.light_direction = scene.light_direction; +} +function add_plot(scene, plot_data1) { + const p = new Plot(scene, plot_data1); + plot_cache[p.uuid] = p.mesh; + scene.add(p.mesh); const next_insert = new Set(ON_NEXT_INSERT); next_insert.forEach((f)=>f()); } -function connect_uniforms(mesh, updater) { - updater.on(([name, data])=>{ - if (name === "none") { - return; - } - const uniform = mesh.material.uniforms[name]; - if (uniform.value.isTexture) { - const im_data = uniform.value.image; - const [size, tex_data] = data; - if (tex_data.length == im_data.data.length) { - im_data.data.set(tex_data); - } else { - const old_texture = uniform.value; - uniform.value = re_create_texture(old_texture, tex_data, size); - old_texture.dispose(); - } - uniform.value.needsUpdate = true; - } else { - if (is_three_fixed_array(uniform.value)) { - uniform.value.fromArray(data); - } else { - uniform.value = data; - } - } - }); -} function convert_RGB_to_RGBA(rgbArray) { const length = rgbArray.length; const rgbaArray = new rgbArray.constructor(length / 3 * 4); @@ -22525,24 +22574,24 @@ function convert_RGB_to_RGBA(rgbArray) { return rgbaArray; } function create_texture_from_data(data) { - let buffer = data.data; + let buffer1 = data.data; if (data.size.length == 3) { - const tex = new mod.Data3DTexture(buffer, data.size[0], data.size[1], data.size[2]); + const tex = new mod.Data3DTexture(buffer1, data.size[0], data.size[1], data.size[2]); tex.format = mod[data.three_format]; tex.type = mod[data.three_type]; return tex; } else { let format = mod[data.three_format]; if (data.three_format == "RGBFormat") { - buffer = convert_RGB_to_RGBA(buffer); + buffer1 = convert_RGB_to_RGBA(buffer1); format = mod.RGBAFormat; } - return new mod.DataTexture(buffer, data.size[0], data.size[1], format, mod[data.three_type]); + return new mod.DataTexture(buffer1, data.size[0], data.size[1], format, mod[data.three_type]); } } function create_texture(scene, data) { - const buffer = data.data; - if (buffer == "texture_atlas") { + const buffer1 = data.data; + if (buffer1 == "texture_atlas") { const { texture_atlas } = scene.screen; if (texture_atlas) { return texture_atlas; @@ -22565,14 +22614,14 @@ function create_texture(scene, data) { return create_texture_from_data(data); } } -function re_create_texture(old_texture, buffer, size) { +function re_create_texture(old_texture, buffer1, size) { let tex; if (size.length == 3) { - tex = new mod.Data3DTexture(buffer, size[0], size[1], size[2]); + tex = new mod.Data3DTexture(buffer1, size[0], size[1], size[2]); tex.format = old_texture.format; tex.type = old_texture.type; } else { - tex = new mod.DataTexture(buffer, size[0], size[1] ? size[1] : 1, old_texture.format, old_texture.type); + tex = new mod.DataTexture(buffer1, size[0], size[1] ? size[1] : 1, old_texture.format, old_texture.type); } tex.minFilter = old_texture.minFilter; tex.magFilter = old_texture.magFilter; @@ -22586,26 +22635,26 @@ function re_create_texture(old_texture, buffer, size) { } return tex; } -function BufferAttribute(buffer) { - const jsbuff = new mod.BufferAttribute(buffer.flat, buffer.type_length); +function BufferAttribute(buffer1) { + const jsbuff = new mod.BufferAttribute(buffer1.flat, buffer1.type_length); jsbuff.setUsage(mod.DynamicDrawUsage); return jsbuff; } -function InstanceBufferAttribute(buffer) { - const jsbuff = new mod.InstancedBufferAttribute(buffer.flat, buffer.type_length); +function InstanceBufferAttribute(buffer1) { + const jsbuff = new mod.InstancedBufferAttribute(buffer1.flat, buffer1.type_length); jsbuff.setUsage(mod.DynamicDrawUsage); return jsbuff; } function attach_geometry(buffer_geometry, vertexarrays, faces) { for(const name in vertexarrays){ const buff = vertexarrays[name]; - let buffer; + let buffer1; if (buff.to_update) { - buffer = new mod.BufferAttribute(buff.to_update, buff.itemSize); + buffer1 = new mod.BufferAttribute(buff.to_update, buff.itemSize); } else { - buffer = BufferAttribute(buff); + buffer1 = BufferAttribute(buff); } - buffer_geometry.setAttribute(name, buffer); + buffer_geometry.setAttribute(name, buffer1); } buffer_geometry.setIndex(faces); buffer_geometry.boundingSphere = new mod.Sphere(); @@ -22615,8 +22664,8 @@ function attach_geometry(buffer_geometry, vertexarrays, faces) { } function attach_instanced_geometry(buffer_geometry, instance_attributes) { for(const name in instance_attributes){ - const buffer = InstanceBufferAttribute(instance_attributes[name]); - buffer_geometry.setAttribute(name, buffer); + const buffer1 = InstanceBufferAttribute(instance_attributes[name]); + buffer_geometry.setAttribute(name, buffer1); } } function recreate_geometry(mesh, vertexarrays, faces) { @@ -22634,17 +22683,17 @@ function recreate_instanced_geometry(mesh) { ...mesh.geometry.index.array ]; Object.keys(mesh.geometry.attributes).forEach((name)=>{ - const buffer = mesh.geometry.attributes[name]; - const copy = buffer.to_update ? buffer.to_update : buffer.array.map((x)=>x); - if (buffer.isInstancedBufferAttribute) { + const buffer1 = mesh.geometry.attributes[name]; + const copy = buffer1.to_update ? buffer1.to_update : buffer1.array.map((x)=>x); + if (buffer1.isInstancedBufferAttribute) { instance_attributes[name] = { flat: copy, - type_length: buffer.itemSize + type_length: buffer1.itemSize }; } else { vertexarrays[name] = { flat: copy, - type_length: buffer.itemSize + type_length: buffer1.itemSize }; } }); @@ -22707,11 +22756,11 @@ function connect_attributes(mesh, updater) { function re_assign_buffers() { const attributes = mesh.geometry.attributes; Object.keys(attributes).forEach((name)=>{ - const buffer = attributes[name]; - if (buffer.isInstancedBufferAttribute) { - instance_buffers[name] = buffer; + const buffer1 = attributes[name]; + if (buffer1.isInstancedBufferAttribute) { + instance_buffers[name] = buffer1; } else { - geometry_buffers[name] = buffer; + geometry_buffers[name] = buffer1; } }); first_instance_buffer = first(instance_buffers); @@ -22723,7 +22772,7 @@ function connect_attributes(mesh, updater) { } re_assign_buffers(); updater.on(([name, new_values, length])=>{ - const buffer = mesh.geometry.attributes[name]; + const buffer1 = mesh.geometry.attributes[name]; let buffers; let real_length; let is_instance = false; @@ -22738,19 +22787,19 @@ function connect_attributes(mesh, updater) { real_length = real_geometry_length; } if (length <= real_length[0]) { - buffer.set(new_values); - buffer.needsUpdate = true; + buffer1.set(new_values); + buffer1.needsUpdate = true; if (is_instance) { mesh.geometry.instanceCount = length; } } else { - buffer.to_update = new_values; + buffer1.to_update = new_values; const all_have_same_length = Object.values(buffers).every((x)=>x.to_update && x.to_update.length / x.itemSize == length); if (all_have_same_length) { if (is_instance) { recreate_instanced_geometry(mesh); re_assign_buffers(); - mesh.geometry.instanceCount = new_values.length / buffer.itemSize; + mesh.geometry.instanceCount = new_values.length / buffer1.itemSize; } else { recreate_geometry(mesh, buffers, mesh.geometry.index); re_assign_buffers(); @@ -22771,8 +22820,6 @@ function deserialize_scene(data, screen) { scene.backgroundcolor_alpha = data.backgroundcolor_alpha; scene.clearscene = data.clearscene; scene.visible = data.visible; - scene.camera_relative_light = data.camera_relative_light; - scene.light_direction = data.light_direction; const camera = new MakieCamera(); scene.wgl_camera = camera; function update_cam(camera_matrices, force) { @@ -22790,8 +22837,17 @@ function deserialize_scene(data, screen) { update_cam(data.camera.value, true); camera.update_light_dir(data.light_direction.value); data.camera.on(update_cam); - data.plots.forEach((plot_data)=>{ - add_plot(scene, plot_data); + if (data.camera_relative_light) { + scene.light_direction = camera.light_direction; + } else { + const light_dir = new mod.Vector3().fromArray(data.light_direction.value); + scene.light_direction = new mod.Uniform(light_dir); + data.light_direction.on((value)=>{ + plot_data.uniforms.light_direction.value.fromArray(value); + }); + } + data.plots.forEach((plot_data1)=>{ + add_plot(scene, plot_data1); }); scene.scene_children = data.children.map((child)=>{ const childscene = deserialize_scene(child, screen); @@ -23278,8 +23334,8 @@ function pick_closest(scene, xy, range) { const y1 = Math.min(canvas.height, Math.ceil(px_per_unit * (xy[1] + range))); const dx = x1 - x0; const dy = y1 - y0; - const [plot_data, _] = pick_native(scene, x0, y0, dx, dy, false); - const plot_matrix = plot_data.data; + const [plot_data1, _] = pick_native(scene, x0, y0, dx, dy, false); + const plot_matrix = plot_data1.data; let min_dist = px_per_unit * px_per_unit * range * range; let selection = [ null, @@ -23319,11 +23375,11 @@ function pick_sorted(scene, xy, range) { const y1 = Math.min(canvas.height, Math.ceil(px_per_unit * (xy[1] + range))); const dx = x1 - x0; const dy = y1 - y0; - const [plot_data, selected] = pick_native(scene, x0, y0, dx, dy, false); + const [plot_data1, selected] = pick_native(scene, x0, y0, dx, dy, false); if (selected.length == 0) { return null; } - const plot_matrix = plot_data.data; + const plot_matrix = plot_data1.data; const distances = selected.map((x)=>1e30); const x = xy[0] * px_per_unit + 1 - x0; const y = xy[1] * px_per_unit + 1 - y0; diff --git a/WGLMakie/test/runtests.jl b/WGLMakie/test/runtests.jl index a54d8ebce69..f97f4463979 100644 --- a/WGLMakie/test/runtests.jl +++ b/WGLMakie/test/runtests.jl @@ -22,7 +22,7 @@ excludes = Set([ "Image on Surface Sphere", # TODO: texture rotated 180° # "heatmaps & surface", # TODO: fix direct NaN -> nancolor conversion "Array of Images Scatter", # scatter does not support texture images - + "Order Independent Transparency", "fast pixel marker", "Textured meshscatter", # not yet implemented @@ -52,6 +52,8 @@ end session = edisplay.browserdisplay.handler.session session_size = Base.summarysize(session) / 10^6 texture_atlas_size = Base.summarysize(WGLMakie.TEXTURE_ATLAS) / 10^6 + @show typeof.(last.(WGLMakie.TEXTURE_ATLAS.listeners)) + @show length(WGLMakie.TEXTURE_ATLAS.listeners) @show session_size texture_atlas_size @test session_size / 10^6 < 6 @test texture_atlas_size < 6 @@ -91,7 +93,7 @@ end rm(filename) end - + f, a, p = scatter(rand(10)); filename = "$(tempname()).mp4" try @@ -124,4 +126,4 @@ end @test events(f).tick[] == tick # TODO: test normal rendering -end \ No newline at end of file +end diff --git a/src/interaction/inspector.jl b/src/interaction/inspector.jl index f5e166ef393..8e3baed0fd6 100644 --- a/src/interaction/inspector.jl +++ b/src/interaction/inspector.jl @@ -184,23 +184,20 @@ mutable struct DataInspector root::Scene attributes::Attributes - temp_plots::Vector{AbstractPlot} + temp_plots::Vector{Plot} plot::Tooltip - selection::AbstractPlot + selection::Plot obsfuncs::Vector{Any} - lock::Threads.ReentrantLock end function DataInspector(scene::Scene, plot::AbstractPlot, attributes) - x = DataInspector(scene, attributes, AbstractPlot[], plot, plot, Any[], Threads.ReentrantLock()) - # finalizer(cleanup, x) # doesn't get triggered when this is dereferenced - x + return DataInspector(scene, attributes, Plot[], plot, plot, Any[]) end function cleanup(inspector::DataInspector) - off.(inspector.obsfuncs) + foreach(off, inspector.obsfuncs) empty!(inspector.obsfuncs) delete!(inspector.root, inspector.plot) clear_temporary_plots!(inspector, inspector.selection) @@ -281,9 +278,17 @@ function DataInspector(scene::Scene; priority = 100, kwargs...) # We delegate the hover processing to another channel, # So that we can skip queued up updates with empty_channel! # And also not slow down the processing of e.mouseposition/e.scroll + was_open = false channel = Channel{Nothing}(Inf) do ch for _ in ch - on_hover(inspector) + if isopen(scene) + was_open = true + on_hover(inspector) + end + if !isopen(scene) && was_open + close(ch) + break + end end end listeners = onany(e.mouseposition, e.scroll) do _, _ @@ -306,33 +311,31 @@ DataInspector(; kwargs...) = DataInspector(current_figure(); kwargs...) function on_hover(inspector) parent = inspector.root - lock(inspector.lock) do - (inspector.attributes.enabled[] && is_mouseinside(parent)) || return Consume(false) - - mp = mouseposition_px(parent) - should_clear = true - for (plt, idx) in pick_sorted(parent, mp, inspector.attributes.range[]) - if to_value(get(plt.attributes, :inspectable, true)) - # show_data should return true if it created a tooltip - if show_data_recursion(inspector, plt, idx) - should_clear = false - break - end + (inspector.attributes.enabled[] && is_mouseinside(parent)) || return Consume(false) + + mp = mouseposition_px(parent) + should_clear = true + for (plt, idx) in pick_sorted(parent, mp, inspector.attributes.range[]) + if to_value(get(plt.attributes, :inspectable, true)) + # show_data should return true if it created a tooltip + if show_data_recursion(inspector, plt, idx) + should_clear = false + break end end + end - if should_clear - plot = inspector.selection - if to_value(get(plot, :inspector_clear, automatic)) !== automatic - plot[:inspector_clear][](inspector, plot) - end - inspector.plot.visible[] = false - inspector.attributes.indicator_visible[] = false - inspector.plot.offset.val = inspector.attributes.offset[] + if should_clear + plot = inspector.selection + if to_value(get(plot, :inspector_clear, automatic)) !== automatic + plot[:inspector_clear][](inspector, plot) end - - return Consume(false) + inspector.plot.visible[] = false + inspector.attributes.indicator_visible[] = false + inspector.plot.offset.val = inspector.attributes.offset[] end + + return Consume(false) end diff --git a/src/layouting/transformation.jl b/src/layouting/transformation.jl index d1ee9652edf..1bb59e62a8e 100644 --- a/src/layouting/transformation.jl +++ b/src/layouting/transformation.jl @@ -15,11 +15,13 @@ end function Observables.connect!(parent::Transformation, child::Transformation; connect_func=true) tfuncs = [] + # Observables.clear(child.parent_model) obsfunc = on(parent.model; update=true) do m return child.parent_model[] = m end push!(tfuncs, obsfunc) if connect_func + # Observables.clear(child.transform_func) t2 = on(parent.transform_func; update=true) do f child.transform_func[] = f return diff --git a/src/scenes.jl b/src/scenes.jl index 5903e5a9e7c..fd1f12d16de 100644 --- a/src/scenes.jl +++ b/src/scenes.jl @@ -498,15 +498,13 @@ function free(plot::AbstractPlot) Observables.off(f) end foreach(free, plot.plots) - empty!(plot.plots) + # empty!(plot.plots) empty!(plot.deregister_callbacks) - empty!(plot.attributes) free(plot.transformation) return end function Base.delete!(scene::Scene, plot::AbstractPlot) - len = length(scene.plots) filter!(x -> x !== plot, scene.plots) # TODO, if we want to delete a subplot of a plot, # It won't be in scene.plots directly, but will still be deleted @@ -523,6 +521,46 @@ function Base.delete!(scene::Scene, plot::AbstractPlot) free(plot) end +supports_move_to(::MakieScreen) = false + +function supports_move_to(plot::Plot) + scene = get_scene(plot) + return all(scene.current_screens) do screen + return supports_move_to(screen) + end +end + +# function move_to!(screen::MakieScreen, plot::Plot, scene::Scene) +# # TODO, move without deleting! +# # Will be easier with Observable refactor +# delete!(screen, scene, plot) +# insert!(screen, scene, plot) +# return +# end + + +function move_to!(plot::Plot, scene::Scene) + if plot.parent === scene + return + end + + if is_space_compatible(plot, scene) + obsfunc = connect!(transformation(scene), transformation(plot)) + append!(plot.deregister_callbacks, obsfunc) + end + for screen in root(scene).current_screens + if supports_move_to(screen) + move_to!(screen, plot, scene) + end + end + current_parent = parent_scene(plot) + filter!(x -> x !== plot, current_parent.plots) + push!(scene.plots, plot) + plot.parent = scene + return +end + + events(x) = events(get_scene(x)) events(scene::Scene) = scene.events events(scene::SceneLike) = events(scene.parent) diff --git a/src/specapi.jl b/src/specapi.jl index 8f4bf7dd11e..5d03e321363 100644 --- a/src/specapi.jl +++ b/src/specapi.jl @@ -326,12 +326,13 @@ function distance_score(at::Tuple{Int,GP,GridLayoutSpec}, bt::Tuple{Int,GP,GridL end end -function find_min_distance(f, to_compare, list, scores) +function find_min_distance(f, to_compare, list, scores, penalty=(key, score)-> score) isempty(list) && return -1 minscore = 2.0 idx = -1 for key in keys(list) score = distance_score(to_compare, f(list[key], key), scores) + score = penalty(key, score) # apply custom penalty if score ≈ 0.0 # shortcuircit for exact matches return key end @@ -353,8 +354,15 @@ function find_layoutable( return (idx, layoutables[idx]...) end -function find_reusable_plot(plotspec::PlotSpec, plots::IdDict{PlotSpec,Plot}, scores) - idx = find_min_distance((_, spec) -> spec, plotspec, plots, scores) +function find_reusable_plot(scene::Scene, plotspec::PlotSpec, plots::IdDict{PlotSpec,Plot}, scores) + function penalty(key, score) + # penalize plots with different parents + # needs to be implemented via this penalty function, since parent scenes arent part of the spec + plot = plots[key] + move_to_penalty = ((!Makie.supports_move_to(plot)) * 100) + 1 + return norm(Float64[plot.parent !== scene, score]) * move_to_penalty + end + idx = find_min_distance((_, spec) -> spec, plotspec, plots, scores, penalty) idx == -1 && return nothing, nothing return plots[idx], idx end @@ -564,18 +572,21 @@ function push_without_add!(scene::Scene, plot) end end -function diff_plotlist!(scene::Scene, plotspecs::Vector{PlotSpec}, obs_to_notify, reusable_plots, - plotlist::Union{Nothing,PlotList}=nothing) - new_plots = IdDict{PlotSpec,Plot}() # needed to be mutated +function diff_plotlist!( + scene::Scene, plotspecs::Vector{PlotSpec}, + obs_to_notify, + plotlist::Union{Nothing,PlotList}=nothing, + reusable_plots = IdDict{PlotSpec, Plot}(), + new_plots = IdDict{PlotSpec,Plot}()) + # needed to be mutated empty!(scene.cycler.counters) # Global list of observables that need updating # Updating them all at once in the end avoids problems with triggering updates while updating # And at some point we may be able to optimize notify(list_of_observables) - empty!(obs_to_notify) scores = IdDict{Any, Float64}() for plotspec in plotspecs # we need to compare by types with compare_specs, since we can only update plots if the types of all attributes match - reused_plot, old_spec = find_reusable_plot(plotspec, reusable_plots, scores) + reused_plot, old_spec = find_reusable_plot(scene, plotspec, reusable_plots, scores) if isnothing(reused_plot) # Create new plot, store it into our `cached_plots` dictionary @debug("Creating new plot for spec") @@ -601,45 +612,57 @@ function diff_plotlist!(scene::Scene, plotspecs::Vector{PlotSpec}, obs_to_notify @debug("updating old plot with spec") # Delete the plots from reusable_plots, so that we don't re-use it multiple times! delete!(reusable_plots, old_spec) + if reused_plot.parent !== scene + @assert Makie.supports_move_to(reused_plot) + move_to!(reused_plot, scene) + end update_plot!(obs_to_notify, reused_plot, old_spec, plotspec) new_plots[plotspec] = reused_plot + end end return new_plots end -function update_plotspecs!(scene::Scene, list_of_plotspecs::Observable, plotlist::Union{Nothing, PlotList}=nothing) +function update_plotspecs!( + scene::Scene, list_of_plotspecs::Observable, + plotlist::Union{Nothing,PlotList}=nothing, + unused_plots=IdDict{PlotSpec,Plot}(), + new_plots=IdDict{PlotSpec,Plot}(), + own_plots=true + ) # Cache plots here so that we aren't re-creating plots every time; # if a plot still exists from last time, update it accordingly. # If the plot is removed from `plotspecs`, we'll delete it from here # and re-create it if it ever returns. - unused_plots = IdDict{PlotSpec,Plot}() obs_to_notify = Observable[] - update_plotlist(spec::PlotSpec) = update_plotlist([spec]) function update_plotlist(plotspecs) # Global list of observables that need updating # Updating them all at once in the end avoids problems with triggering updates while updating # And at some point we may be able to optimize notify(list_of_observables) - empty!(obs_to_notify) empty!(scene.cycler.counters) # Reset Cycler # diff_plotlist! deletes all plots that get re-used from unused_plots # so, this will become our list of unused plots! - new_plots = diff_plotlist!(scene, plotspecs, obs_to_notify, unused_plots, plotlist) + diff_plotlist!(scene, plotspecs, obs_to_notify, plotlist, unused_plots, new_plots) # 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 (_, plot) in unused_plots - if !isnothing(plotlist) - filter!(x -> x !== plot, plotlist.plots) + if own_plots + for (_, plot) in unused_plots + if !isnothing(plotlist) + filter!(x -> x !== plot, plotlist.plots) + end + delete!(scene, plot) end - delete!(scene, plot) + # Transfer all new plots into unused_plots for the next update! + @assert !any(x-> x in unused_plots, new_plots) + empty!(unused_plots) + merge!(unused_plots, new_plots) + empty!(new_plots) + # finally, notify all changes at once end - # Transfer all new plots into unused_plots for the next update! - @assert !any(x-> x in unused_plots, new_plots) - empty!(unused_plots) - merge!(unused_plots, new_plots) - # finally, notify all changes at once foreach(notify, obs_to_notify) + empty!(obs_to_notify) return end l = Base.ReentrantLock() @@ -786,9 +809,7 @@ function update_layoutable!(block::T, plot_obs, old_spec::BlockSpec, spec::Block empty!(block.scene.cycler.counters) end if T <: AbstractAxis - if plot_obs[] != spec.plots - plot_obs[] = spec.plots - end + plot_obs[] = spec.plots scene = get_scene(block) if any(needs_tight_limits, scene.plots) tightlimits!(block) @@ -853,7 +874,7 @@ end function update_gridlayout!(gridlayout::GridLayout, nesting::Int, oldgridspec::Union{Nothing, GridLayoutSpec}, - gridspec::GridLayoutSpec, previous_contents, new_layoutables) + gridspec::GridLayoutSpec, previous_contents, new_layoutables, global_unused_plots, new_plots) update_layoutable!(gridlayout, nothing, oldgridspec, gridspec) scores = IdDict{Any, Float64}() @@ -869,7 +890,7 @@ function update_gridlayout!(gridlayout::GridLayout, nesting::Int, oldgridspec::U if new_layoutable isa AbstractAxis obs = Observable(spec.plots) scene = get_scene(new_layoutable) - update_plotspecs!(scene, obs) + update_plotspecs!(scene, obs, nothing, global_unused_plots, new_plots, false) if any(needs_tight_limits, scene.plots) tightlimits!(new_layoutable) end @@ -877,7 +898,7 @@ function update_gridlayout!(gridlayout::GridLayout, nesting::Int, oldgridspec::U elseif new_layoutable isa GridLayout # Make sure all plots & blocks are inserted update_gridlayout!(new_layoutable, nesting + 1, spec, spec, previous_contents, - new_layoutables) + new_layoutables, global_unused_plots, new_plots) end push!(new_layoutables, (nesting, position, spec) => (new_layoutable, obs)) else @@ -888,7 +909,8 @@ function update_gridlayout!(gridlayout::GridLayout, nesting::Int, oldgridspec::U (layoutable, plot_obs) = layoutable_obs gridlayout[position...] = layoutable if layoutable isa GridLayout - update_gridlayout!(layoutable, nesting + 1, old_spec, spec, previous_contents, new_layoutables) + update_gridlayout!(layoutable, nesting + 1, old_spec, spec, previous_contents, + new_layoutables, global_unused_plots, new_plots) else update_layoutable!(layoutable, plot_obs, old_spec, spec) update_state_before_display!(layoutable) @@ -913,13 +935,16 @@ function delete_layoutable!(grid::GridLayout) end function update_gridlayout!(target_layout::GridLayout, layout_spec::GridLayoutSpec, unused_layoutables, - new_layoutables) + new_layoutables, unused_plots, new_plots) # For each update we look into `unused_layoutables` to see if we can re-use a layoutable (GridLayout/Block). # Every re-used layoutable and every newly created gets pushed into `new_layoutables`, # while it gets removed from `unused_layoutables`. empty!(new_layoutables) + update_gridlayout!( + target_layout, 1, nothing, layout_spec, unused_layoutables, + new_layoutables, unused_plots, new_plots + ) - update_gridlayout!(target_layout, 1, nothing, layout_spec, unused_layoutables, new_layoutables) foreach(unused_layoutables) do (p, (block, obs)) # disconnect! all unused layoutables, so they dont show up anymore if block isa Block @@ -941,6 +966,16 @@ function update_gridlayout!(target_layout::GridLayout, layout_spec::GridLayoutSp GridLayoutBase.update!(l) end + for (_, plot) in unused_plots + delete!(plot.parent, plot) + end + # Transfer all new plots into unused_plots for the next update! + @assert isempty(unused_plots) || !any(x -> x in unused_plots, new_plots) + empty!(unused_plots) + merge!(unused_plots, new_plots) + empty!(new_plots) + # finally, notify all changes at once + # foreach(unused_layoutables) do (p, (block, obs)) # # Finally, disconnect all blocks that haven't been used! # disconnect!(block) @@ -962,9 +997,12 @@ function update_fig!(fig::Union{Figure,GridPosition,GridSubposition}, layout_obs sizehint!(new_layoutables, 50) l = Base.ReentrantLock() layout = get_layout!(fig) + unused_plots = IdDict{PlotSpec,Plot}() + new_plots = IdDict{PlotSpec,Plot}() on(get_topscene(fig), layout_obs; update=true) do layout_spec lock(l) do - update_gridlayout!(layout, layout_spec, unused_layoutables, new_layoutables) + update_gridlayout!(layout, layout_spec, unused_layoutables, new_layoutables, + unused_plots, new_plots) return end end diff --git a/test/specapi.jl b/test/specapi.jl index cea645640aa..9c0a6f23ac4 100644 --- a/test/specapi.jl +++ b/test/specapi.jl @@ -50,19 +50,19 @@ import Makie.SpecApi as S plotspecs = [S.Scatter(1:4; color=:red), S.Scatter(1:4; color=:red)] reusable_plots = IdDict{PlotSpec,Plot}() obs_to_notify = Observable[] - new_plots = Makie.diff_plotlist!(scene, plotspecs, obs_to_notify, reusable_plots) + new_plots = Makie.diff_plotlist!(scene, plotspecs, obs_to_notify, nothing, reusable_plots) @test length(new_plots) == 2 @test Set(scene.plots) == Set(values(new_plots)) @test isempty(obs_to_notify) - new_plots2 = Makie.diff_plotlist!(scene, plotspecs, obs_to_notify, new_plots) + new_plots2 = Makie.diff_plotlist!(scene, plotspecs, obs_to_notify, nothing, new_plots) @test isempty(new_plots) # they got all used up @test Set(scene.plots) == Set(values(new_plots2)) @test isempty(obs_to_notify) plotspecs = [S.Scatter(1:4; color=:yellow), S.Scatter(1:4; color=:green)] - new_plots3 = Makie.diff_plotlist!(scene, plotspecs, obs_to_notify, new_plots2) + new_plots3 = Makie.diff_plotlist!(scene, plotspecs, obs_to_notify, nothing, new_plots2) @test isempty(new_plots) # they got all used up @test Set(scene.plots) == Set(values(new_plots3)) From 743a443e6e4c2f033275ef30218806f02c646e96 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com> Date: Mon, 4 Nov 2024 19:54:09 +0100 Subject: [PATCH 14/17] HTML video size annotation (#4563) * Annotate inline video with logical video size for correct scaling * add test * changelog entry * use `size(scene)` * rename `root_scene` to `scene` and add unit test --- CHANGELOG.md | 1 + CairoMakie/test/runtests.jl | 9 ++++++++- GLMakie/src/display.jl | 8 ++++---- GLMakie/src/picking.jl | 8 ++++---- GLMakie/src/rendering.jl | 8 ++++---- GLMakie/src/screen.jl | 20 ++++++++++---------- GLMakie/test/unit_tests.jl | 18 +++++++++++++++++- src/recording.jl | 7 ++++++- 8 files changed, 54 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74cebd149f0..d73e4b9b8be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Expand PlotList plots to expose their child plots to the legend interface, allowing `axislegend`show plots within PlotSpecs as individual entries. [#4546](https://github.com/MakieOrg/Makie.jl/pull/4546) - Implement S.Colorbar(plotspec) [#4520](https://github.com/MakieOrg/Makie.jl/pull/4520). - Fixed a hang when `Record` was created inside a closure passed to `IOCapture.capture` [#4562](https://github.com/MakieOrg/Makie.jl/pull/4562). +- Added logical size annotation to `text/html` inline videos so that sizes are appropriate independent of the current `px_per_unit` value [#4563](https://github.com/MakieOrg/Makie.jl/pull/4563). ## [0.21.15] - 2024-10-25 diff --git a/CairoMakie/test/runtests.jl b/CairoMakie/test/runtests.jl index 01b2f4283a9..e4a15c9c8d9 100644 --- a/CairoMakie/test/runtests.jl +++ b/CairoMakie/test/runtests.jl @@ -122,7 +122,9 @@ end @testset "VideoStream & screen options" begin N = 3 points = Observable(Point2f[]) - f, ax, pl = scatter(points, axis=(type=Axis, aspect=DataAspect(), limits=(0.4, N + 0.6, 0.4, N + 0.6),), figure=(size=(600, 800),)) + width = 600 + height = 800 + f, ax, pl = scatter(points, axis=(type=Axis, aspect=DataAspect(), limits=(0.4, N + 0.6, 0.4, N + 0.6),), figure=(size=(width, height),)) vio = Makie.VideoStream(f; format="mp4", px_per_unit=2.0, backend=CairoMakie) tmp_path = vio.path @@ -132,6 +134,11 @@ end @test vio.screen.device_scaling_factor == 2.0 Makie.recordframe!(vio) + + html = repr(MIME"text/html"(), vio) + @test occursin("width=\"$width\"", html) + @test occursin("height=\"$height\"", html) + save("test.mp4", vio) save("test_2.mkv", vio) save("test_3.mp4", vio) diff --git a/GLMakie/src/display.jl b/GLMakie/src/display.jl index 0763cf6023b..e477f0f42c1 100644 --- a/GLMakie/src/display.jl +++ b/GLMakie/src/display.jl @@ -2,13 +2,13 @@ function Base.display(screen::Screen, scene::Scene; connect=true) # So, the GLFW window events are not guarantee to fire # when we close a window, so we ensure this here! if !Makie.is_displayed(screen, scene) - if !isnothing(screen.root_scene) - delete!(screen, screen.root_scene) - screen.root_scene = nothing + if !isnothing(screen.scene) + delete!(screen, screen.scene) + screen.scene = nothing end display_scene!(screen, scene) else - @assert screen.root_scene === scene "internal error. Scene already displayed by screen but not as root scene" + @assert screen.scene === scene "internal error. Scene already displayed by screen but not as root scene" end pollevents(screen, Makie.BackendTick) return screen diff --git a/GLMakie/src/picking.jl b/GLMakie/src/picking.jl index cffc12dd73b..cde1cd4d59f 100644 --- a/GLMakie/src/picking.jl +++ b/GLMakie/src/picking.jl @@ -12,7 +12,7 @@ function pick_native(screen::Screen, rect::Rect2i) glReadBuffer(GL_COLOR_ATTACHMENT1) rx, ry = minimum(rect) rw, rh = widths(rect) - w, h = size(screen.root_scene) + w, h = size(screen.scene) ppu = screen.px_per_unit[] if rx >= 0 && ry >= 0 && rx + rw <= w && ry + rh <= h rx, ry, rw, rh = round.(Int, ppu .* (rx, ry, rw, rh)) @@ -32,7 +32,7 @@ function pick_native(screen::Screen, xy::Vec{2, Float64}) glBindFramebuffer(GL_FRAMEBUFFER, fb.id[1]) glReadBuffer(GL_COLOR_ATTACHMENT1) x, y = floor.(Int, xy) - w, h = size(screen.root_scene) + w, h = size(screen.scene) ppu = screen.px_per_unit[] if x > 0 && y > 0 && x <= w && y <= h x, y = round.(Int, ppu .* (x, y)) @@ -67,7 +67,7 @@ end # Skips one set of allocations function Makie.pick_closest(scene::Scene, screen::Screen, xy, range) isopen(screen) || return (nothing, 0) - w, h = size(screen.root_scene) # unitless dimensions + w, h = size(screen.scene) # unitless dimensions ((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h)) || return (nothing, 0) fb = screen.framebuffer @@ -106,7 +106,7 @@ end # Skips some allocations function Makie.pick_sorted(scene::Scene, screen::Screen, xy, range) isopen(screen) || return Tuple{AbstractPlot, Int}[] - w, h = size(screen.root_scene) # unitless dimensions + w, h = size(screen.scene) # unitless dimensions if !((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h)) return Tuple{AbstractPlot, Int}[] end diff --git a/GLMakie/src/rendering.jl b/GLMakie/src/rendering.jl index dda559f5ec3..18e41427383 100644 --- a/GLMakie/src/rendering.jl +++ b/GLMakie/src/rendering.jl @@ -1,8 +1,8 @@ function setup!(screen::Screen) glEnable(GL_SCISSOR_TEST) - if isopen(screen) && !isnothing(screen.root_scene) + if isopen(screen) && !isnothing(screen.scene) ppu = screen.px_per_unit[] - glScissor(0, 0, round.(Int, size(screen.root_scene) .* ppu)...) + glScissor(0, 0, round.(Int, size(screen.scene) .* ppu)...) glClearColor(1, 1, 1, 1) glClear(GL_COLOR_BUFFER_BIT) for (id, scene) in screen.screens @@ -43,9 +43,9 @@ function render_frame(screen::Screen; resize_buffers=true) # render order here may introduce artifacts because of that. fb = screen.framebuffer - if resize_buffers && !isnothing(screen.root_scene) + if resize_buffers && !isnothing(screen.scene) ppu = screen.px_per_unit[] - resize!(fb, round.(Int, ppu .* size(screen.root_scene))...) + resize!(fb, round.(Int, ppu .* size(screen.scene))...) end # prepare stencil (for sub-scenes) diff --git a/GLMakie/src/screen.jl b/GLMakie/src/screen.jl index a6782635144..0add2c5279d 100644 --- a/GLMakie/src/screen.jl +++ b/GLMakie/src/screen.jl @@ -180,7 +180,7 @@ mutable struct Screen{GLWindow} <: MakieScreen window_open::Observable{Bool} scalefactor::Observable{Float32} - root_scene::Union{Scene, Nothing} + scene::Union{Scene, Nothing} reuse::Bool close_after_renderloop::Bool # To trigger rerenders that aren't related to an existing renderobject. @@ -400,8 +400,8 @@ function apply_config!(screen::Screen, config::ScreenConfig; start_renderloop::B else stop_renderloop!(screen) end - if !isnothing(screen.root_scene) - resize!(screen, size(screen.root_scene)...) + if !isnothing(screen.scene) + resize!(screen, size(screen.scene)...) end set_screen_visibility!(screen, config.visible) return screen @@ -442,7 +442,7 @@ function display_scene!(screen::Screen, scene::Scene) insertplots!(screen, scene) Makie.push_screen!(scene, screen) connect_screen(scene, screen) - screen.root_scene = scene + screen.scene = scene return end @@ -612,10 +612,10 @@ function Base.empty!(screen::Screen) delete!(screen, Makie.rootparent(plot), plot) end - if !isnothing(screen.root_scene) - Makie.disconnect_screen(screen.root_scene, screen) - delete!(screen, screen.root_scene) - screen.root_scene = nothing + if !isnothing(screen.scene) + Makie.disconnect_screen(screen.scene, screen) + delete!(screen, screen.scene) + screen.scene = nothing end @assert isempty(screen.renderlist) @@ -875,8 +875,8 @@ end scalechangecb(screen) = (window, xscale, yscale) -> scalechangecb(screen, window, xscale, yscale) function scalechangeobs(screen, _) - if !isnothing(screen.root_scene) - resize!(screen, size(screen.root_scene)...) + if !isnothing(screen.scene) + resize!(screen, size(screen.scene)...) end return nothing end diff --git a/GLMakie/test/unit_tests.jl b/GLMakie/test/unit_tests.jl index d62843af195..2da385dbbe3 100644 --- a/GLMakie/test/unit_tests.jl +++ b/GLMakie/test/unit_tests.jl @@ -288,7 +288,7 @@ end @test isempty(screen.px_per_unit.listeners) @test isempty(screen.scalefactor.listeners) - @test screen.root_scene === nothing + @test screen.scene === nothing @test screen.rendertask === nothing @test (Base.summarysize(screen) / 10^6) < 1.4 end @@ -438,6 +438,22 @@ end GLMakie.closeall() end +@testset "html video size annotation" begin + width = 600 + height = 800 + f = Figure(; size = (width, height)) + + vio = Makie.VideoStream(f; format="mp4", px_per_unit=2.0, backend=GLMakie) + + @test size(vio.screen) == size(f.scene) .* 2 + + Makie.recordframe!(vio) + + html = repr(MIME"text/html"(), vio) + @test occursin("width=\"$width\"", html) + @test occursin("height=\"$height\"", html) +end + @testset "image size changes" begin s = Scene() im = image!(s, 0..10, 0..10, zeros(RGBf, 10, 20)) diff --git a/src/recording.jl b/src/recording.jl index 7f01de597db..db4f705f399 100644 --- a/src/recording.jl +++ b/src/recording.jl @@ -179,13 +179,18 @@ function Record(func, figlike, iter; kw_args...) end function Base.show(io::IO, ::MIME"text/html", vs::VideoStream) + scene = vs.screen.scene + if !(scene isa Scene) + error("Expected Screen to hold a reference to a Scene but got $(repr(scene))") + end + w, h = size(scene) mktempdir() do dir path = save(joinpath(dir, "video.mp4"), vs) #