Skip to content

Commit

Permalink
empty! the GLMakie screen for reuse instead of closeing and reope…
Browse files Browse the repository at this point in the history
…ning (#3881)

* `empty!` the GLMakie screen for reuse instead of `close`ing and reopening

This fixes a problem observed on Linux (across at least a couple of
different desktop environments) with windows flashing away and coming
back in a different place (can be a different monitor!) when reusing the
shared singleton screen.

Also, wait for renderloop task to stop before restoring
`close_after_renderloop` value. Without this, one can observe that the
singleton window is fully closed and reopened by applying the following
patch:

```diff
diff --git a/GLMakie/src/screen.jl b/GLMakie/src/screen.jl
index ff8b69fdc..e18b8ba6b 100644
--- a/GLMakie/src/screen.jl
+++ b/GLMakie/src/screen.jl
@@ -839,6 +839,7 @@ function pause_renderloop!(screen::Screen)
 end

 function stop_renderloop!(screen::Screen; close_after_renderloop=screen.close_after_renderloop)
+    yield()
     # don't double close when stopping renderloop
     c = screen.close_after_renderloop
     screen.close_after_renderloop = close_after_renderloop
@@ -974,7 +975,7 @@ function renderloop(screen)
     end
     if screen.close_after_renderloop
         try
-            @debug("Closing screen after quiting renderloop!")
+            @info("Closing screen after quiting renderloop!")
             close(screen)
         catch e
             @warn "error closing screen" exception=(e, Base.catch_backtrace())
```
which aims to force the renderloop's task to run via the call to
`yield()` so that the task is sleeping during the rest of the function
call. (The logging change just makes the particular action easier to
find than enabling debug-level logging.)

Opening a plot and replotting at the REPL, I observe both the window
quickly close and reappear and the log message being printed by the end
of the renderloop:
```julia-repl
julia> using GLMakie
Precompiling GLMakie
        Info Given GLMakie was explicitly requested, output will be shown live
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
[ Info: Closing screen after quiting renderloop!
  1 dependency successfully precompiled in 21 seconds. 303 already precompiled.
  1 dependency had output during precompilation:
┌ GLMakie
│  [Output was shown above]
└

julia> scatter([1, 6])

julia> scatter([1, 6])
[ Info: Closing screen after quiting renderloop!

```

With the changes in this commit, all of the log message disappear,
including from the precompile process.

* Update CHANGELOG.md

---------

Co-authored-by: Simon <[email protected]>
Co-authored-by: Frederic Freyer <[email protected]>
  • Loading branch information
3 people authored Dec 16, 2024
1 parent d0909e7 commit a1e9fdf
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- Added `transform_marker` attribute to meshscatter and changed the default behavior to not transform marker/mesh vertices [#4606](https://github.com/MakieOrg/Makie.jl/pull/4606)
- Fixed some issues with meshscatter not correctly transforming with transform functions and float32 rescaling [#4606](https://github.com/MakieOrg/Makie.jl/pull/4606)
- Fixed `poly` pipeline for 3D and/or Float64 polygons that begin from an empty vector [#4615](https://github.com/MakieOrg/Makie.jl/pull/4615).
- `empty!` GLMakie screen instead of closing, fixing issue with resetted window position [#3881](https://github.com/MakieOrg/Makie.jl/pull/3881)
- Added option to display the front spines in Axis3 to close the outline box [#2349](https://github.com/MakieOrg/Makie.jl/pull/4305)
- Fixed gaps in corners of `poly(Rect2(...))` stroke [#4664](https://github.com/MakieOrg/Makie.jl/pull/4664)
- Fixed an issue where `reinterpret`ed arrays of line points were not handled correctly in CairoMakie [#4668](https://github.com/MakieOrg/Makie.jl/pull/4668).
Expand Down
6 changes: 3 additions & 3 deletions GLMakie/src/screen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,8 @@ function singleton_screen(debugging::Bool)
if !isempty(SINGLETON_SCREEN)
@debug("reusing singleton screen")
screen = SINGLETON_SCREEN[1]
close(screen; reuse=false)
stop_renderloop!(screen; close_after_renderloop=false)
empty!(screen)
else
@debug("new singleton screen")
# reuse=false, because we "manually" re-use the singleton screen!
Expand Down Expand Up @@ -845,8 +846,6 @@ function stop_renderloop!(screen::Screen; close_after_renderloop=screen.close_af
c = screen.close_after_renderloop
screen.close_after_renderloop = close_after_renderloop
screen.stop_renderloop[] = true
screen.close_after_renderloop = c

# stop_renderloop! may be called inside renderloop as part of close
# in which case we should not wait for the task to finish (deadlock)
if Base.current_task() != screen.rendertask
Expand All @@ -855,6 +854,7 @@ function stop_renderloop!(screen::Screen; close_after_renderloop=screen.close_af
screen.rendertask = nothing
end
# else, we can't do that much in the rendertask itself
screen.close_after_renderloop = c
return
end

Expand Down

0 comments on commit a1e9fdf

Please sign in to comment.