Improve Axis3 interactivity performance (#1835)

* skip unnecessary updates

* use input observable

* reduce update dublication

* fix test

* fix tests

* fix test

* undo some bad changes

* hacky fix for GLMakie text updates

* undo last change

* hacky fix for text updates #2

* fix typos

Co-authored-by: SimonDanisch <sdanisch@gmail.com>

GLMakie/src/drawing_primitives.jl

@@ -310,24 +310,8 @@ function draw_atomic(screen::GLScreen, scene::Scene,
         markerspace = gl_attributes[:markerspace]
         offset = pop!(gl_attributes, :offset, Vec2f(0))
 
-        # TODO: This is a hack before we get better updating of plot objects and attributes going.
-        # Here we only update the glyphs when the glyphcollection changes, if it's a singular glyphcollection.
-        # The if statement will be compiled away depending on the parameter of Text.
-        # This means that updates of a text vector and a separate position vector will still not work if only the text
-        # vector is triggered, but basically all internal objects use the vector of tuples version, and that triggers
-        # both glyphcollection and position, so it still works
-        if glyphcollection[] isa Makie.GlyphCollection
-            # here we use the glyph collection observable directly
-            gcollection = glyphcollection
-        else
-            # and here we wrap it into another observable
-            # so it doesn't trigger dimension mismatches
-            # the actual, new value gets then taken in the below lift with to_value
-            gcollection = Observable(glyphcollection)
-        end
-
         # calculate quad metrics
-        glyph_data = map(pos, gcollection, offset, transfunc) do pos, gc, offset, transfunc
+        glyph_data = map(pos, glyphcollection, offset, transfunc) do pos, gc, offset, transfunc
             Makie.text_quads(pos, to_value(gc), offset, transfunc)
         end
 

ReferenceTests/src/tests/text.jl

@@ -298,7 +298,7 @@ end
 
     Makie.step!(st)
     ## change lengths
-    textnode.val = push!(textnode[], L"\int_0^5x^2+2ab")
+    textnode[] = push!(textnode[], L"\int_0^5x^2+2ab")
     posnode[] = push!(posnode[], Point2f(150, 150))
     Makie.step!(st)
     st

src/basic_recipes/text.jl

@@ -25,15 +25,13 @@ plot!(plot::Text{<:Tuple{<:GlyphCollection}}) = plot
 plot!(plot::Text{<:Tuple{<:AbstractArray{<:GlyphCollection}}}) = plot
 
 function plot!(plot::Text{<:Tuple{<:AbstractArray{<:AbstractString}}})
-
     glyphcollections = Observable(GlyphCollection[])
-    position = Observable{Any}(nothing)
     rotation = Observable{Any}(nothing)
 
-    onany(plot[1], plot.textsize, plot.position,
-            plot.font, plot.align, plot.rotation, plot.justification,
-            plot.lineheight, plot.color, plot.strokecolor, plot.strokewidth) do str,
-                    ts, pos, f, al, rot, jus, lh, col, scol, swi
+    onany(plot[1], plot.textsize, plot.font, plot.align, 
+            plot.rotation, plot.justification, plot.lineheight, plot.color, 
+            plot.strokecolor, plot.strokewidth) do str,
+                    ts, f, al, rot, jus, lh, col, scol, swi
 
         ts = to_textsize(ts)
         f = to_font(f)
@@ -47,9 +45,9 @@ function plot!(plot::Text{<:Tuple{<:AbstractArray{<:AbstractString}}})
             subgl = layout_text(str, ts, f, al, rot, jus, lh, col, scol, swi)
             push!(gcs, subgl)
         end
-        position.val = pos
         rotation.val = rot
         glyphcollections[] = gcs
+        return
     end
 
     # run onany once to initialize
@@ -78,10 +76,11 @@ function plot!(plot::Text{<:Tuple{<:AbstractArray{<:Tuple{<:AbstractString, <:Po
     on(strings_and_positions) do str_pos
         strs = first.(str_pos)
         poss = to_ndim.(Ref(Point3f), last.(str_pos), 0)
-        # first mutate strings without triggering redraw
-        t[1].val = strs
-        # then update positions with trigger
-        positions[] = poss
+
+        t[1].val != strs && (t[1][] = strs)
+        positions.val != poss && (positions[] = poss)
+
+        return
     end
     plot
 end
@@ -91,8 +90,8 @@ function plot!(plot::Text{<:Tuple{<:Union{LaTeXString, AbstractVector{<:LaTeXStr
 
     # attach a function to any text that calculates the glyph layout and stores it
     lineels_glyphcollection_offset = lift(plot[1], plot.textsize, plot.align, plot.rotation,
-            plot.model, plot.color, plot.strokecolor, plot.strokewidth, plot.position) do latexstring,
-                ts, al, rot, mo, color, scolor, swidth, _
+            plot.model, plot.color, plot.strokecolor, plot.strokewidth) do latexstring,
+                ts, al, rot, mo, color, scolor, swidth
 
         ts = to_textsize(ts)
         rot = to_rotation(rot)
@@ -125,10 +124,13 @@ function plot!(plot::Text{<:Tuple{<:Union{LaTeXString, AbstractVector{<:LaTeXStr
 
     scene = Makie.parent_scene(plot)
 
-    onany(lineels_glyphcollection_offset, scene.camera.projectionview) do (allels, gcs, offs), projview
+    onany(lineels_glyphcollection_offset, plot.position, scene.camera.projectionview
+            ) do (allels, gcs, offs), pos, projview
 
-        inv_projview = inv(projview)
-        pos = plot.position[]
+        if pos isa Vector && (length(pos) != length(allels))
+            return
+        end
+        # inv_projview = inv(projview)
         ts = plot.textsize[]
         rot = plot.rotation[]
 
@@ -171,6 +173,7 @@ function plot!(plot::Text{<:Tuple{<:Union{LaTeXString, AbstractVector{<:LaTeXStr
             append!(linepairs.val, first.(pairs))
         end
         notify(linepairs)
+        return
     end
 
     notify(plot.position)

src/layouting/layouting.jl

@@ -297,21 +297,30 @@ function text_quads(position::Vector, gcs::Vector{<: GlyphCollection}, offset, t
     pad = GLYPH_PADDING[] / PIXELSIZE_IN_ATLAS[]
     off = _offset_to_vec(offset)
 
+    # To avoid errors due to asynchronous updates of text attributes
+    M = min(
+        length(gcs), length(position), 
+        ifelse(off isa StaticVector || length(off) == 1, typemax(Int), length(off))
+    )
+
     char_offsets = Vector{Vec3f}(undef, length(pos)) # TODO can this be Vec2f?
     quad_offsets = Vector{Vec2f}(undef, length(pos))
     scales = Vector{Vec2f}(undef, length(pos))
     uvs = Vector{Vec4f}(undef, length(pos))
 
     k = 1
-    for (j, gc) in enumerate(gcs), i in eachindex(gc.origins)
-        glyph_bb, extent = FreeTypeAbstraction.metrics_bb(
-            gc.glyphs[i], gc.fonts[i], gc.scales[i]
-        )
-        uvs[k] = glyph_uv_width!(atlas, gc.glyphs[i], gc.fonts[i])
-        scales[k] = widths(glyph_bb) .+ gc.scales[i] * 2pad
-        char_offsets[k] = gc.origins[i] .+ _offset_at(off, j)
-        quad_offsets[k] = minimum(glyph_bb) .- gc.scales[i] .* pad
-        k += 1
+    for j in 1:M
+        gc = gcs[j]
+        for i in eachindex(gc.origins)
+            glyph_bb, extent = FreeTypeAbstraction.metrics_bb(
+                gc.glyphs[i], gc.fonts[i], gc.scales[i]
+            )
+            uvs[k] = glyph_uv_width!(atlas, gc.glyphs[i], gc.fonts[i])
+            scales[k] = widths(glyph_bb) .+ gc.scales[i] * 2pad
+            char_offsets[k] = gc.origins[i] .+ _offset_at(off, j)
+            quad_offsets[k] = minimum(glyph_bb) .- gc.scales[i] .* pad
+            k += 1
+        end
     end
 
     return pos, char_offsets, quad_offsets, uvs, scales

src/makielayout/blocks/axis3d.jl

@@ -19,6 +19,25 @@ function initialize_block!(ax::Axis3)
     cam = OrthographicCamera()
     cameracontrols!(scene, cam)
 
+    mi1 = Observable(!(pi/2 <= ax.azimuth[] % 2pi < 3pi/2))
+    mi2 = Observable(0 <= ax.azimuth[] % 2pi < pi)
+    mi3 = Observable(ax.elevation[] > 0)
+
+    on(ax.azimuth) do x
+        b = !(pi/2 <= x % 2pi < 3pi/2)
+        mi1.val == b || (mi1[] = b)
+        return
+    end
+    on(ax.azimuth) do x
+        b = 0 <= x % 2pi < pi
+        mi2.val == b || (mi2[] = b)
+        return
+    end
+    on(ax.elevation) do x
+        mi3.val == (x > 0) || (mi3[] = x > 0)
+        return
+    end
+
     matrices = lift(calculate_matrices, finallimits, scene.px_area, ax.elevation, ax.azimuth, ax.perspectiveness, ax.aspect, ax.viewmode)
 
     on(matrices) do (view, proj, eyepos)
@@ -39,10 +58,6 @@ function initialize_block!(ax::Axis3)
         tl = get_ticks(ticks, identity, format, minimum(lims)[3], maximum(lims)[3])
     end
 
-    mi1 = @lift(!(pi/2 <= $(ax.azimuth) % 2pi < 3pi/2))
-    mi2 = @lift(0 <= $(ax.azimuth) % 2pi < pi)
-    mi3 = @lift($(ax.elevation) > 0)
-
     add_panel!(scene, ax, 1, 2, 3, finallimits, mi3)
     add_panel!(scene, ax, 2, 3, 1, finallimits, mi1)
     add_panel!(scene, ax, 1, 3, 2, finallimits, mi2)
@@ -397,9 +412,8 @@ function add_gridlines_and_frames!(topscene, scene, ax, dim::Int, limits, tickno
         visible = attr(:gridvisible), inspectable = false)
 
 
-    framepoints = lift(limits, min1, min2,
-            scene.camera.projectionview, scene.px_area) do lims, mi1, mi2, pview, pxa
-
+    framepoints = lift(limits, scene.camera.projectionview, scene.px_area, min1, min2
+            ) do lims, _, pxa, mi1, mi2
         o = pxa.origin
 
         f(mi) = mi ? minimum : maximum
@@ -451,7 +465,6 @@ function add_ticks_and_ticklabels!(topscene, scene, ax, dim::Int, limits, tickno
     tick_segments = lift(limits, tickvalues, miv, min1, min2,
             scene.camera.projectionview, scene.px_area) do lims, ticks, miv, min1, min2,
                 pview, pxa
-
         f1 = !min1 ? minimum(lims)[d1] : maximum(lims)[d1]
         f2 = min2 ? minimum(lims)[d2] : maximum(lims)[d2]
 
@@ -481,8 +494,7 @@ function add_ticks_and_ticklabels!(topscene, scene, ax, dim::Int, limits, tickno
     # we are going to transform the 3d tick segments into 2d of the topscene
     # because otherwise they
     # be cut when they extend beyond the scene boundary
-    tick_segments_2dz = lift(tick_segments,
-            scene.camera.projectionview, scene.px_area) do ts, _, _
+    tick_segments_2dz = lift(tick_segments, scene.camera.projectionview, scene.px_area) do ts, _, _
         map(ts) do p1_p2
             to_topscene_z_2d.(p1_p2, Ref(scene))
         end
@@ -529,10 +541,14 @@ function add_ticks_and_ticklabels!(topscene, scene, ax, dim::Int, limits, tickno
 
     translate!(ticklabels, 0, 0, 1000)
 
-    label_pos_rot_valign = lift(scene.px_area, scene.camera.projectionview,
-            limits, miv, min1, min2, attr(:labeloffset),
-            attr(:labelrotation)) do pxa, pv, lims, miv, min1, min2,
-                labeloffset, lrotation
+    label_position = Observable(Point2f(0))
+    label_rotation = Observable(0f0)
+    label_align = Observable((:center, :top))
+
+    onany(
+            scene.px_area, scene.camera.projectionview, limits, miv, min1, min2, 
+            attr(:labeloffset), attr(:labelrotation), attr(:labelalign)
+            ) do pxa, pv, lims, miv, min1, min2, labeloffset, lrotation, lalign
 
         o = pxa.origin
 
@@ -586,27 +602,32 @@ function add_ticks_and_ticklabels!(topscene, scene, ax, dim::Int, limits, tickno
         end
 
         valign = offset_vec[2] > 0 || slight_flip ? :bottom : :top
-
-        plus_offset, labelrotation, valign
-    end
-
-    labelalign = lift(label_pos_rot_valign, attr(:labelalign)) do (_, _, valign), lalign
-        if lalign == Makie.automatic
+        align = if lalign == Makie.automatic
             (:center, valign)
         else
             lalign
         end
+
+        label_align[] != align && (label_align[] = align)
+        label_rotation[] != labelrotation && (label_rotation[] = labelrotation)
+        label_position[] = plus_offset
+
+        return
     end
+    notify(attr(:labelalign))
+
     label = text!(topscene, attr(:label),
         color = attr(:labelcolor),
         textsize = attr(:labelsize),
         font = attr(:labelfont),
-        position = @lift($label_pos_rot_valign[1]),
-        rotation = @lift($label_pos_rot_valign[2]),
-        align = labelalign,
+        position = label_position,
+        rotation = label_rotation,
+        align = label_align,
         visible = attr(:labelvisible),
         inspectable = false
     )
+
+
     return ticks, ticklabels, label
 end