Improve accuracy of set framerate in GLMakie (#3954)

* track time budget across frames

* update changelog

* add BudgetedTimer

* add GC.safepoint

* add tests

* pass through min_sleep, update docstring

---------

Co-authored-by: Simon <sdanisch@protonmail.com>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Improved accuracy of framerate settings in GLMakie [#3954](https://github.com/MakieOrg/Makie.jl/pull/3954)
 - Fix label_formatter being called twice in barplot [#4046](https://github.com/MakieOrg/Makie.jl/pull/4046).
 - Fix error with automatic `highclip` or `lowclip` and scalar colors [#4048](https://github.com/MakieOrg/Makie.jl/pull/4048).
 - Correct a bug in the `project` function when projecting using a `Scene`. [#3909](https://github.com/MakieOrg/Makie.jl/pull/3909).

GLMakie/src/GLMakie.jl

@@ -16,6 +16,7 @@ using Makie: @get_attribute, to_value, to_colormap, extrema_nan
 using Makie: ClosedInterval, (..)
 using Makie: to_native
 using Makie: spaces, is_data_space, is_pixel_space, is_relative_space, is_clip_space
+using Makie: BudgetedTimer, reset!
 import Makie: to_font, el32convert, Shape, CIRCLE, RECTANGLE, ROUNDED_RECTANGLE, DISTANCEFIELD, TRIANGLE
 import Makie: RelocatableFolders
 

GLMakie/src/screen.jl

@@ -163,6 +163,7 @@ mutable struct Screen{GLWindow} <: MakieScreen
     config::Union{Nothing, ScreenConfig}
     stop_renderloop::Bool
     rendertask::Union{Task, Nothing}
+    timer::BudgetedTimer
     px_per_unit::Observable{Float32}
 
     screen2scene::Dict{WeakRef, ScreenID}
@@ -202,7 +203,7 @@ mutable struct Screen{GLWindow} <: MakieScreen
         s = size(framebuffer)
         screen = new{GLWindow}(
             glscreen, shader_cache, framebuffer,
-            config, stop_renderloop, rendertask,
+            config, stop_renderloop, rendertask, BudgetedTimer(1.0 / 30.0),
             Observable(0f0), screen2scene,
             screens, renderlist, postprocessors, cache, cache2plot,
             Matrix{RGB{N0f8}}(undef, s), Observable(nothing),
@@ -898,29 +899,25 @@ function vsynced_renderloop(screen)
         end
         pollevents(screen) # GLFW poll
         render_frame(screen)
-        GLFW.SwapBuffers(to_native(screen))
         yield()
+        GC.safepoint()
+        GLFW.SwapBuffers(to_native(screen))
     end
 end
 
 function fps_renderloop(screen::Screen)
+    reset!(screen.timer, 1.0 / screen.config.framerate)
     while isopen(screen) && !screen.stop_renderloop
-        if screen.config.pause_renderloop
-            pollevents(screen); sleep(0.1)
-            continue
-        end
-        time_per_frame = 1.0 / screen.config.framerate
-        t = time_ns()
-        pollevents(screen) # GLFW poll
-        render_frame(screen)
-        GLFW.SwapBuffers(to_native(screen))
-        t_elapsed = (time_ns() - t) / 1e9
-        diff = time_per_frame - t_elapsed
-        if diff > 0.001 # can't sleep less than 0.001
-            sleep(diff)
-        else # if we don't sleep, we still need to yield explicitely to other tasks
-            yield()
+        pollevents(screen)
+
+        if !screen.config.pause_renderloop
+            pollevents(screen) # GLFW poll
+            render_frame(screen)
+            GLFW.SwapBuffers(to_native(screen))
         end
+
+        GC.safepoint()
+        sleep(screen.timer)
     end
 end
 
@@ -933,24 +930,26 @@ function requires_update(screen::Screen)
     return false
 end
 
+
+# const time_record = sizehint!(Float64[], 100_000)
+
 function on_demand_renderloop(screen::Screen)
+    # last_time = time_ns()
+    reset!(screen.timer, 1.0 / screen.config.framerate)
     while isopen(screen) && !screen.stop_renderloop
-        t = time_ns()
-        time_per_frame = 1.0 / screen.config.framerate
         pollevents(screen) # GLFW poll
 
         if !screen.config.pause_renderloop && requires_update(screen)
             render_frame(screen)
             GLFW.SwapBuffers(to_native(screen))
         end
 
-        t_elapsed = (time_ns() - t) / 1e9
-        diff = time_per_frame - t_elapsed
-        if diff > 0.001 # can't sleep less than 0.001
-            sleep(diff)
-        else # if we don't sleep, we still need to yield explicitely to other tasks
-            yield()
-        end
+        GC.safepoint()
+        sleep(screen.timer)
+
+        # t = time_ns()
+        # push!(time_record, 1e-9 * (t - last_time))
+        # last_time = t
     end
     cause = screen.stop_renderloop ? "stopped renderloop" : "closing window"
     @debug("Leaving renderloop, cause: $(cause)")

src/Makie.jl

@@ -112,6 +112,7 @@ include("utilities/quaternions.jl")
 include("utilities/stable-hashing.jl")
 include("bezier.jl")
 include("types.jl")
+include("utilities/timing.jl")
 include("utilities/texture_atlas.jl")
 include("interaction/observables.jl")
 include("interaction/liftmacro.jl")

src/utilities/timing.jl

@@ -0,0 +1,150 @@
+
+
+mutable struct BudgetedTimer
+    callback::Any
+
+    target_delta_time::Float64
+    min_sleep::Float64
+    budget::Float64
+    last_time::UInt64
+
+    running::Bool
+    task::Union{Nothing, Task}
+
+    function BudgetedTimer(callback, delta_time::Float64, running::Bool, task::Union{Nothing, Task}, min_sleep = 0.015)
+        return new(callback, delta_time, min_sleep, 0.0, time_ns(), running, task)
+    end
+end
+
+"""
+    BudgetedTimer(target_delta_time)
+    BudgetedTimer(callback, target_delta_time[, start = true])
+
+A timer that keeps track of a time budget between invocations of `sleep(timer)`,
+`busysleep(timer)` or roundtrips of the timed task. The budget is then used to 
+correct the next sleep so that the average sleep time matches the targeted delta
+time.
+
+To avoid lag spikes from hurrying the timer for multiple iterations/invocations
+only the difference to the nearest multiple of `target_delta_time` is counted. 
+E.g. if two calls to `sleep(timer)` are 2.3 delta times apart, 0.3 will be 
+relevant difference for the budget.
+"""
+function BudgetedTimer(delta_time::AbstractFloat; min_sleep = 0.015)
+    return BudgetedTimer(identity, delta_time, false, nothing, min_sleep)
+end
+
+function BudgetedTimer(callback, delta_time::AbstractFloat, start = true; min_sleep = 0.015)
+    timer = BudgetedTimer(callback, delta_time, true, nothing, min_sleep)
+    if start
+        timer.task = @async while timer.running
+            timer.callback()
+            sleep(timer)
+        end
+    end
+    return timer
+end
+
+function start!(timer::BudgetedTimer)
+    timer.budget = 0.0
+    timer.last_time = time_ns()
+    timer.running = true
+    timer.task = @async while timer.running
+        timer.callback()
+        sleep(timer)
+    end
+    return
+end
+
+function start!(callback, timer::BudgetedTimer)
+    timer.callback = callback
+    return start!(timer)
+end
+
+function stop!(timer::BudgetedTimer)
+    timer.running = false
+    return
+end
+
+function reset!(timer::BudgetedTimer, delta_time = timer.target_delta_time)
+    timer.target_delta_time = delta_time
+    timer.budget = 0.0
+    timer.last_time = time_ns()
+end
+
+function update_budget!(timer::BudgetedTimer)
+    # The real time that has passed
+    t = time_ns()
+    time_passed = 1e-9 * (t - timer.last_time)
+    # Update budget
+    diff_to_target = timer.target_delta_time + timer.budget - time_passed
+    if diff_to_target > -0.5 * timer.target_delta_time
+        # used 0 .. 1.5 delta_time, keep difference (1 .. -0.5 delta times) as budget
+        timer.budget = diff_to_target
+    else
+        # more than 1.5 delta_time used, get difference to next multiple of 
+        # delta_time as the budget
+        timer.budget = ((diff_to_target - 0.5 * timer.target_delta_time) 
+            % timer.target_delta_time) + 0.5 * timer.target_delta_time
+    end
+    timer.last_time = t
+    return
+end
+
+"""
+    sleep(timer::BudgetedTimer)
+
+Sleep until one `timer.target_delta_time` has passed since the last call to
+`sleep(timer)` or `busysleep(timer)` with the current time budget included.
+
+This only relies on `Base.sleep()` for waiting.
+
+This always yields to other tasks.
+"""
+function Base.sleep(timer::BudgetedTimer)
+    # time since last sleep
+    time_passed = 1e-9 * (time_ns() - timer.last_time)
+    # How much time we should sleep for considering the real time we slept
+    # for in the last iteration and biasing for the minimum sleep time
+    sleep_time = timer.target_delta_time + timer.budget - time_passed - 0.5 * timer.min_sleep
+    if sleep_time > 0.0
+        sleep(sleep_time) 
+    else
+        yield()
+    end
+
+    update_budget!(timer)
+    return
+end
+
+"""
+    busysleep(timer::BudgetedTimer)
+
+Sleep until one `timer.target_delta_time` has passed since the last call to
+`sleep(timer)` or `busysleep(timer)` with the current time budget included.
+
+This uses `Base.sleep()` for an initial longer sleep and a time-checking while 
+loop for the remaining time for more precision.
+
+This always yields to other tasks.
+"""
+function busysleep(timer::BudgetedTimer)
+    # use normal sleep as much as possible
+    time_passed = 1e-9 * (time_ns() - timer.last_time)
+    sleep_time = timer.target_delta_time - time_passed + timer.budget - timer.min_sleep
+    if sleep_time > 0.0
+        sleep(sleep_time) 
+    else
+        yield()
+    end
+
+    # busy sleep remaining time
+    time_passed = 1e-9 * (time_ns() - timer.last_time)
+    sleep_time = timer.target_delta_time - time_passed + timer.budget
+    while time_ns() < timer.last_time + 1e9 * timer.target_delta_time + timer.budget
+        yield()
+    end
+
+    update_budget!(timer)
+    return
+end

test/timing.jl

@@ -0,0 +1,67 @@
+@testset "BudgetedTimer" begin
+
+    t = time_ns()
+    dt = 1.0 / 30.0
+    timer = Makie.BudgetedTimer(dt)
+
+    @testset "Initialization" begin
+        @test timer.target_delta_time == dt
+        @test timer.budget == 0.0
+        @test t < timer.last_time < time_ns()
+    end
+
+    @testset "sleep()" begin
+        sleep(timer) # just in case for compilation
+
+        t = time_ns()
+        for _ in 1:100
+            sleep(timer)
+        end
+        real_dt = 1e-9 * (time_ns() - t)
+
+        @test 98 * dt < real_dt < 102 * dt
+    end
+
+    t = time_ns()
+    dt = 0.03
+
+    @testset "reset!()" begin
+        Makie.reset!(timer, dt)
+
+        @test timer.target_delta_time == dt
+        @test timer.budget == 0.0
+        @test t < timer.last_time < time_ns()
+    end
+
+    @testset "busysleep()" begin
+        Makie.busysleep(timer)
+        t = time_ns()
+        for _ in 1:100
+            Makie.busysleep(timer)
+        end
+        real_dt = 1e-9 * (time_ns() - t)
+
+        @test 99.9 * dt < real_dt < 100.1 * dt
+    end
+
+    @testset "callbacks" begin
+        counter = 0
+        timer = Makie.BudgetedTimer(1.0 / 30.0, false) do 
+            global counter += 1
+        end
+        sleep(0.5)
+        @test counter == 0
+
+        t = time_ns()
+        Makie.start!(timer)
+        sleep(1.0)
+        Makie.stop!(timer)
+        real_dt = 1e-9 * (time_ns() - t)
+        N = counter
+
+        @test real_dt * 30.0 - 2 < counter < real_dt * 30.0 + 2
+
+        sleep(0.5)
+        @test counter == N
+    end
+end