From 68627eea88c5ad252c7ad97c89dc5c22a269349f Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:23:11 -0400 Subject: [PATCH 01/13] Fix `Pause` action evaluation and expose the 2-argument constructor `(::Pause)(view, t)` computed `t / duration(move)`, but the method binds the change to `pause`, so any `Pause` carrying an action threw an `UndefVarError` when evaluated. The docstring already advertised `Pause(duration, [action])`, but only the 1-argument method existed, so an action could not be attached through the public API at all. Give `action` a default in that method instead. Co-Authored-By: Claude Opus 5 (1M context) --- src/pathchange.jl | 4 ++-- test/runtests.jl | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/pathchange.jl b/src/pathchange.jl index 3eed4a5..e444d10 100644 --- a/src/pathchange.jl +++ b/src/pathchange.jl @@ -38,7 +38,7 @@ end Pause at the current position for `duration`. """ -Pause(duration::T) where T = Pause{T}(duration) +Pause(duration::T, action=nothing) where T = Pause{T}(duration, action) Base.convert(::Type{Pause{T}}, p::Pause) where T = Pause{T}(p.duration, p.action) Base.convert(::Type{PathChange{T}}, p::Pause) where T = convert(Pause{T}, p) @@ -151,7 +151,7 @@ function (pause::Pause{T})(view::ViewState{T}, t) where T checkt(t, pause) action = pause.action if action !== nothing - tf = t / duration(move) + tf = t / duration(pause) act(action, tf) end return view diff --git a/test/runtests.jl b/test/runtests.jl index c2deec7..7b4b671 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -32,6 +32,19 @@ using Test @test newpath(0.5).eyeposition == view.eyeposition @test path*Pause(1.0) isa Path{Float64} + + @testset "action" begin + ts = Float64[] + pause = Pause(2.0, t -> push!(ts, t)) + @test pause isa Pause{Float64} + newpath = path*pause + # The action fires with the fraction of the pause that has elapsed + @test newpath(1.0).eyeposition == view.eyeposition + @test ts == [0.5] + newpath(0.0) + newpath(2.0) + @test ts == [0.5, 0.0, 1.0] + end end @testset "ConstrainedMove" begin move = ConstrainedMove(5, ViewState(eyeposition=[0, 10, 0]), :none, :constant) From b0de4030c8294746cb54ac34c88b3ce494a7b9d2 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:27:25 -0400 Subject: [PATCH 02/13] Clamp the local time `path(t)` hands to a `PathChange` `path(t)` walks the changes accumulating `tend`, and a change is selected when `t <= tend + duration(change)`. Because `tend` is accumulated separately from the caller's `t`, `t - tend` can exceed `duration(change)` by an ulp, and `checkt` then rejects a time we had just decided belongs to that change. This is not a Float32-only effect: a five-segment `Path{Float64}` of 0.2 s moves already throws `ArgumentError: t=0.20000000000000007 is not in [0, 0.2]` at `nextfloat(0.6)`. Clamp at the call site instead of weakening `checkt`, which should still catch genuinely out-of-range input. Co-Authored-By: Claude Opus 5 (1M context) --- src/path.jl | 5 ++++- test/runtests.jl | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/path.jl b/src/path.jl index 7974ef1..8034eb8 100644 --- a/src/path.jl +++ b/src/path.jl @@ -34,7 +34,10 @@ function (path::Path{T})(t) where T for change in path.changes tnext = tend + duration(change) if t <= tnext - return change(view, t - tend) + # `tend` is accumulated separately from `t`, so `t - tend` can land a few ulps + # outside `[0, duration(change)]` even though `t` selected this change. Clamp + # rather than let `checkt` reject a time we just decided belongs here. + return change(view, clamp(t - tend, zero(T), duration(change))) end tend, view = tnext, filldefaults(target(view, change), view) end diff --git a/test/runtests.jl b/test/runtests.jl index 7b4b671..2362a34 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -80,5 +80,28 @@ using Test @test mid.lookat == view.lookat @test mid.upvector == view.upvector end + @testset "segment boundaries" begin + # `path(t)` accumulates the segment start times, so the local time handed to a + # `PathChange` can exceed that change's duration by an ulp even though `t` + # itself selected the segment. + view0 = ViewState{Float64}(eyeposition=[10, 0, 0], lookat=[0, 0, 0], upvector=[0, 0, 1], fov=45) + bpath = Path(view0) + for i in 1:5 + bpath = bpath * ConstrainedMove(0.2, ViewState{Float64}(eyeposition=[10, i, 0]), :none, :constant) + end + for k in 0:5 + t = 0.2k + @test bpath(t) isa ViewState{Float64} + @test bpath(prevfloat(t)) isa ViewState{Float64} + @test bpath(nextfloat(t)) isa ViewState{Float64} + end + # ...and the view is continuous across a boundary + @test bpath(prevfloat(0.6)).eyeposition ≈ bpath(nextfloat(0.6)).eyeposition + + # `checkt` should still reject times that are genuinely out of range + move = ConstrainedMove(1.0, ViewState{Float64}(eyeposition=[0, 10, 0]), :none, :constant) + @test_throws ArgumentError move(view0, 1.5) + @test_throws ArgumentError move(view0, -0.5) + end end end From 9c4f07f3f9a588cc1ea8d947085b5a5185f8ae95 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:28:10 -0400 Subject: [PATCH 03/13] Promote the `ViewState` element type from its arguments `ViewState(; kwargs...)` hard-coded `ViewState{Float32}`, so `ViewState(eyeposition = Point3d(...), fov = 40.0)` silently narrowed to Float32. Every `ConstrainedMove` built against such a state is then a `PathChange{Float32}`, and `path(t)` accumulates its segment start times in Float32; across ~750 chained moves spanning 122 s the drift reaches ~1e-6 s, which is enough to push a frame time past a segment's end. The element type is now promoted over the supplied values. Integers (and the all-`nothing` case) express no preference about precision, so they keep the `Float32` default that Makie cameras use and that the README and docs show; `ViewState{T}(; ...)` is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- src/viewstate.jl | 23 ++++++++++++++++++++++- test/runtests.jl | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/viewstate.jl b/src/viewstate.jl index ad166d2..fa18e8d 100644 --- a/src/viewstate.jl +++ b/src/viewstate.jl @@ -7,7 +7,28 @@ end ViewState{T}(; eyeposition=nothing, lookat=nothing, upvector=nothing, fov=nothing) where T = ViewState{T}(eyeposition, lookat, upvector, fov) -ViewState(; kwargs...) = ViewState{Float32}(; kwargs...) + +_eltype(::Nothing) = Union{} # contributes nothing to the promotion +_eltype(x::Number) = typeof(x) +_eltype(x) = eltype(x) + +# `Union{}` (nothing was supplied) is a subtype of everything, hence the first test +_floattype(::Type{T}) where T = T !== Union{} && T <: AbstractFloat ? T : Float32 + +""" + ViewState(; eyeposition, lookat, upvector, fov) + +Construct a `ViewState` whose element type is promoted from the supplied arguments; +e.g. `Point3d` positions or a `Float64` `fov` give a `ViewState{Float64}`. + +Integers express no preference about precision, so they (like omitting an argument +altogether) keep the `Float32` default, `Float32` being the space Makie cameras work in. +Use `ViewState{T}(; ...)` to choose the element type explicitly. +""" +function ViewState(; eyeposition=nothing, lookat=nothing, upvector=nothing, fov=nothing) + T = promote_type(_eltype(eyeposition), _eltype(lookat), _eltype(upvector), _eltype(fov)) + return ViewState{_floattype(T)}(eyeposition, lookat, upvector, fov) +end Base.convert(::Type{ViewState{T}}, v::ViewState) where T = ViewState{T}(v.eyeposition, v.lookat, v.upvector, v.fov) diff --git a/test/runtests.jl b/test/runtests.jl index 2362a34..ef94479 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,5 +1,6 @@ using FlyThroughPaths using LinearAlgebra +using StaticArrays using Test @testset "FlyThroughPaths.jl" begin @@ -19,6 +20,23 @@ using Test # Round-trippability with display @test eval(Meta.parse(str)) == view end + @testset "element type" begin + # The element type is promoted from the supplied values + view64 = ViewState(eyeposition = SVector(1.0, 2.0, 3.0), lookat = SVector(0.0, 0.0, 0.0), + upvector = SVector(0.0, 0.0, 1.0), fov = 40.0) + @test view64 isa ViewState{Float64} + @test view64.eyeposition == [1, 2, 3] + # Float32 input still yields a Float32 ViewState + @test ViewState(eyeposition = SVector{3,Float32}(1, 2, 3), fov = 40f0) isa ViewState{Float32} + @test ViewState(eyeposition = SVector{3,Float16}(1, 2, 3)) isa ViewState{Float16} + # A single Float64 field is enough to promote the whole state + @test ViewState(eyeposition = SVector{3,Float32}(1, 2, 3), fov = 40.0) isa ViewState{Float64} + # Integers carry no precision preference, so they keep the Float32 default + @test ViewState(eyeposition = [-10, 0, 0], fov = 45) isa ViewState{Float32} + @test ViewState() isa ViewState{Float32} + # Explicitly-typed construction is unaffected + @test ViewState{Float32}(eyeposition = SVector(1.0, 2.0, 3.0), fov = 40.0) isa ViewState{Float32} + end end @testset "Path" begin view = ViewState(eyeposition = [-10, 0, 0], lookat=[0, 0, 0], upvector=[0, 0, 1], fov=45) @@ -103,5 +121,21 @@ using Test @test_throws ArgumentError move(view0, 1.5) @test_throws ArgumentError move(view0, -0.5) end + @testset "long path" begin + # A 122 s flight assembled from 750 short moves: in Float32 the segment start + # times accumulated by `path(t)` drift away from the sampled frame times. + view0 = ViewState(eyeposition = SVector(10.0, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), + upvector = SVector(0.0, 0.0, 1.0), fov = 45.0) + n, tend = 750, 122.0 + longpath = Path(view0) + for i in 1:n + θ = 2π * i / n + longpath = longpath * ConstrainedMove(tend/n, ViewState(eyeposition = SVector(10cos(θ), 10sin(θ), 0.0)), :none, :constant) + end + @test longpath isa Path{Float64} + @test FlyThroughPaths.duration(longpath) ≈ tend + @test all(t -> longpath(t) isa ViewState{Float64}, range(0, tend; length = 1001)) + @test all(k -> longpath(k*(tend/n)) isa ViewState{Float64}, 0:n) + end end end From c10908da75f3e047afb6c9155b278313538a690d Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:31:23 -0400 Subject: [PATCH 04/13] Make the `:rotation` constraint an actual slerp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ConstrainedMove(...; constraint = :rotation)` blended the two offsets from the lookat point as `cospi(f/2) * vold + sinpi(f/2) * vnew`. With `d = norm(vold) = norm(vnew)` and `θ` the angle between them, that has squared length `d^2 * (1 + sinpi(f) * cos(θ))`, i.e. the distance to the lookat point is preserved only at `θ = 90°`. A "stationary" rotation (`θ = 0`) swells the radius to `d*sqrt(2)` halfway through, flying the camera 41% further out and back, and an antipodal one (`θ = 180°`) takes the radius through zero, i.e. through the point being looked at. Only the interior of the move was wrong; both endpoints were already correct. Replace it with a real slerp of the direction plus a separate interpolation of the length, so the distance to the lookat point moves monotonically between the endpoint distances for every `θ`. Degenerate cases: a zero-length offset falls back to a lerp, `θ ≈ 0` uses the chord (the great circle is degenerate there), and `θ ≈ 180°` is ill-conditioned, so the rotation plane is chosen deterministically from the axis `vold` is least aligned with. The length is interpolated geometrically rather than linearly, a constant relative rate of approach reading more evenly for a camera dolly. Co-Authored-By: Claude Opus 5 (1M context) --- Project.toml | 2 ++ src/FlyThroughPaths.jl | 1 + src/pathchange.jl | 44 +++++++++++++++++++++++++++++++++++++++++- test/runtests.jl | 29 ++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 9b7cb67..404465c 100644 --- a/Project.toml +++ b/Project.toml @@ -7,6 +7,7 @@ version = "0.1.3" projects = ["test", "docs"] [deps] +LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" StaticArrays = "90137ffa-7385-5640-81b9-e52037218182" [weakdeps] @@ -16,6 +17,7 @@ Makie = "ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a" FlyThroughPathsMakieExt = "Makie" [compat] +LinearAlgebra = "1" Makie = "0.21, 0.22, 0.23, 0.24" StaticArrays = "1" julia = "1.10" diff --git a/src/FlyThroughPaths.jl b/src/FlyThroughPaths.jl index 22d09ef..b6abe9b 100644 --- a/src/FlyThroughPaths.jl +++ b/src/FlyThroughPaths.jl @@ -1,5 +1,6 @@ module FlyThroughPaths +using LinearAlgebra using StaticArrays export ViewState, Path, Pause, ConstrainedMove, BezierMove diff --git a/src/pathchange.jl b/src/pathchange.jl index e444d10..1537943 100644 --- a/src/pathchange.jl +++ b/src/pathchange.jl @@ -145,6 +145,48 @@ Base.@nospecializeinfer function act(@nospecialize(action), t::Real) return nothing end +""" + slerp(vold, vnew, f) + +Interpolate between the vectors `vold` and `vnew` at fraction `f`, rotating the direction +along the great circle joining them while interpolating the length geometrically. The +length therefore varies monotonically between `norm(vold)` and `norm(vnew)`, and is +constant when those are equal. + +The naive blend `cospi(f/2) * vold + sinpi(f/2) * vnew` is not a rotation: writing +`d = norm(vold) = norm(vnew)` and `θ` for the angle between the two vectors, its squared +length is `d^2 * (1 + sinpi(f) * cos(θ))`, which is `d^2` only for `θ = 90°`. At `θ = 0` it +swells to `d^2*2` halfway through, and at `θ = 180°` it passes through zero, i.e. through +the point being looked at. +""" +function slerp(vold::SVector{3,T}, vnew::SVector{3,T}, f) where T + f <= 0 && return vold + f >= 1 && return vnew + dold, dnew = norm(vold), norm(vnew) + # A vector of zero length has no direction to rotate, so interpolate linearly instead + (iszero(dold) || iszero(dnew)) && return (1 - f) * vold + f * vnew + uold, unew = vold / dold, vnew / dnew + # Geometric interpolation of the length: a constant relative rate of approach reads + # more evenly than a linear one when the camera dollies in or out. + d = dold * (dnew / dold)^f + c = clamp(dot(uold, unew), -one(T), one(T)) + s = sqrt(max(zero(T), 1 - c * c)) # sin(θ) + if s > sqrt(eps(T)) + θ = atan(s, c) + return d * normalize(sin((1 - f) * θ) * uold + sin(f * θ) * unew) + elseif c > 0 + # θ ≈ 0: the great circle is degenerate, but the chord approximates it well + return d * normalize((1 - f) * uold + f * unew) + end + # θ ≈ 180°: ill-conditioned, since every plane containing `uold` also contains `unew` + # and the direction of travel is therefore arbitrary. Pick one deterministically, by + # rotating in the plane spanned by `uold` and the axis it is least aligned with. + i = argmin(abs.(uold)) + uperp = normalize(cross(uold, SVector(ntuple(j -> T(j == i), 3)))) + θ = atan(s, c) + return d * (cos(f * θ) * uold + sin(f * θ) * uperp) +end + # Compute the view from a PathChange at (relative) time t function (pause::Pause{T})(view::ViewState{T}, t) where T @@ -173,7 +215,7 @@ function (move::ConstrainedMove{T})(view::ViewState{T}, t) where T elseif constraint === :rotation vold = eyeposition - lookat vnew = eyeposition_new - lookat_new - eyeposition = cospi(f/2) * vold + sinpi(f/2) * vnew + lookatf + eyeposition = slerp(vold, vnew, f) + lookatf end upvector = (1 - f) * upvector + f * upvector_new fov = (1 - f) * fov + f * fov_new diff --git a/test/runtests.jl b/test/runtests.jl index ef94479..9e3f2b6 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -87,6 +87,35 @@ using Test v = newpath(1.25) @test norm(v.eyeposition - view.eyeposition) < 0.9 * norm(v.eyeposition - [-5, 5, 0]) end + @testset ":rotation constraint" begin + # A `cospi(f/2)*vold + sinpi(f/2)*vnew` blend has squared length + # d²(1 + sinpi(f)*cos(θ)), which is d² only for θ = 90°. The distance to the + # lookat point must instead stay between the two endpoint distances. + for θ in (0, 45, 90, 179, 180), (dold, dnew) in ((10.0, 10.0), (10.0, 4.0), (4.0, 10.0)) + view0 = ViewState(eyeposition = SVector(dold, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), + upvector = SVector(0.0, 0.0, 1.0), fov = 45.0) + eyenew = SVector(dnew*cosd(θ), dnew*sind(θ), 0.0) + rpath = Path(view0) * ConstrainedMove(1.0, ViewState(eyeposition = eyenew), :rotation, :constant) + # The endpoints are exact + @test rpath(0.0).eyeposition == view0.eyeposition + @test rpath(1.0).eyeposition == eyenew + radii = [norm(rpath(f).eyeposition - rpath(f).lookat) for f in range(0, 1; length = 101)] + @test !any(isnan, radii) + @test all(r -> min(dold, dnew) - 1e-8 <= r <= max(dold, dnew) + 1e-8, radii) + # ...and it varies monotonically, so equal endpoint radii stay constant + @test issorted(round.(radii; digits = 9); rev = dnew < dold) + end + # The interpolation is a rotation, not a chord: halfway through a 90° move at + # constant radius the camera sits at 45°. + view0 = ViewState(eyeposition = SVector(10.0, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), + upvector = SVector(0.0, 0.0, 1.0), fov = 45.0) + rpath = Path(view0) * ConstrainedMove(1.0, ViewState(eyeposition = SVector(0.0, 10.0, 0.0)), :rotation, :constant) + @test rpath(0.5).eyeposition ≈ [10/sqrt(2), 10/sqrt(2), 0] + @test rpath(0.25).eyeposition ≈ 10 .* [cosd(22.5), sind(22.5), 0] + # A move that only changes the distance still interpolates the distance smoothly + rpath = Path(view0) * ConstrainedMove(1.0, ViewState(eyeposition = SVector(5.0, 0.0, 0.0)), :rotation, :constant) + @test rpath(0.5).eyeposition ≈ [sqrt(50), 0, 0] # geometric mean of 10 and 5 + end @testset "BezierMove" begin move = BezierMove(5, ViewState(eyeposition=[0, 10, 0]), [ViewState(eyeposition=[-20, 20, 0])]) newpath = path*move From ec15974792b1bdb497aa7347baac2e8acada7b79 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:35:41 -0400 Subject: [PATCH 05/13] Fix the frame count in the Makie extension's `record` `record(fig, file, path::Path)` sampled the path with `round(Int, tend / framerate)` frames. That is inverted: the number of frames is `tend * framerate`. Every path shorter than about 1.5x the framerate produced a degenerate range, e.g. a 10 s path at 24 fps asked `LinRange` for `round(Int, 10/24) == 0` points. Factor the count into `FlyThroughPaths.nframes(path, rate)`, which never returns fewer than two samples, and use it for the recipe's `density` sampling too (`tend*density` was already the right expression there, but it rounds to 0 for a path shorter than one sampling interval). Co-Authored-By: Claude Opus 5 (1M context) --- ext/FlyThroughPathsMakieExt.jl | 4 ++-- src/path.jl | 11 +++++++++++ test/runtests.jl | 13 +++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/ext/FlyThroughPathsMakieExt.jl b/ext/FlyThroughPathsMakieExt.jl index 285bceb..5b1abb6 100644 --- a/ext/FlyThroughPathsMakieExt.jl +++ b/ext/FlyThroughPathsMakieExt.jl @@ -27,7 +27,7 @@ FlyThroughPaths.set_view!(axis::Makie.AbstractAxis, view::ViewState) = set_view! function Makie.record(fig::Makie.FigureLike, file::String, path::Path; framerate = 24, kwargs...) tend = FlyThroughPaths.duration(path) - trange = LinRange(0, tend, round(Int, tend / framerate)) + trange = LinRange(0, tend, FlyThroughPaths.nframes(path, framerate)) iterator = path.(trange) return Makie.record(fig, file, iterator; framerate, kwargs...) end @@ -57,7 +57,7 @@ function Makie.plot!(plot::PlotCameraPath) trange_obs = Observable{LinRange{Float64}}() onany(plot, plot.path, plot.density; update = true) do path, density tend = FlyThroughPaths.duration(path) - trange_obs.val = LinRange(0.0, Float64(tend), round(Int, tend*density)) + trange_obs.val = LinRange(0.0, Float64(tend), FlyThroughPaths.nframes(path, density)) eyepositions_obs.val = Makie.Point3d.(getproperty.(path.(trange_obs.val), :eyeposition)) notify(eyepositions_obs) notify(trange_obs) diff --git a/src/path.jl b/src/path.jl index 8034eb8..8aa4697 100644 --- a/src/path.jl +++ b/src/path.jl @@ -27,6 +27,17 @@ end duration(path::Path{T}) where T = sum(duration, path.changes; init = zero(T)) +""" + nframes(path, rate) + +Return the number of samples needed to traverse `path` at `rate` samples per second, +e.g. the number of frames to render at a given framerate. + +At least two samples are returned, so that a path shorter than one sampling interval +still yields a non-degenerate range. +""" +nframes(path::Path, rate) = max(2, round(Int, duration(path) * rate)) + function (path::Path{T})(t) where T view = path.initialview tend = zero(T) diff --git a/test/runtests.jl b/test/runtests.jl index 9e3f2b6..0728235 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -150,6 +150,19 @@ using Test @test_throws ArgumentError move(view0, 1.5) @test_throws ArgumentError move(view0, -0.5) end + @testset "nframes" begin + # Used by the Makie extension to sample a path for `record` + view0 = ViewState(eyeposition = SVector(10.0, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), + upvector = SVector(0.0, 0.0, 1.0), fov = 45.0) + tenseconds = Path(view0) * Pause(10.0) + @test FlyThroughPaths.nframes(tenseconds, 24) == 240 + @test FlyThroughPaths.nframes(tenseconds, 30) == 300 + @test FlyThroughPaths.nframes(Path(view0) * Pause(122.0), 30) == 3660 + @test FlyThroughPaths.nframes(Path(view0) * Pause(0.5), 24) == 12 + # A path shorter than a frame interval still needs a non-degenerate range + @test FlyThroughPaths.nframes(Path(view0) * Pause(0.01), 24) == 2 + @test FlyThroughPaths.nframes(Path(view0), 24) == 2 + end @testset "long path" begin # A 122 s flight assembled from 750 short moves: in Float32 the segment start # times accumulated by `path(t)` drift away from the sampled frame times. From 4562cdfb699ecdc1160ba88c03a57aefc9e802f8 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:35:59 -0400 Subject: [PATCH 06/13] Apply the path's views while recording in the Makie extension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `record(fig, file, path::Path)` materialized `path.(trange)` and passed that vector of `ViewState`s straight back to `Makie.record`, which threw `MethodError: no method matching record(::Figure, ::String, ::Vector{ViewState{Float64}})` — `Makie.record` takes the per-frame function first, and nothing in the old body applied the views to the figure anyway. So the method could not work regardless of the frame count. Iterate over the sample times with a function that sets the view. For a `Figure` that view goes to its current axis, via new `set_view!` methods for `Figure` and `FigureAxisPlot` that complete the `Scene`/`AbstractAxis` pair. Co-Authored-By: Claude Opus 5 (1M context) --- ext/FlyThroughPathsMakieExt.jl | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/ext/FlyThroughPathsMakieExt.jl b/ext/FlyThroughPathsMakieExt.jl index 5b1abb6..95ced51 100644 --- a/ext/FlyThroughPathsMakieExt.jl +++ b/ext/FlyThroughPathsMakieExt.jl @@ -24,12 +24,25 @@ function FlyThroughPaths.set_view!(scene::Scene, view::ViewState) return scene end FlyThroughPaths.set_view!(axis::Makie.AbstractAxis, view::ViewState) = set_view!(axis.scene, view) +FlyThroughPaths.set_view!(figaxplot::Makie.FigureAxisPlot, view::ViewState) = set_view!(figaxplot.axis, view) +function FlyThroughPaths.set_view!(fig::Makie.Figure, view::ViewState) + axis = Makie.current_axis(fig) + axis === nothing && throw(ArgumentError("`fig` has no current axis whose view could be set; pass the axis or scene instead.")) + return set_view!(axis, view) +end + +""" + record(figlike, file, path::Path; framerate = 24, kwargs...) +Record a video of `figlike` flying along `path`, sampling the path `framerate` times per +second of path time. For a `Figure`, the view is set on its current axis. +""" function Makie.record(fig::Makie.FigureLike, file::String, path::Path; framerate = 24, kwargs...) tend = FlyThroughPaths.duration(path) trange = LinRange(0, tend, FlyThroughPaths.nframes(path, framerate)) - iterator = path.(trange) - return Makie.record(fig, file, iterator; framerate, kwargs...) + return Makie.record(fig, file, trange; framerate, kwargs...) do t + set_view!(fig, path(t)) + end end # Define the recipe From 7562db7d145887b2883990fb93e948bc63c7bdf4 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:36:15 -0400 Subject: [PATCH 07/13] Note that a captured `ViewState`'s element type follows the camera `capture_view` now preserves the element type of the camera it reads, which is Float64 for `Camera3D` in recent versions of Makie. Co-Authored-By: Claude Opus 5 (1M context) --- docs/src/makie.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/src/makie.md b/docs/src/makie.md index c23911f..91555c2 100644 --- a/docs/src/makie.md +++ b/docs/src/makie.md @@ -23,9 +23,10 @@ First, we extract the initial view state from the axis `ax`. ```@example simple view0 = capture_view(ax) ``` -Note that this `ViewState` is a Float32 object, since that's the space -Makie cameras work in. If you want this to be Float64, you can -simply `convert(ViewState{Float64}, view0)`. +Note that the element type of this `ViewState` follows the space the Makie +camera works in, which is Float32 for older versions of Makie and Float64 +for newer ones. If you want a different element type, you can simply +`convert(ViewState{Float64}, view0)`. ### Creating a path From 633e1a0d846bae99c4a0eab86e7203b15a247ede Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 14:42:14 -0400 Subject: [PATCH 08/13] Exercise `record(fig, file, path)` in the manual Makie script The `Path` method of `record` is the one CI cannot reach, so drive it from the script that already needs a backend. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 1 + test/glmakie.jl | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index ac959bf..6f05882 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ /docs/Manifest.toml /docs/build/ fly_animation.mp4 +fly_animation_path.mp4 diff --git a/test/glmakie.jl b/test/glmakie.jl index bdfd0ca..7d6dd56 100644 --- a/test/glmakie.jl +++ b/test/glmakie.jl @@ -26,3 +26,7 @@ tlist = range(0, stop=15, length=31) record(fig, "fly_animation.mp4", tlist; framerate=round(Int, length(tlist)/last(tlist))) do t set_view!(ax, path(t)) end + +# `record` also accepts a `Path` directly, sampling it at `framerate` frames per second +# and setting the view on each frame (on the figure's current axis). +record(fig, "fly_animation_path.mp4", path; framerate=24) From edc04d4df5a33457c2554ddb6f4aefe18901633f Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 16:26:11 -0400 Subject: [PATCH 09/13] Use the GeometryOps tangent-vector form for the `:rotation` slerp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interpolate the direction as `cos(f*θ)*a + sin(f*θ)*dir`, with `dir` the unit tangent `normalize((a × b) × a)` at `a`, as `GeometryOps.UnitSpherical.slerp` does after S2. The plane of rotation is now taken from `a × b`, which stays accurate to a few `eps` however close the endpoints are to antipodal, so a move that is nearly a half turn follows the great circle its endpoints actually determine instead of falling back to an arbitrary axis. Only an exact half turn, where no plane is determined, still picks one arbitrarily. Co-Authored-By: Claude Opus 5 (1M context) --- src/pathchange.jl | 48 +++++++++++++++++++++++++++++++---------------- test/runtests.jl | 17 +++++++++++++++++ 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/pathchange.jl b/src/pathchange.jl index 1537943..862863b 100644 --- a/src/pathchange.jl +++ b/src/pathchange.jl @@ -158,6 +158,15 @@ The naive blend `cospi(f/2) * vold + sinpi(f/2) * vnew` is not a rotation: writi length is `d^2 * (1 + sinpi(f) * cos(θ))`, which is `d^2` only for `θ = 90°`. At `θ = 0` it swells to `d^2*2` halfway through, and at `θ = 180°` it passes through zero, i.e. through the point being looked at. + +The rotation uses the tangent-vector form `cos(r)*a + sin(r)*dir`, where `a` is the unit +direction of `vold`, `r = f*θ`, and `dir = normalize((a × b) × a)` is the unit tangent at +`a` pointing towards the unit direction `b` of `vnew`. This is the formulation used by +[GeometryOps.jl's `UnitSpherical.slerp`](https://github.com/JuliaGeo/GeometryOps.jl/blob/main/src/utils/UnitSpherical/slerp.jl), +which adapts it from Google's S2 geometry library. It is preferred to the textbook +`(sin((1-f)θ)*a + sin(f*θ)*b) / sin(θ)` because that divisor collapses as `θ` approaches +0 or `π`, whereas the tangent form only has to special-case the two configurations where +the plane of rotation is genuinely undetermined. """ function slerp(vold::SVector{3,T}, vnew::SVector{3,T}, f) where T f <= 0 && return vold @@ -165,26 +174,33 @@ function slerp(vold::SVector{3,T}, vnew::SVector{3,T}, f) where T dold, dnew = norm(vold), norm(vnew) # A vector of zero length has no direction to rotate, so interpolate linearly instead (iszero(dold) || iszero(dnew)) && return (1 - f) * vold + f * vnew - uold, unew = vold / dold, vnew / dnew + a, b = vold / dold, vnew / dnew # Geometric interpolation of the length: a constant relative rate of approach reads # more evenly than a linear one when the camera dollies in or out. d = dold * (dnew / dold)^f - c = clamp(dot(uold, unew), -one(T), one(T)) - s = sqrt(max(zero(T), 1 - c * c)) # sin(θ) - if s > sqrt(eps(T)) - θ = atan(s, c) - return d * normalize(sin((1 - f) * θ) * uold + sin(f * θ) * unew) - elseif c > 0 - # θ ≈ 0: the great circle is degenerate, but the chord approximates it well - return d * normalize((1 - f) * uold + f * unew) - end - # θ ≈ 180°: ill-conditioned, since every plane containing `uold` also contains `unew` - # and the direction of travel is therefore arbitrary. Pick one deterministically, by - # rotating in the plane spanned by `uold` and the axis it is least aligned with. - i = argmin(abs.(uold)) - uperp = normalize(cross(uold, SVector(ntuple(j -> T(j == i), 3)))) + # `n` is the normal of the plane of rotation and `norm(n)` is `sin(θ)`. `n` carries an + # absolute error of a few `eps(T)` however close `a` and `b` are to each other or to + # antipodal, so a norm at that level -- and only then -- means the inputs do not + # determine a plane at all. + n = cross(a, b) + s, c = norm(n), dot(a, b) θ = atan(s, c) - return d * (cos(f * θ) * uold + sin(f * θ) * uperp) + if s <= 8 * eps(T) + # θ ≈ 0: `a` and `b` are the same direction, so only the length changes + c > 0 && return d * a + # θ ≈ 180°: every plane containing `a` also contains `b`, so the direction of + # travel really is arbitrary. Rotate in the plane spanned by `a` and the + # coordinate axis it is least aligned with: an arbitrary choice, but a + # deterministic and well-conditioned one. (S2, and GeometryOps after it, resolve + # this case with exact arithmetic and symbolic perturbation instead, because + # their predicates must agree with each other from one call to the next. A camera + # fly-through has no such requirement, so that machinery is not reproduced here.) + i = argmin(abs.(a)) + n = cross(a, SVector(ntuple(j -> T(j == i), 3))) + end + dir = normalize(cross(n, a)) + r = f * θ + return d * normalize(cos(r) * a + sin(r) * dir) end # Compute the view from a PathChange at (relative) time t diff --git a/test/runtests.jl b/test/runtests.jl index 0728235..6f2f6c2 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -115,6 +115,23 @@ using Test # A move that only changes the distance still interpolates the distance smoothly rpath = Path(view0) * ConstrainedMove(1.0, ViewState(eyeposition = SVector(5.0, 0.0, 0.0)), :rotation, :constant) @test rpath(0.5).eyeposition ≈ [sqrt(50), 0, 0] # geometric mean of 10 and 5 + + # A move that is nearly, but not exactly, a half turn still follows the great + # circle its endpoints determine. Here that circle runs through +y, and the + # cross product still fixes its plane to a relative accuracy of 1e-7 even + # though `dot(a, b)` has already rounded to exactly -1 in Float64. + eyenew = 10 .* normalize(SVector(-1.0, 1e-9, 0.0)) + rpath = Path(view0) * ConstrainedMove(1.0, ViewState(eyeposition = eyenew), :rotation, :constant) + @test rpath(0.5).eyeposition ≈ [0, 10, 0] atol = 1e-6 + @test rpath(0.25).eyeposition ≈ 10 .* [cosd(45), sind(45), 0] atol = 1e-6 + # An exact half turn is ambiguous, so any great circle will do, but the radius + # must still be preserved and the move must stay perpendicular to its own axis + rpath = Path(view0) * ConstrainedMove(1.0, ViewState(eyeposition = SVector(-10.0, 0.0, 0.0)), :rotation, :constant) + @test norm(rpath(0.5).eyeposition) ≈ 10 + @test dot(rpath(0.5).eyeposition, view0.eyeposition) ≈ 0 atol = 1e-12 + # ...and the arc must be traced continuously, not jumped through + arc = [rpath(f).eyeposition for f in range(0, 1; length = 201)] + @test maximum(norm.(diff(arc))) < 0.2 end @testset "BezierMove" begin move = BezierMove(5, ViewState(eyeposition=[0, 10, 0]), [ViewState(eyeposition=[-20, 20, 0])]) From 9bf4188c93434de53ca8e30438b8be6ee273984c Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 16:26:17 -0400 Subject: [PATCH 10/13] Add a vectorized `path(ts)` for sampling a whole path The scalar `path(t)` restarts its walk over the path's changes on every call, both to find the change that owns `t` and to accumulate the `ViewState` it starts from. `path(ts)` does that walk once for a sorted vector of times and then locates each time with `searchsortedfirst` over the segment end times, giving results identical to `path.(ts)`. Co-Authored-By: Claude Opus 5 (1M context) --- src/path.jl | 46 ++++++++++++++++++++++++++++++++++++++++++++++ test/runtests.jl | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/src/path.jl b/src/path.jl index 8aa4697..c4bf613 100644 --- a/src/path.jl +++ b/src/path.jl @@ -54,3 +54,49 @@ function (path::Path{T})(t) where T end return view end + +""" + (path::Path)(ts::AbstractVector) + +Evaluate `path` at every time in `ts`, which must be sorted, and return the resulting +`Vector{ViewState}`. The result is identical to `path.(ts)`, elementwise. + +Prefer this to broadcasting when sampling a whole path, e.g. once per frame of a video. +The scalar method walks the path's changes from the beginning on every call, both to find +the change that owns `t` and to accumulate the `ViewState` that change starts from; this +method does that walk once and then locates each time by `searchsortedfirst` over the +segment end times. +""" +function (path::Path{T})(ts::AbstractVector) where T + issorted(ts) || throw(ArgumentError("`ts` must be sorted; broadcast `path.(ts)` instead")) + changes = path.changes + nchanges = length(changes) + # The start time of each change, the view it starts from, and its end time, all + # accumulated exactly as the scalar method accumulates them + tstarts = Vector{T}(undef, nchanges) + tstops = Vector{T}(undef, nchanges) + startviews = Vector{ViewState{T}}(undef, nchanges) + tend = zero(T) + endview = path.initialview + for (i, change) in enumerate(changes) + tstarts[i], startviews[i] = tend, endview + tstops[i] = tend = tend + duration(change) + endview = filldefaults(target(endview, change), endview) + end + result = Vector{ViewState{T}}(undef, length(ts)) + i = 1 # the changes are visited in order, since `ts` is sorted + for (k, t) in enumerate(ts) + if t < zero(T) + result[k] = path.initialview + continue + end + i <= nchanges && (i += searchsortedfirst(@view(tstops[i:nchanges]), t) - 1) + result[k] = if i > nchanges + endview + else + change = changes[i] + change(startviews[i], clamp(t - tstarts[i], zero(T), duration(change))) + end + end + return result +end diff --git a/test/runtests.jl b/test/runtests.jl index 6f2f6c2..bb1816e 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -167,6 +167,44 @@ using Test @test_throws ArgumentError move(view0, 1.5) @test_throws ArgumentError move(view0, -0.5) end + @testset "vector evaluation" begin + # `path(ts)` samples a whole sorted vector of times in one pass; it must agree + # with the scalar method exactly, including at the segment boundaries where + # `searchsortedfirst` has to make the same choice the scalar walk does. + view0 = ViewState(eyeposition = SVector(10.0, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), + upvector = SVector(0.0, 0.0, 1.0), fov = 45.0) + vpath = Path(view0) + for i in 1:6 + θ = 2π * i / 6 + target = ViewState(eyeposition = SVector(10cos(θ), 10sin(θ), 0.0)) + vpath = vpath * (isodd(i) ? ConstrainedMove(0.2, target, :rotation, :constant) : + ConstrainedMove(0.3, target, :none, :sinusoidal)) + vpath = vpath * Pause(0.1) + end + tend = FlyThroughPaths.duration(vpath) + bounds = cumsum(FlyThroughPaths.duration.(vpath.changes)) + ts = sort(vcat(collect(range(0, tend; length = 97)), bounds, + prevfloat.(bounds), nextfloat.(bounds), + [-1.0, -0.0, 0.0, tend, nextfloat(tend), tend + 1])) + @test vpath(ts) == vpath.(ts) + # An empty path and a single-element sample are not special-cased away + @test Path(view0)(ts) == Path(view0).(ts) + @test vpath([0.35]) == [vpath(0.35)] + @test isempty(vpath(Float64[])) + @test vpath(ts) isa Vector{ViewState{Float64}} + # Unsorted input would break the single forward pass, so it is rejected + @test_throws ArgumentError vpath([1.0, 0.5]) + + # The long Float32 path is the case the fast path exists for + longpath = Path(ViewState(eyeposition = SVector{3,Float32}(10, 0, 0), lookat = SVector{3,Float32}(0, 0, 0), + upvector = SVector{3,Float32}(0, 0, 1), fov = 45f0)) + for i in 1:200 + θ = 2π * i / 200 + longpath = longpath * ConstrainedMove(0.16f0, ViewState(eyeposition = SVector{3,Float32}(10cos(θ), 10sin(θ), 0)), :rotation, :constant) + end + trange = LinRange(0f0, FlyThroughPaths.duration(longpath), FlyThroughPaths.nframes(longpath, 24)) + @test longpath(trange) == longpath.(trange) + end @testset "nframes" begin # Used by the Makie extension to sample a path for `record` view0 = ViewState(eyeposition = SVector(10.0, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), From 8d207ad38f57f8f91aeafb9dc7f97672c8f8c65b Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 7 Aug 2026 16:26:23 -0400 Subject: [PATCH 11/13] Dispatch the extension's `record` on the object the path drives `set_view!(::Figure, ...)` guessed its target with `current_axis`; drop it. `record` therefore takes the object being flown -- a `Scene`, an axis, or a `FigureAxisPlot`, which names its axis -- rather than a figure whose axis has to be guessed. Recording an axis renders the whole figure it belongs to. The frames come from the vectorized `path(trange)`, so the path is walked once rather than searched per frame; the `plotcamerapath` recipe samples the same way. Co-Authored-By: Claude Opus 5 (1M context) --- ext/FlyThroughPathsMakieExt.jl | 40 ++++++++++++++++++++++------------ test/glmakie.jl | 5 +++-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/ext/FlyThroughPathsMakieExt.jl b/ext/FlyThroughPathsMakieExt.jl index 95ced51..1c9ebf6 100644 --- a/ext/FlyThroughPathsMakieExt.jl +++ b/ext/FlyThroughPathsMakieExt.jl @@ -24,24 +24,36 @@ function FlyThroughPaths.set_view!(scene::Scene, view::ViewState) return scene end FlyThroughPaths.set_view!(axis::Makie.AbstractAxis, view::ViewState) = set_view!(axis.scene, view) +# A `FigureAxisPlot` names its axis, so there is nothing to guess here FlyThroughPaths.set_view!(figaxplot::Makie.FigureAxisPlot, view::ViewState) = set_view!(figaxplot.axis, view) -function FlyThroughPaths.set_view!(fig::Makie.Figure, view::ViewState) - axis = Makie.current_axis(fig) - axis === nothing && throw(ArgumentError("`fig` has no current axis whose view could be set; pass the axis or scene instead.")) - return set_view!(axis, view) -end """ - record(figlike, file, path::Path; framerate = 24, kwargs...) + record(object, file, path::Path; framerate = 24, kwargs...) + +Record a video of the camera of `object` flying along `path`, sampling the path +`framerate` times per second of path time. Remaining keyword arguments are forwarded to +`Makie.record`. -Record a video of `figlike` flying along `path`, sampling the path `framerate` times per -second of path time. For a `Figure`, the view is set on its current axis. +`object` is whatever the path drives: a `Scene`, an axis, or a `FigureAxisPlot` (which +names its axis). Recording an axis records the whole figure it belongs to. A bare +`Figure` is not accepted, because it does not say which of its axes should be flown; use +`record(fig, file, trange) do t; set_view!(ax, path(t)); end` when you need one. """ -function Makie.record(fig::Makie.FigureLike, file::String, path::Path; framerate = 24, kwargs...) - tend = FlyThroughPaths.duration(path) - trange = LinRange(0, tend, FlyThroughPaths.nframes(path, framerate)) - return Makie.record(fig, file, trange; framerate, kwargs...) do t - set_view!(fig, path(t)) +function Makie.record(scene::Scene, file::String, path::Path; kwargs...) + return _record_path(scene, scene, file, path; kwargs...) +end +function Makie.record(axis::Makie.AbstractAxis, file::String, path::Path; kwargs...) + return _record_path(Makie.root(axis.scene), axis, file, path; kwargs...) +end +function Makie.record(figaxplot::Makie.FigureAxisPlot, file::String, path::Path; kwargs...) + return Makie.record(figaxplot.axis, file, path; kwargs...) +end + +function _record_path(figlike, viewtarget, file::String, path::Path; framerate = 24, kwargs...) + trange = LinRange(0, FlyThroughPaths.duration(path), FlyThroughPaths.nframes(path, framerate)) + views = path(trange) # one pass over the path, rather than a search per frame + return Makie.record(figlike, file, views; framerate, kwargs...) do viewstate + set_view!(viewtarget, viewstate) end end @@ -71,7 +83,7 @@ function Makie.plot!(plot::PlotCameraPath) onany(plot, plot.path, plot.density; update = true) do path, density tend = FlyThroughPaths.duration(path) trange_obs.val = LinRange(0.0, Float64(tend), FlyThroughPaths.nframes(path, density)) - eyepositions_obs.val = Makie.Point3d.(getproperty.(path.(trange_obs.val), :eyeposition)) + eyepositions_obs.val = Makie.Point3d.(getproperty.(path(trange_obs.val), :eyeposition)) notify(eyepositions_obs) notify(trange_obs) end diff --git a/test/glmakie.jl b/test/glmakie.jl index 7d6dd56..9aa9072 100644 --- a/test/glmakie.jl +++ b/test/glmakie.jl @@ -28,5 +28,6 @@ record(fig, "fly_animation.mp4", tlist; framerate=round(Int, length(tlist)/last( end # `record` also accepts a `Path` directly, sampling it at `framerate` frames per second -# and setting the view on each frame (on the figure's current axis). -record(fig, "fly_animation_path.mp4", path; framerate=24) +# and setting the view of the object it is given on each frame. That object is the axis +# (or scene) being flown, not the figure, which would not say which axis to fly. +record(ax, "fly_animation_path.mp4", path; framerate=24) From b03a2ee9d3618c2393955a97db4584c660dab808 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sat, 8 Aug 2026 16:29:58 -0400 Subject: [PATCH 12/13] Remove the extension's `record` method `Makie.record(figlike, file, path)` did not earn its keep: it only saved writing the `do` block that the docs already show, and it had to decide which object a path drives. Drop it, along with the `set_view!` method for `FigureAxisPlot` that existed to support it. Co-Authored-By: Claude Opus 5 (1M context) --- ext/FlyThroughPathsMakieExt.jl | 89 +++++++++++++++------------------- test/glmakie.jl | 8 +-- 2 files changed, 42 insertions(+), 55 deletions(-) diff --git a/ext/FlyThroughPathsMakieExt.jl b/ext/FlyThroughPathsMakieExt.jl index 1c9ebf6..2dac3ae 100644 --- a/ext/FlyThroughPathsMakieExt.jl +++ b/ext/FlyThroughPathsMakieExt.jl @@ -24,53 +24,30 @@ function FlyThroughPaths.set_view!(scene::Scene, view::ViewState) return scene end FlyThroughPaths.set_view!(axis::Makie.AbstractAxis, view::ViewState) = set_view!(axis.scene, view) -# A `FigureAxisPlot` names its axis, so there is nothing to guess here -FlyThroughPaths.set_view!(figaxplot::Makie.FigureAxisPlot, view::ViewState) = set_view!(figaxplot.axis, view) +# Define the recipe +import FlyThroughPaths: plotcamerapath, plotcamerapath! """ - record(object, file, path::Path; framerate = 24, kwargs...) + plotcamerapath(path::Path, [time]) -Record a video of the camera of `object` flying along `path`, sampling the path -`framerate` times per second of path time. Remaining keyword arguments are forwarded to -`Makie.record`. - -`object` is whatever the path drives: a `Scene`, an axis, or a `FigureAxisPlot` (which -names its axis). Recording an axis records the whole figure it belongs to. A bare -`Figure` is not accepted, because it does not say which of its axes should be flown; use -`record(fig, file, trange) do t; set_view!(ax, path(t)); end` when you need one. +Plot the eye positions along `path` as a line coloured by time, with an arrow showing +where the camera is looking at `time` (0 by default). """ -function Makie.record(scene::Scene, file::String, path::Path; kwargs...) - return _record_path(scene, scene, file, path; kwargs...) -end -function Makie.record(axis::Makie.AbstractAxis, file::String, path::Path; kwargs...) - return _record_path(Makie.root(axis.scene), axis, file, path; kwargs...) -end -function Makie.record(figaxplot::Makie.FigureAxisPlot, file::String, path::Path; kwargs...) - return Makie.record(figaxplot.axis, file, path; kwargs...) -end - -function _record_path(figlike, viewtarget, file::String, path::Path; framerate = 24, kwargs...) - trange = LinRange(0, FlyThroughPaths.duration(path), FlyThroughPaths.nframes(path, framerate)) - views = path(trange) # one pass over the path, rather than a search per frame - return Makie.record(figlike, file, views; framerate, kwargs...) do viewstate - set_view!(viewtarget, viewstate) - end -end - -# Define the recipe -import FlyThroughPaths: plotcamerapath, plotcamerapath! -@recipe(PlotCameraPath, path, time) do scene - Attributes( - colormap = Makie.inherit(scene, :colormap, :plasma), - color = Makie.inherit(scene, :color, :black), - linewidth = Makie.inherit(scene, :linewidth, 1.0), - linestyle = Makie.inherit(scene, :linestyle, :solid), - camera_marker = Makie.inherit(scene, :marker, :none), - camera_color = Makie.inherit(scene, :color, :black), - camera_markersize = Vec3f(2, 2, 3), - density = 30, # points per second - cycle = [:color,], - ) +@recipe PlotCameraPath (path, time) begin + "Colormap for the path, which is coloured by time." + colormap = @inherit colormap :plasma + color = @inherit color :black + linewidth = @inherit linewidth 1.0 + linestyle = @inherit linestyle :solid + camera_color = @inherit color :black + """ + Scales the arrow marking the camera. `automatic` sizes it from the bounding box of + the path, which is usually what you want, since a path can span any distance. + """ + camera_markerscale = Makie.automatic + "Sampling rate of the path, in points per second of path time." + density = 30 + cycle = [:color] end Makie.convert_arguments(::Type{<: PlotCameraPath}, path::Path, time::Number) = (path, Float64(time)) @@ -115,15 +92,25 @@ function Makie.plot!(plot::PlotCameraPath) linewidth = plot.linewidth, linestyle = plot.linestyle, ) - arrows!( - plot, - @lift([$eyeposition_obs]), + # The camera arrow has to be sized against the path, not against itself: `automatic` + # would scale it by its own bounding box, which is a single unit-length arrow. + arrowscale_obs = lift(plot, plot.camera_markerscale, eyepositions_obs) do scale, eyepositions + scale isa Makie.Automatic || return Float64(scale) + length(eyepositions) < 2 && return 1.0 + return 0.15 * maximum(Makie.widths(Rect3d(eyepositions))) + end + + # `align = :tail` puts the arrow's base at the eye, so it points where the camera looks + arrows3d!( + plot, + @lift([$eyeposition_obs]), @lift([$viewdir_obs]); - color = plot.camera_color, - arrowsize = plot.camera_markersize, - normalize = true, - shading = Makie.MultiLightShading, - align = :headstart, + color = plot.camera_color, + lengthscale = arrowscale_obs, + markerscale = arrowscale_obs, + normalize = true, + shading = true, + align = :tail, ) diff --git a/test/glmakie.jl b/test/glmakie.jl index 9aa9072..71cc52a 100644 --- a/test/glmakie.jl +++ b/test/glmakie.jl @@ -27,7 +27,7 @@ record(fig, "fly_animation.mp4", tlist; framerate=round(Int, length(tlist)/last( set_view!(ax, path(t)) end -# `record` also accepts a `Path` directly, sampling it at `framerate` frames per second -# and setting the view of the object it is given on each frame. That object is the axis -# (or scene) being flown, not the figure, which would not say which axis to fly. -record(ax, "fly_animation_path.mp4", path; framerate=24) +# The path itself can be plotted, in the space it flies through +f2, a2, p2 = surface(-8..8, -8..8, Makie.peaks()) +FlyThroughPaths.plotcamerapath!(a2, path, 7) +display(f2) From 5fa8a57dc959ea0003cb7ef5bd549ce6ebf78cc8 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sat, 8 Aug 2026 16:29:58 -0400 Subject: [PATCH 13/13] Port the `plotcamerapath` recipe to Makie 0.24 The recipe could not be instantiated at all: `Makie.inherit(scene, ...)` in an `@recipe ... do scene` block throws `MethodError: no method matching lookup_default(::Observable{Any}, ::Attributes)`, and `arrows!` no longer dispatches from inside a recipe. Move to the current `@recipe Name (args) begin ... end` form with `@inherit`, and to `arrows3d!` with `align = :tail` (the old `:headstart`) and `shading = true` (`MultiLightShading` is deprecated). `arrowsize` is gone, so the vector `camera_markersize` becomes a scalar `camera_markerscale`; it defaults to `automatic`, sized from the bounding box of the path rather than from the arrow itself, which made it invisible. Drops the unused `camera_marker` attribute, and narrows the `Makie` compat bound to the version this is tested against. Co-Authored-By: Claude Opus 5 (1M context) --- Project.toml | 2 +- test/runtests.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Project.toml b/Project.toml index 404465c..d96b61e 100644 --- a/Project.toml +++ b/Project.toml @@ -18,6 +18,6 @@ FlyThroughPathsMakieExt = "Makie" [compat] LinearAlgebra = "1" -Makie = "0.21, 0.22, 0.23, 0.24" +Makie = "0.24" StaticArrays = "1" julia = "1.10" diff --git a/test/runtests.jl b/test/runtests.jl index bb1816e..f194e13 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -206,7 +206,7 @@ using Test @test longpath(trange) == longpath.(trange) end @testset "nframes" begin - # Used by the Makie extension to sample a path for `record` + # Used by the Makie extension to sample a path at a given rate view0 = ViewState(eyeposition = SVector(10.0, 0.0, 0.0), lookat = SVector(0.0, 0.0, 0.0), upvector = SVector(0.0, 0.0, 1.0), fov = 45.0) tenseconds = Path(view0) * Pause(10.0)