Accessing created Blocks/Plots for SpecApi objects (#4354)

* fix user attributes for specapi + updating

* add changelog

* fix makie tests

* make specapi work with interactions

* more correctly apply 	hen function

* re-use blocks

* remove double visible

* fix inspecotr for Resampler

* use better score

* fix tests

* properly free on(visible)

* correctly forward visible

* add more tests and bugfixes

* some optimizations

* performance improvements

* text update optimizations

* update for more efficient update

* optimizations for text updates

* fix WGLMakie

* fix wglmakie for real

* revert for now, until we flatten GlyphCollections

* fix tests

* add docs

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Optimize SpecApi, re-use Blocks better and add API to access the created block objects [#4354](https://github.com/MakieOrg/Makie.jl/pull/4354).
 - Fix `merge(attr1, attr2)` modifying nested attributes in `attr1` [#4416](https://github.com/MakieOrg/Makie.jl/pull/4416)
 - Fixed issue with CairoMakie rendering scene backgrounds at the wrong position [#4425](https://github.com/MakieOrg/Makie.jl/pull/4425)
 - Fix incorrect inverse transformation in `position_on_plot` for lines, causing incorrect tooltip placement in DataInspector [#4402](https://github.com/MakieOrg/Makie.jl/pull/4402)

WGLMakie/src/particles.jl

@@ -179,7 +179,7 @@ function scatter_shader(scene::Scene, attributes, plot)
     end
 
     per_instance = filter(attributes) do (k, v)
-        return k in per_instance_keys && !(isscalar(v[]))
+        return k in per_instance_keys && !(isscalar(to_value(v)))
     end
 
     for (k, v) in per_instance
@@ -218,7 +218,6 @@ function scatter_shader(scene::Scene, attributes, plot)
 
     handle_color!(plot, uniform_dict, per_instance, :color)
     handle_color_getter!(uniform_dict, per_instance)
-
     if haskey(uniform_dict, :color) && haskey(per_instance, :color)
         to_value(uniform_dict[:color]) isa Bool && delete!(uniform_dict, :color)
         to_value(per_instance[:color]) isa Bool && delete!(per_instance, :color)
@@ -238,14 +237,23 @@ function scatter_shader(scene::Scene, attributes, plot)
     get!(uniform_dict, :strokecolor, RGBAf(0, 0, 0, 0))
     get!(uniform_dict, :glowwidth, 0f0)
     get!(uniform_dict, :glowcolor, RGBAf(0, 0, 0, 0))
-
+    _, arr = first(per_instance)
+    if any(v-> length(arr) != length(v), values(per_instance))
+        lens = [k => length(v) for (k, v) in per_instance]
+        error("Not all have the same length: $(lens)")
+    end
     return InstancedProgram(WebGL(), lasset("sprites.vert"), lasset("sprites.frag"),
                             instance, VertexArray(; per_instance...), uniform_dict)
 end
 
 function create_shader(scene::Scene, plot::Scatter)
     # Potentially per instance attributes
-    attributes = copy(plot.attributes.attributes)
+    # create new dict so we don't automatically convert to observables
+    # Which is the case for Dict{Symbol, Observable}
+    attributes = Dict{Symbol, Any}()
+    for (k, v) in plot.attributes.attributes
+        attributes[k] = v
+    end
     space = get(attributes, :space, :data)
     attributes[:preprojection] = Mat4f(I) # calculate this in JS
     f32c, model = Makie.patch_model(plot)
@@ -272,7 +280,6 @@ function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.Gl
     glyphcollection = plot[1]
     f32c, model = Makie.patch_model(plot)
     pos = apply_transform_and_f32_conversion(plot, f32c, plot.position)
-    space = plot.space
     offset = plot.offset
 
     atlas = wgl_texture_atlas()
@@ -282,24 +289,23 @@ function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.Gl
 
     # unpack values from the one signal:
     positions, char_offset, quad_offset, uv_offset_width, scale = map((1, 2, 3, 4, 5)) do i
-        return lift(getindex, plot, glyph_data, i)
+        return lift(getindex, plot, glyph_data, i; ignore_equal_values=true)
     end
 
-    uniform_color = lift(plot, glyphcollection) do gc
+    uniform_color = lift(plot, glyphcollection; ignore_equal_values=true) do gc
         if gc isa AbstractArray
-            reduce(vcat, (Makie.collect_vector(g.colors, length(g.glyphs)) for g in gc),
-                init = RGBAf[])
+            reduce(vcat, (Makie.collect_vector(g.colors, length(g.glyphs)) for g in gc);
+                    init=RGBAf[])
         else
-            Makie.collect_vector(gc.colors, length(gc.glyphs))
+            gc.colors.sv
         end
     end
-
-    uniform_rotation = lift(plot, glyphcollection) do gc
+    uniform_rotation = lift(plot, glyphcollection; ignore_equal_values=true) do gc
         if gc isa AbstractArray
-            reduce(vcat, (Makie.collect_vector(g.rotations, length(g.glyphs)) for g in gc),
-                init = Quaternionf[])
+            reduce(vcat, (Makie.collect_vector(g.rotations, length(g.glyphs)) for g in gc);
+                    init=Quaternionf[])
         else
-            Makie.collect_vector(gc.rotations, length(gc.glyphs))
+            gc.rotations.sv
         end
     end
 
@@ -321,6 +327,5 @@ function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.Gl
         :glowwidth => plot.glowwidth,
         :glowcolor => plot.glowcolor,
     )
-
     return scatter_shader(scene, uniforms, plot_attributes)
 end

docs/src/explanations/specapi.md

@@ -310,3 +310,48 @@ nothing # hide
 ```@raw html
 <video autoplay loop muted playsinline controls src="./interactive_specapi.mp4" />
 ```
+
+
+## Accessing created Blocks
+
+You can access created blocks with the `then(f)` syntax:
+
+```julia
+import Makie.SpecApi as S
+ax =  S.Axis(...)
+ax.then() do actual_axis_object
+    return on(events(actual_axis_object).mouseposition) do mp
+        println("mouse: $(mp)")
+    end
+end
+```
+Note, that the callback must be pure, since the objects will get re-used and the callback will be called again.
+To allow `on` or `onany`, one can return an array of `ObserverFunctions` or single one as in the above example.
+```julia
+ax.then() do ax
+    obs1 = on(f1, events(ax).keyboardbutton)
+    obs2 = on(f2, events(ax).mousebutton)
+    obs_array = onany(f3, some_obs1, some_obs2)
+    return [obs1, obs2, obs_array...]
+end
+This allows the SpecApi to clean up the callbacks on re-use.
+Note that things like `hidedecorations!(axis)` is not yet supported, since we will need some better book keeping of what got mutated by that call.
+One of the few functions that's already supported is `linkaxes!`:
+
+```julia
+axes_1 = [S.Axis(title="Axis (1): $(i)") for i in 1:3]
+axes_2 = [S.Axis(title="Axis (2): $(i)") for i in 1:3]
+for ax1 in axes_1
+    for ax2 in axes_2
+        if ax1 != ax2
+            ax1.then() do iax
+                ax2.then() do jax
+                    linkaxes!(iax, jax)
+                    return
+                end
+                return
+            end
+        end
+    end
+end
+```

precompile/shared-precompile.jl

@@ -5,7 +5,6 @@ using GeometryBasics
 
 @compile poly(Recti(0, 0, 200, 200), strokewidth=20, strokecolor=:red, color=(:black, 0.4))
 
-@compile scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
 @compile scatter(0..1, rand(10), markersize=rand(10) .* 20)
 @compile scatter(LinRange(0, 1, 10), rand(10))
 @compile scatter(-1..1, x -> x^2)

src/basic_recipes/text.jl

@@ -26,8 +26,10 @@ function plot!(plot::Text)
         attributes(plot)[:text] = plot[2]
     end
     calc_color = plot.calculated_colors[]
+
     color_scaled = calc_color isa ColorMapping ? calc_color.color_scaled : plot.color
     cmap = calc_color isa ColorMapping ? calc_color.colormap : plot.colormap
+
     onany(plot, plot.text, plot.fontsize, plot.font, plot.fonts, plot.align,
           plot.rotation, plot.justification, plot.lineheight, color_scaled, cmap,
             plot.strokecolor, plot.strokewidth, plot.word_wrap_width, plot.offset) do str,
@@ -63,6 +65,7 @@ function plot!(plot::Text)
             # glyph_collection as per character.
             push_args(str, 1, ts, f, fs, al, rot, jus, lh, col, scol, swi, www, offs)
         end
+
         glyphcollections[] = gcs
         linewidths[] = lwidths
         linecolors[] = lcolors

src/interfaces.jl

@@ -372,7 +372,7 @@ function connect_plot!(parent::SceneLike, plot::Plot{F}) where {F}
     if t_user isa Transformation
         plot.transformation = t_user
     else
-        if t_user isa Automatic
+        if t_user isa Union{Nothing, Automatic}
             plot.transformation = Transformation()
         else
             t = Transformation()

src/layouting/text_layouting.jl

@@ -45,21 +45,18 @@ function layout_text(
         font, fonts, align, rotation, justification, lineheight, color,
         strokecolor, strokewidth, word_wrap_width
     )
-
     ft_font = to_font(font)
+    # TODO, somehow some unicode symbols don't get rendered if we dont have one font per char
+    # Which is really odd
+    fontperchar = attribute_per_char(string, ft_font)
     rscale = to_fontsize(fontsize)
     rot = to_rotation(rotation)
 
-    fontperchar = attribute_per_char(string, ft_font)
-    fontsizeperchar = attribute_per_char(string, rscale)
-
-    glyphcollection = glyph_collection(
-        string, fontperchar, fontsizeperchar, align[1], align[2],
+    return glyph_collection(
+        string, fontperchar, rscale, align[1], align[2],
         lineheight, justification, rot, color,
         strokecolor, strokewidth, word_wrap_width
     )
-
-    return glyphcollection
 end
 
 """
@@ -76,8 +73,8 @@ function glyph_collection(
     )
 
     isempty(str) && return GlyphCollection(
-        [], [], Point3f[],FreeTypeAbstraction.FontExtent{Float32}[],
-        Vec2f[], Float32[], RGBAf[], RGBAf[], Float32[])
+        [], NativeFont[], Point3f[], FreeTypeAbstraction.FontExtent{Float32}[],
+        Vec2f[], Float32[], RGBAf[], RGBAf[], 0f0)
 
     # collect information about every character in the string
     charinfos = broadcast((c for c in str), font_per_char, fontscale_px) do char, font, scale
@@ -93,7 +90,8 @@ function glyph_collection(
     # split the character info vector into lines after every \n
     lineinfos, xs = let
         last_line_start = 1
-        lineinfos = typeof(view(charinfos, last_line_start:last_line_start))[]
+        ViewType = typeof(view(charinfos, last_line_start:last_line_start))
+        lineinfos = ViewType[]
 
         last_space_local_idx = 0
         last_space_global_idx = 0
@@ -231,17 +229,19 @@ function glyph_collection(
     # these values should be enough to draw characters correctly,
     # compute boundingboxes without relayouting and maybe implement
     # interactive features that need to know where characters begin and end
-    per_char(attr) = collect(attribute_per_char(str, attr)) # attribute_per_char returns generators
+    function scalar_or_vec(attr)
+        attr # attribute_per_char returns generators
+    end
     return GlyphCollection(
         [FreeTypeAbstraction.glyph_index(x.font, x.char) for x in charinfos],
-        [x.font for x in charinfos],
+        font_per_char,
         reduce(vcat, charorigins),
         [x.extent for x in charinfos],
-        [Vec2f(x.scale) for x in charinfos],
-        per_char(rotation), # rotations is used as one rotation per string above. TODO, allow one rotation per char
-        per_char(color),
-        per_char(strokecolor),
-        per_char(strokewidth)
+        fontscale_px,
+        scalar_or_vec(rotation), # rotations is used as one rotation per string above. TODO, allow one rotation per char
+        scalar_or_vec(color),
+        scalar_or_vec(strokecolor),
+        scalar_or_vec(strokewidth)
     )
 end
 

src/makielayout/blocks.jl

@@ -466,18 +466,43 @@ function Base.delete!(block::Block)
     # detach plots, cameras, transformations, viewport
     empty!(block.blockscene)
 
+    disconnect!(block)
+    block.parent = nothing
+    return
+end
+
+function unhide!(block::Block)
+    if !block.blockscene.visible[]
+        block.blockscene.visible[] = true
+    end
+    if hasproperty(block, :scene) && !block.scene.visible[]
+        block.scene.visible[] = true
+    end
+end
+
+function hide!(block::Block)
+    if block.blockscene.visible[]
+        block.blockscene.visible[] = false
+    end
+    if hasproperty(block, :scene) && block.scene.visible[]
+        block.scene.visible[] = false
+    end
+end
+
+function disconnect!(block::Block)
+    hide!(block)
     gc = GridLayoutBase.gridcontent(block)
     if gc !== nothing
         GridLayoutBase.remove_from_gridlayout!(gc)
     end
 
     if block.parent !== nothing
-        delete_from_parent!(block.parent, block)
-        block.parent = nothing
+        Makie.delete_from_parent!(block.parent, block)
     end
     return
 end
 
+
 # do nothing for scene and nothing
 function delete_from_parent!(parent, block::Block)
 end

src/makielayout/blocks/legend.jl

@@ -12,7 +12,7 @@ function initialize_block!(leg::Legend; entrygroups)
     legend_area = lift(round_to_IRect2D, blockscene, leg.layoutobservables.computedbbox)
 
     scene = Scene(blockscene, blockscene.viewport, camera = campixel!)
-
+    leg.scene = scene
     # the rectangle in which the legend is drawn when margins are removed
     legendrect = lift(blockscene, legend_area, leg.margin) do la, lm
         enlarge(la, -lm[1], -lm[2], -lm[3], -lm[4])

src/makielayout/types.jl

@@ -953,7 +953,7 @@ end
 end
 
 """
-A grid of one or more horizontal `Slider`s, where each slider has a 
+A grid of one or more horizontal `Slider`s, where each slider has a
 name label on the left and a value label on the right.
 
 Each `NamedTuple` you pass specifies one `Slider`. You always have to pass `range`
@@ -1310,6 +1310,7 @@ end
 const EntryGroup = Tuple{Any, Vector{LegendEntry}}
 
 @Block Legend begin
+    scene::Scene
     entrygroups::Observable{Vector{EntryGroup}}
     _tellheight::Observable{Bool}
     _tellwidth::Observable{Bool}