check screen outside animationframe + upload electron logs in CI

.github/workflows/reference_tests.yml

@@ -157,6 +157,11 @@ jobs:
         with:
           name: ReferenceImages_WGLMakie_${{ matrix.version }}
           path: ./WGLMakie/test/reference_images/
+      - name: Upload test Electron logs
+        uses: actions/upload-artifact@v4
+        with:
+          name: Electron_Logs_WGLMakie_${{ matrix.version }}
+          path: ./WGLMakie/test/electron.log
       - name: Fail after artifacts if tests failed
         if: ${{ env.TESTS_SUCCESSFUL != 'true' }}
         run: exit 1

.gitignore

@@ -25,3 +25,4 @@ docs/documentation/news.md
 metrics/ttfp/current-pr-project/
 metrics/ttfp/benchmark-projects/*
 *benchmark.json
+WGLMakie/test/electron.log

WGLMakie/src/wglmakie.bundled.js

@@ -22911,19 +22911,22 @@ function dispose_screen(screen) {
     Object.keys(screen).forEach((key)=>delete screen[key]);
     return;
 }
-function render_scene(scene, picking = false) {
-    if (scene.screen == {}) {
-        return false;
-    }
-    const { camera , renderer , px_per_unit  } = scene.screen;
-    if (!renderer) {
-        dispose_screen(scene.screen);
+function check_screen(screen) {
+    if (!screen || !screen.renderer) {
+        dispose_screen(screen);
         return false;
     }
-    const canvas = renderer.domElement;
+    const canvas = screen.renderer.domElement;
     if (!document.body.contains(canvas)) {
         console.log("removing WGL context, canvas is not in the DOM anymore!");
-        dispose_screen(scene.screen);
+        dispose_screen(screen);
+        return false;
+    }
+    return true;
+}
+function render_scene(scene, picking = false) {
+    const { renderer , camera , px_per_unit  } = scene.screen;
+    if (!check_screen(scene.screen)) {
         return false;
     }
     if (!scene.visible.value) {
@@ -22952,16 +22955,26 @@ function start_renderloop(three_scene) {
     const time_per_frame = 1 / fps * 1000;
     let last_time_stamp = performance.now();
     function renderloop(timestamp) {
+        if (!check_screen(three_scene.screen)) {
+            return false;
+        }
         if (timestamp - last_time_stamp > time_per_frame) {
             const all_rendered = render_scene(three_scene);
             if (!all_rendered) {
                 return;
             }
             last_time_stamp = performance.now();
         }
-        window.requestAnimationFrame(renderloop);
+        requestAnimationFrame(renderloop);
+    }
+    function _check_screen() {
+        if (!check_screen(three_scene.screen)) {
+            return;
+        }
+        setTimeout(_check_screen, 1000);
     }
     render_scene(three_scene);
+    _check_screen();
     renderloop();
 }
 function get_body_size() {

WGLMakie/src/wglmakie.js

@@ -50,19 +50,23 @@ function dispose_screen(screen) {
     return;
 }
 
-export function render_scene(scene, picking = false) {
-    if (scene.screen == {}) {
-        return false;
-    }
-    const { camera, renderer, px_per_unit } = scene.screen;
-    if (!renderer) {
-        dispose_screen(scene.screen);
+function check_screen(screen) {
+    if (!screen || !screen.renderer) {
+        dispose_screen(screen);
         return false;
     }
-    const canvas = renderer.domElement;
+    const canvas = screen.renderer.domElement;
     if (!document.body.contains(canvas)) {
         console.log("removing WGL context, canvas is not in the DOM anymore!");
-        dispose_screen(scene.screen);
+        dispose_screen(screen);
+        return false;
+    }
+    return true;
+}
+
+export function render_scene(scene, picking = false) {
+    const { renderer, camera, px_per_unit } = scene.screen;
+    if (!check_screen(scene.screen)) {
         return false;
     }
     // dont render invisible scenes
@@ -95,6 +99,9 @@ function start_renderloop(three_scene) {
     // make sure we immediately render the first frame and dont wait 30ms
     let last_time_stamp = performance.now();
     function renderloop(timestamp) {
+        if (!check_screen(three_scene.screen)) {
+            return false;
+        }
         if (timestamp - last_time_stamp > time_per_frame) {
             const all_rendered = render_scene(three_scene);
             if (!all_rendered) {
@@ -104,10 +111,22 @@ function start_renderloop(three_scene) {
             }
             last_time_stamp = performance.now();
         }
-        window.requestAnimationFrame(renderloop);
+        requestAnimationFrame(renderloop);
     }
+    function _check_screen() {
+        // make sure we delete the screen if the canvas is not in the DOM anymore
+        if (!check_screen(three_scene.screen)){
+            return;
+        }
+        // this can't happen via requestAnimationFrame
+        // since it may not be called after the canvas got removed.
+        // So we need another check outside the renderloop (which can run a lot slower)
+        setTimeout(_check_screen, 1000);
+    }
+
     // render one time before starting loop, so that we don't wait 30ms before first render
     render_scene(three_scene);
+    _check_screen();
     renderloop();
 }
 

WGLMakie/test/runtests.jl

@@ -1,3 +1,6 @@
+ENV["ELECTRON_LOG_FILE"] = joinpath(@__DIR__, "electron.log")
+ENV["ELECTRON_ENABLE_LOGGING"] = "true"
+
 using FileIO
 using WGLMakie, Makie, Test
 using WGLMakie.Bonito
@@ -30,129 +33,131 @@ excludes = Set([
 ])
 
 Makie.inline!(Makie.automatic)
-edisplay = Bonito.use_electron_display(devtools=false)
-
-@testset "refimages" begin
-    WGLMakie.activate!()
-    ReferenceTests.mark_broken_tests(excludes)
-    recorded_files, recording_dir = @include_reference_tests WGLMakie "refimages.jl"
-    missing_images, scores = ReferenceTests.record_comparison(recording_dir, "WGLMakie")
-    ReferenceTests.test_comparison(scores; threshold = 0.05)
-end
-
-@testset "memory leaks" begin
-    Makie.CURRENT_FIGURE[] = nothing
-    app = App(nothing)
-    display(edisplay, app)
-    GC.gc(true);
-    # Somehow this may take a while to get emptied completely
-    p_key = "Object.keys(WGL.plot_cache)"
-    value = @time Bonito.wait_for(() -> (GC.gc(true); isempty(run(edisplay.window, p_key))); timeout=20)
-    @show run(edisplay.window, p_key)
-    @test value == :success
-
-    s_keys = "Object.keys(Bonito.Sessions.SESSIONS)"
-    value = @time Bonito.wait_for(() -> (GC.gc(true); length(run(edisplay.window, s_keys)) == 2); timeout=20)
-    @show run(edisplay.window, s_keys)
-    @show app.session[].id
-    @show app.session[].parent
-    # It seems, we don't free all sessions right now, which needs fixing.
-    # @test value == :success
-
-    wgl_plots = run(edisplay.window, "Object.keys(WGL.scene_cache)")
-    @test isempty(wgl_plots)
-
-    session = edisplay.browserdisplay.handler.session
-    session_size = Base.summarysize(session) / 10^6
-    texture_atlas_size = Base.summarysize(WGLMakie.TEXTURE_ATLAS) / 10^6
-
-    @test length(WGLMakie.TEXTURE_ATLAS.listeners) == 1 # Only one from permanent Retain
-    @test length(session.session_objects) == 1 # Also texture atlas because of Retain
-    @testset "Session fields empty" for field in [:on_document_load, :stylesheets, :imports, :message_queue, :deregister_callbacks, :inbox]
-        @test isempty(getfield(session, field))
+edisplay = Bonito.use_electron_display(devtools=true)
+
+@testset "reference tests" begin
+    @testset "refimages" begin
+        WGLMakie.activate!()
+        ReferenceTests.mark_broken_tests(excludes)
+        recorded_files, recording_dir = @include_reference_tests WGLMakie "refimages.jl"
+        missing_images, scores = ReferenceTests.record_comparison(recording_dir, "WGLMakie")
+        ReferenceTests.test_comparison(scores; threshold = 0.05)
     end
-    server = session.connection.server
-    @test length(server.websocket_routes.table) == 1
-    @test server.websocket_routes.table[1][2] == session.connection
-    @test length(server.routes.table) == 2
-    @test server.routes.table[1][1] == "/browser-display"
-    @test server.routes.table[2][2] isa HTTPAssetServer
-    @show typeof.(last.(WGLMakie.TEXTURE_ATLAS.listeners))
-    @show length(WGLMakie.TEXTURE_ATLAS.listeners)
-    @show session_size texture_atlas_size
-
-    # TODO, this went up from 6 to 11mb, likely because of a session not getting freed
-    # It could be related to the error in the console:
-    # " Trying to send to a closed session"
-    # So maybe a subsession closes and doesn't get freed?
-    @test session_size < 11
-    @test texture_atlas_size < 11
-
-    js_sessions = run(edisplay.window, "Bonito.Sessions.SESSIONS")
-    js_objects = run(edisplay.window, "Bonito.Sessions.GLOBAL_OBJECT_CACHE")
-    # @test Set([app.session[].id, app.session[].parent.id]) == keys(js_sessions)
-    # we used Retain for global_obs, so it should stay as long as root session is open
-    @test keys(js_objects) == Set([WGLMakie.TEXTURE_ATLAS.id])
-end
 
-@testset "Tick Events" begin
-    function check_tick(tick, state, count)
-        @test tick.state == state
-        @test tick.count == count
-        @test tick.time > 1e-9
-        @test tick.delta_time > 1e-9
+    @testset "memory leaks" begin
+        Makie.CURRENT_FIGURE[] = nothing
+        app = App(nothing)
+        display(edisplay, app)
+        GC.gc(true);
+        # Somehow this may take a while to get emptied completely
+        p_key = "Object.keys(WGL.plot_cache)"
+        value = @time Bonito.wait_for(() -> (GC.gc(true); isempty(run(edisplay.window, p_key))); timeout=50)
+        @show run(edisplay.window, p_key)
+        @test value == :success
+
+        s_keys = "Object.keys(Bonito.Sessions.SESSIONS)"
+        value = @time Bonito.wait_for(() -> (GC.gc(true); length(run(edisplay.window, s_keys)) == 2); timeout=50)
+        @show run(edisplay.window, s_keys)
+        @show app.session[].id
+        @show app.session[].parent
+        # It seems, we don't free all sessions right now, which needs fixing.
+        # @test value == :success
+
+        wgl_plots = run(edisplay.window, "Object.keys(WGL.scene_cache)")
+        @test isempty(wgl_plots)
+
+        session = edisplay.browserdisplay.handler.session
+        session_size = Base.summarysize(session) / 10^6
+        texture_atlas_size = Base.summarysize(WGLMakie.TEXTURE_ATLAS) / 10^6
+
+        @test length(WGLMakie.TEXTURE_ATLAS.listeners) == 1 # Only one from permanent Retain
+        @test length(session.session_objects) == 1 # Also texture atlas because of Retain
+        @testset "Session fields empty" for field in [:on_document_load, :stylesheets, :imports, :message_queue, :deregister_callbacks, :inbox]
+            @test isempty(getfield(session, field))
+        end
+        server = session.connection.server
+        @test length(server.websocket_routes.table) == 1
+        @test server.websocket_routes.table[1][2] == session.connection
+        @test length(server.routes.table) == 2
+        @test server.routes.table[1][1] == "/browser-display"
+        @test server.routes.table[2][2] isa HTTPAssetServer
+        @show typeof.(last.(WGLMakie.TEXTURE_ATLAS.listeners))
+        @show length(WGLMakie.TEXTURE_ATLAS.listeners)
+        @show session_size texture_atlas_size
+
+        # TODO, this went up from 6 to 11mb, likely because of a session not getting freed
+        # It could be related to the error in the console:
+        # " Trying to send to a closed session"
+        # So maybe a subsession closes and doesn't get freed?
+        @test session_size < 11
+        @test texture_atlas_size < 11
+
+        js_sessions = run(edisplay.window, "Bonito.Sessions.SESSIONS")
+        js_objects = run(edisplay.window, "Bonito.Sessions.GLOBAL_OBJECT_CACHE")
+        # @test Set([app.session[].id, app.session[].parent.id]) == keys(js_sessions)
+        # we used Retain for global_obs, so it should stay as long as root session is open
+        @test keys(js_objects) == Set([WGLMakie.TEXTURE_ATLAS.id])
     end
 
-    f, a, p = scatter(rand(10));
-    @test events(f).tick[] == Makie.Tick()
-
-    filename = "$(tempname()).png"
-    try
-        tick_record = Makie.Tick[]
-        on(tick -> push!(tick_record, tick), events(f).tick)
-        save(filename, f)
-        idx = findfirst(tick -> tick.state == Makie.OneTimeRenderTick, tick_record)
-        tick = tick_record[idx]
-        @test tick.state == Makie.OneTimeRenderTick
-        @test tick.count == 0
-        @test tick.time == 0.0
-        @test tick.delta_time == 0.0
-    finally
-        close(f.scene.current_screens[1])
-        rm(filename)
-    end
+    @testset "Tick Events" begin
+        function check_tick(tick, state, count)
+            @test tick.state == state
+            @test tick.count == count
+            @test tick.time > 1e-9
+            @test tick.delta_time > 1e-9
+        end
 
+        f, a, p = scatter(rand(10));
+        @test events(f).tick[] == Makie.Tick()
 
-    f, a, p = scatter(rand(10));
-    filename = "$(tempname()).mp4"
-    try
-        tick_record = Makie.Tick[]
-        on(tick -> push!(tick_record, tick), events(f).tick)
-        record(_ -> nothing, f, filename, 1:10, framerate = 30)
+        filename = "$(tempname()).png"
+        try
+            tick_record = Makie.Tick[]
+            on(tick -> push!(tick_record, tick), events(f).tick)
+            save(filename, f)
+            idx = findfirst(tick -> tick.state == Makie.OneTimeRenderTick, tick_record)
+            tick = tick_record[idx]
+            @test tick.state == Makie.OneTimeRenderTick
+            @test tick.count == 0
+            @test tick.time == 0.0
+            @test tick.delta_time == 0.0
+        finally
+            close(f.scene.current_screens[1])
+            rm(filename)
+        end
 
-        start = findfirst(tick -> tick.state == Makie.OneTimeRenderTick, tick_record)
-        dt = 1.0 / 30.0
 
-        for (i, tick) in enumerate(tick_record[start:end])
-            @test tick.state == Makie.OneTimeRenderTick
-            @test tick.count == i-1
-            @test tick.time ≈ dt * (i-1)
-            @test tick.delta_time ≈ dt
+        f, a, p = scatter(rand(10));
+        filename = "$(tempname()).mp4"
+        try
+            tick_record = Makie.Tick[]
+            on(tick -> push!(tick_record, tick), events(f).tick)
+            record(_ -> nothing, f, filename, 1:10, framerate = 30)
+
+            start = findfirst(tick -> tick.state == Makie.OneTimeRenderTick, tick_record)
+            dt = 1.0 / 30.0
+
+            for (i, tick) in enumerate(tick_record[start:end])
+                @test tick.state == Makie.OneTimeRenderTick
+                @test tick.count == i-1
+                @test tick.time ≈ dt * (i-1)
+                @test tick.delta_time ≈ dt
+            end
+        finally
+            rm(filename)
         end
-    finally
-        rm(filename)
-    end
 
-    # test destruction of tick overwrite
-    f, a, p = scatter(rand(10));
-    let
-        io = VideoStream(f)
-        @test events(f).tick[] == Makie.Tick(Makie.OneTimeRenderTick, 0, 0.0, 1.0 / io.options.framerate)
-        nothing
-    end
-    tick = Makie.Tick(Makie.UnknownTickState, 1, 1.0, 1.0)
-    events(f).tick[] = tick
-    @test events(f).tick[] == tick
+        # test destruction of tick overwrite
+        f, a, p = scatter(rand(10));
+        let
+            io = VideoStream(f)
+            @test events(f).tick[] == Makie.Tick(Makie.OneTimeRenderTick, 0, 0.0, 1.0 / io.options.framerate)
+            nothing
+        end
+        tick = Makie.Tick(Makie.UnknownTickState, 1, 1.0, 1.0)
+        events(f).tick[] = tick
+        @test events(f).tick[] == tick
 
-    # TODO: test normal rendering
+        # TODO: test normal rendering
+    end
 end