From 48939827378bfc760fbbf68271e4daab6ed1a142 Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Thu, 12 May 2022 10:19:28 +0200 Subject: [PATCH] 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 --- GLMakie/src/drawing_primitives.jl | 18 +-------- ReferenceTests/src/tests/text.jl | 2 +- src/basic_recipes/text.jl | 35 ++++++++-------- src/layouting/layouting.jl | 27 ++++++++----- src/makielayout/blocks/axis3d.jl | 67 ++++++++++++++++++++----------- 5 files changed, 83 insertions(+), 66 deletions(-) diff --git a/GLMakie/src/drawing_primitives.jl b/GLMakie/src/drawing_primitives.jl index 066cc53dab0..ff59b9144cb 100644 --- a/GLMakie/src/drawing_primitives.jl +++ b/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 diff --git a/ReferenceTests/src/tests/text.jl b/ReferenceTests/src/tests/text.jl index c563a7ca2f3..5caa941d983 100644 --- a/ReferenceTests/src/tests/text.jl +++ b/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 diff --git a/src/basic_recipes/text.jl b/src/basic_recipes/text.jl index f8db096c985..e1bf95f1127 100644 --- a/src/basic_recipes/text.jl +++ b/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) diff --git a/src/layouting/layouting.jl b/src/layouting/layouting.jl index 2806d72a458..42fe483afd7 100644 --- a/src/layouting/layouting.jl +++ b/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 diff --git a/src/makielayout/blocks/axis3d.jl b/src/makielayout/blocks/axis3d.jl index 9344360212d..8b2bf889c69 100644 --- a/src/makielayout/blocks/axis3d.jl +++ b/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